Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 --- A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 4 files changed, 218 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/1
diff --git a/src/superio/nuvoton/nct668xd/Kconfig b/src/superio/nuvoton/nct668xd/Kconfig new file mode 100644 index 0000000..ac31463 --- /dev/null +++ b/src/superio/nuvoton/nct668xd/Kconfig @@ -0,0 +1,24 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2016 Omar Pakker omarpakker+coreboot@gmail.com +## Copyright (C) 2019 Michael Niewöhner foss@mniewoehner.de +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +config SUPERIO_NUVOTON_NCT668XD + bool + select SUPERIO_NUVOTON_COMMON_PRE_RAM + +config SUPERIO_NUVOTON_NCT668XD_COM_A + bool + depends on SUPERIO_NUVOTON_NCT668XD + default n diff --git a/src/superio/nuvoton/nct668xd/Makefile.inc b/src/superio/nuvoton/nct668xd/Makefile.inc new file mode 100644 index 0000000..5ab05c6 --- /dev/null +++ b/src/superio/nuvoton/nct668xd/Makefile.inc @@ -0,0 +1,17 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2016 Omar Pakker omarpakker+coreboot@gmail.com +## Copyright (C) 2019 Michael Niewöhner foss@mniewoehner.de +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +ramstage-$(CONFIG_SUPERIO_NUVOTON_NCT668XD) += superio.c diff --git a/src/superio/nuvoton/nct668xd/nct668Xd.h b/src/superio/nuvoton/nct668xd/nct668Xd.h new file mode 100644 index 0000000..6b74b2d --- /dev/null +++ b/src/superio/nuvoton/nct668xd/nct668Xd.h @@ -0,0 +1,79 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2016 Omar Pakker omarpakker+coreboot@gmail.com + * Copyright (C) 2019 Michael Niewöhner foss@mniewoehner.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SUPERIO_NUVOTON_NCT668XD_H +#define SUPERIO_NUVOTON_NCT668XD_H + +/* WARNING! + * NCT668XD is a "new generation" SuperIO, which implements parts of its + * functionality in firmware. That means one should check if the desired + * function/register is mentioned in the EC Space datasheet before trying + * to modify any registers in HW. + * Both datasheets (HW and EC Space) are available on request from Nuvoton. + * + * These are the warnings from the datasheet: + * 1. All GPIO pin functions should always be customized by firmware. BIOS / + * Driver should not touch all configuration registers here and related IO + * ports unless firmware opens them. + * 2. Under any situations, CR30h should always be controlled by EC and never + * be opened for BIOS / Drivers !! + * 3. Some GPIO pin functions were configured when related SW functions of EC + * Space were enabled. For such situations BIOS or application programs + * should not alter these setting to avoid abnormal function of underlying + * firmware. Please refer to EC Space Specification before going to change + * any configuration setting of GPIO pins. + */ + +/* Logical Device Numbers (LDN) */ +#define NCT668XD_PP 0x01 /* Parallel Port */ +#define NCT668XD_SP1 0x02 /* UART A */ +#define NCT668XD_SP2 0x03 /* UART B, IR */ +#define NCT668XD_KBC 0x05 /* Keyboard Controller */ +#define NCT668XD_CIR 0x06 /* Consumer IR */ +#define NCT668XD_GPIO01234567 0x07 /* GPIO 0-7 */ +#define NCT668XD_PORT80 0x08 /* Port 80 UART */ +#define NCT668XD_GPIO89 0x09 /* GPIO 8-9, GPIO 1-8 Alternate \ + * Func., GPIO 0-1 Enhance Group \ + */ +#define NCT668XD_ACPI 0x0A /* ACPI */ +#define NCT668XD_EC 0x0B /* EC Space */ +#define NCT668XD_DSLP_PWRFAULT 0x0D /* Deep Sleep, Power Fault */ +#define NCT668XD_FAN_ASSIGN 0x0E /* Fan Assignment */ + +/* Virtual LDNs */ +#define NCT668XD_WDT1 ((0 << 8) | NCT668XD_WDT1_WDTMEM_GPIO01) +#define NCT668XD_WDTMEM ((4 << 8) | NCT668XD_WDT1_WDTMEM_GPIO01) +#define NCT668XD_GPIO0 ((0 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO1 ((1 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO2 ((2 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO3 ((3 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO4 ((4 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO5 ((5 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO6 ((6 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO7 ((7 << 8) | NCT668XD_GPIO01234567) +#define NCT668XD_GPIO8 ((0 << 8) | NCT668XD_GPIO89) +#define NCT668XD_GPIO9 ((1 << 8) | NCT668XD_GPIO89) +#define NCT668XD_DS5 ((0 << 8) | NCT668XD_DS) +#define NCT668XD_DS3 ((1 << 8) | NCT668XD_DS) +#define NCT668XD_PCHDSW ((3 << 8) | NCT668XD_DS) +#define NCT668XD_DSWWOPT ((4 << 8) | NCT668XD_DS) +#define NCT668XD_DS3OPT ((5 << 8) | NCT668XD_DS) +#define NCT668XD_DSDSS ((6 << 8) | NCT668XD_DS) +#define NCT668XD_DSPU ((7 << 8) | NCT668XD_DS) + + +#endif /* SUPERIO_NUVOTON_NCT668XD_H */ diff --git a/src/superio/nuvoton/nct668xd/superio.c b/src/superio/nuvoton/nct668xd/superio.c new file mode 100644 index 0000000..99d448d --- /dev/null +++ b/src/superio/nuvoton/nct668xd/superio.c @@ -0,0 +1,98 @@ +/* + * This file is part of the coreboot project. + * + * Copyright (C) 2011 Advanced Micro Devices, Inc. + * Copyright (C) 2014 Felix Held felix-coreboot@felixheld.de + * Copyright (C) 2014 Edward O'Callaghan eocallaghan@alterapraxis.com + * Copyright (C) 2015 Matt DeVillier matt.devillier@gmail.com + * Copyright (C) 2016 Omar Pakker omarpakker+coreboot@gmail.com + * Copyright (C) 2019 Michael Niewöhner foss@mniewoehner.de + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <device/device.h> +#include <device/pnp.h> +#include <pc80/keyboard.h> +#include <stdlib.h> +#include <superio/conf_mode.h> + +#include "nct668Xd.h" + + +static void nct668Xd_init(struct device *dev) +{ + if (!dev->enabled) + return; + + switch (dev->path.pnp.device) { + case NCT668XD_KBC: + pc_keyboard_init(NO_AUX_DEVICE); + break; + } +} + +static struct device_operations ops = { + .read_resources = pnp_read_resources, + .set_resources = pnp_set_resources, + .enable_resources = pnp_enable_resources, + .enable = pnp_alt_enable, + .init = nct668Xd_init, + .ops_pnp_mode = &pnp_conf_mode_8787_aa, +}; + +static struct pnp_info pnp_dev_info[] = { + { NULL, NCT668XD_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, + 0x0ff8, }, + { NULL, NCT668XD_SP1, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT668XD_SP2, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT668XD_KBC, PNP_IO0 | PNP_IO1 | PNP_IRQ0 | PNP_IRQ1, + 0x0fff, 0x0fff, }, + { NULL, NCT668XD_CIR, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT668XD_PORT80}, + { NULL, NCT668XD_ACPI}, + { NULL, NCT668XD_EC, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT668XD_DSLP_PWRFAULT}, + { NULL, NCT668XD_FAN_ASSIGN}, + { NULL, NCT668XD_WDT1}, + { NULL, NCT668XD_WDTMEM}, + { NULL, NCT668XD_GPIO0}, + { NULL, NCT668XD_GPIO1}, + { NULL, NCT668XD_GPIO2}, + { NULL, NCT668XD_GPIO3}, + { NULL, NCT668XD_GPIO4}, + { NULL, NCT668XD_GPIO5}, + { NULL, NCT668XD_GPIO6}, + { NULL, NCT668XD_GPIO7}, + { NULL, NCT668XD_GPIO8}, + { NULL, NCT668XD_GPIO9}, + { NULL, NCT668XD_DS5}, + { NULL, NCT668XD_DS3}, + { NULL, NCT668XD_PCHDSW}, + { NULL, NCT668XD_DSWWOPT}, + { NULL, NCT668XD_DS3OPT}, + { NULL, NCT668XD_DSDSS}, + { NULL, NCT668XD_DSPU}, +}; + +static void enable_dev(struct device *dev) +{ + pnp_enable_devices(dev, &ops, ARRAY_SIZE(pnp_dev_info), pnp_dev_info); +} + +struct chip_operations superio_nuvoton_nct668Xd_ops = { + CHIP_NAME("NUVOTON NCT668XD Super I/O") + .enable_dev = enable_dev, +};
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#2).
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 4 files changed, 218 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 2:
(2 comments)
haven't fully reviewed the patch yet
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/Kconfig:
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... PS2, Line 21: config SUPERIO_NUVOTON_NCT668XD_COM_A : bool : depends on SUPERIO_NUVOTON_NCT668XD : default n the corresponding part in the nuvoton common code isn't in this patch
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... PS2, Line 46: pnp_enable_resources this conflicts with the second warning from the datasheet you mentioned in a comment in the header file. This function calls pnp_set_enable which sets or clears the corresponding bit in register 0x30 of the corresponding LDN
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... PS2, Line 46: pnp_enable_resources
this conflicts with the second warning from the datasheet you mentioned in a comment in the header f […]
DEVICE_NOOP might be what you're looking for here
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... PS2, Line 46: pnp_enable_resources
DEVICE_NOOP might be what you're looking for here
This only applies to LDN 7 and 9 (GPIO). I clarified that in the comment
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#3).
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 4 files changed, 218 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/3
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#4).
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/superio/nuvoton/common/early_serial.c A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 5 files changed, 244 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/4
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/Kconfig:
https://review.coreboot.org/c/coreboot/+/34948/2/src/superio/nuvoton/nct668x... PS2, Line 21: config SUPERIO_NUVOTON_NCT668XD_COM_A : bool : depends on SUPERIO_NUVOTON_NCT668XD : default n
the corresponding part in the nuvoton common code isn't in this patch
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 4: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... PS4, Line 76: : : don't remove the support for the nct6791 here
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... PS4, Line 90: tmp u8 tmp
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/nct668x... PS4, Line 71: NULL you can pass another device_operations struct here that has DEVICE_NOOP for .enable_resources and will only affect the (virtual) LDNs where it gets passed as override. same for the other virtual LDNs on LDN 7 and 9
Felix Held has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Removed Code-Review+2 by Felix Held felix-coreboot@felixheld.de
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 4: Code-Review-1
hit the wrong button; this patch currently doesn't compile due to one change in early_serial.c
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 4:
(2 comments)
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... PS4, Line 76: : :
don't remove the support for the nct6791 here
ouch.... it was late.....
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/common/... PS4, Line 90: tmp
u8 tmp
Done
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#5).
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/superio/nuvoton/common/early_serial.c A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 5 files changed, 245 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/5
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/5/src/superio/nuvoton/common/... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/c/coreboot/+/34948/5/src/superio/nuvoton/common/... PS5, Line 94: ( is a "~" missing here?
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#6).
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
superio: nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/superio/nuvoton/common/early_serial.c A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 5 files changed, 245 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/6
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/5/src/superio/nuvoton/common/... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/c/coreboot/+/34948/5/src/superio/nuvoton/common/... PS5, Line 94: (
is a "~" missing here?
should be a |. Thanks
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio: nuvoton: add a driver for nct668xd ......................................................................
Patch Set 6: Code-Review+1
Apart from the comment on patchset 4 on the enable function override for the GPIO LDNs this looks good to me. If you're not sure what I meant there, just say so and I'll try to explain it better :) I'm not sure if this might cause some issues, but you mentioned in the patch that changing the bits in register 0x30 of the GPIO LDNs might interfere with the embedded microcontroller in the SIO.
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#7).
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
superio/nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/superio/nuvoton/common/early_serial.c A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 5 files changed, 248 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/7
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 7:
Patch Set 6: Code-Review+1
Apart from the comment on patchset 4 on the enable function override for the GPIO LDNs this looks good to me. If you're not sure what I meant there, just say so and I'll try to explain it better :) I'm not sure if this might cause some issues, but you mentioned in the patch that changing the bits in register 0x30 of the GPIO LDNs might interfere with the embedded microcontroller in the SIO.
Let's see if I got this right - could you check this again, please? :-)
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 7:
(1 comment)
found a regression I introduced in a patch a year ago while reviewing this patch
https://review.coreboot.org/c/coreboot/+/34948/7/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/7/src/superio/nuvoton/nct668x... PS7, Line 74: ops_noenable &ops_noenable
Hello Felix Held, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34948
to look at the new patch set (#8).
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
superio/nuvoton: add a driver for nct668xd
Change-Id: I78eca4ba2948c36a386306887f62773580f6e444 Signed-off-by: Michael Niewöhner foss@mniewoehner.de --- M src/superio/nuvoton/common/early_serial.c A src/superio/nuvoton/nct668xd/Kconfig A src/superio/nuvoton/nct668xd/Makefile.inc A src/superio/nuvoton/nct668xd/nct668Xd.h A src/superio/nuvoton/nct668xd/superio.c 5 files changed, 248 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/34948/8
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/4/src/superio/nuvoton/nct668x... PS4, Line 71: NULL
you can pass another device_operations struct here that has DEVICE_NOOP for . […]
Done
https://review.coreboot.org/c/coreboot/+/34948/7/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/7/src/superio/nuvoton/nct668x... PS7, Line 74: ops_noenable
&ops_noenable
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 8: Code-Review+1
looks good to me; will wait with the +2 though until there's a patch adding a board that uses this SIO support, so that this code gets build-tested. SIO code (well, driver code in general) only gets build-tested when there's some board selecting the driver
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 8:
Patch Set 8: Code-Review+1
looks good to me; will wait with the +2 though until there's a patch adding a board that uses this SIO support, so that this code gets build-tested. SIO code (well, driver code in general) only gets build-tested when there's some board selecting the driver
I had a board that used it (see 34947) but unfortunately the board is dead so I can't continue here. Maybe someone with a Lenovo TS P320 or TC M800/M900 (same board) wants to finish this.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 8:
I had a board that used it (see 34947) but unfortunately the board is dead so I can't continue here. Maybe someone with a Lenovo TS P320 or TC M800/M900 (same board) wants to finish this.
Oh :(
There are probably some more boards using those SIO chips. IIRC one of the boards I had on my desk had some Nuvoton SIO with an external SPI flash; probably for firmware. I can have a look tomorrow and if I still have it, it has one of those SIOs and I find some cardboard box it fits into, I could send it to you if you want to port it. If I don't reply within the next 4 days to this, please poke me again; will be quite busy this week...
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 8:
IIRC one of the boards I had on my desk had some Nuvoton SIO with an external SPI flash; probably for firmware. I can have a look tomorrow and if I still have it, it has one of those SIOs and I find some cardboard box it fits into, I could send it to you if you want to port it.
Had a look and didn't find that board any more, so I guess it was the broken ivy bridge board I threw away a few months ago.
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 8:
Patch Set 8:
IIRC one of the boards I had on my desk had some Nuvoton SIO with an external SPI flash; probably for firmware. I can have a look tomorrow and if I still have it, it has one of those SIOs and I find some cardboard box it fits into, I could send it to you if you want to port it.
Had a look and didn't find that board any more, so I guess it was the broken ivy bridge board I threw away a few months ago.
ooops.... I'm sorry, I didn't check this CR for a month... :S
Well, yes, there are at least Lenovo P320, M800/900 and there was another one but I forgot the model... There are some Intel boards, too.
Those newer Nuvoton eSIOs have a SPI flash with EC firmware on it, that can be customized by OEM.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 9:
Jenkins wants the SPDX-style license headers instead of the long license headers
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34948/9/src/superio/nuvoton/common/... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/c/coreboot/+/34948/9/src/superio/nuvoton/common/... PS9, Line 69: << Please add spaces around the operator.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34948/9/src/superio/nuvoton/nct668x... File src/superio/nuvoton/nct668xd/superio.c:
https://review.coreboot.org/c/coreboot/+/34948/9/src/superio/nuvoton/nct668x... PS9, Line 25: #include <stdlib.h> not used
https://review.coreboot.org/c/coreboot/+/34948/9/src/superio/nuvoton/nct668x... PS9, Line 53: DEVICE_NOOP https://review.coreboot.org/c/coreboot/+/40206 ?
Michael Niewöhner has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34948 )
Change subject: superio/nuvoton: add a driver for nct668xd ......................................................................
Abandoned
have no board with that SIO anymore