happycorsair@yandex.ru has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
superio/nuvoton/nct6793d: Add initial support
Add initial support for Nuvoton NCT6793D superio chip. Right now it's just a proper renamed copy of NCT6791D code. It's used in ASUS PRIME H310* motherboards (trying to make them run coreboot).
Signed-off-by: happycorsair happycorsair@yandex.ru Change-Id: I5484c4a30f60c77004bc0787ba584f50cd173f3c --- M src/superio/nuvoton/Makefile.inc A src/superio/nuvoton/nct6793d/Kconfig A src/superio/nuvoton/nct6793d/Makefile.inc A src/superio/nuvoton/nct6793d/nct6793d.h A src/superio/nuvoton/nct6793d/superio.c 5 files changed, 162 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/48790/1
diff --git a/src/superio/nuvoton/Makefile.inc b/src/superio/nuvoton/Makefile.inc index e9ac2e3..aae29ef 100644 --- a/src/superio/nuvoton/Makefile.inc +++ b/src/superio/nuvoton/Makefile.inc @@ -11,4 +11,5 @@ subdirs-$(CONFIG_SUPERIO_NUVOTON_NCT6776) += nct6776 subdirs-$(CONFIG_SUPERIO_NUVOTON_NCT6779D) += nct6779d subdirs-$(CONFIG_SUPERIO_NUVOTON_NCT6791D) += nct6791d +subdirs-$(CONFIG_SUPERIO_NUVOTON_NCT6793D) += nct6793d subdirs-$(CONFIG_SUPERIO_NUVOTON_NPCD378) += npcd378 diff --git a/src/superio/nuvoton/nct6793d/Kconfig b/src/superio/nuvoton/nct6793d/Kconfig new file mode 100644 index 0000000..245396d --- /dev/null +++ b/src/superio/nuvoton/nct6793d/Kconfig @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0-only + +config SUPERIO_NUVOTON_NCT6793D + bool + select SUPERIO_NUVOTON_COMMON_PRE_RAM diff --git a/src/superio/nuvoton/nct6793d/Makefile.inc b/src/superio/nuvoton/nct6793d/Makefile.inc new file mode 100644 index 0000000..a69daad --- /dev/null +++ b/src/superio/nuvoton/nct6793d/Makefile.inc @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0-only + +ramstage-$(CONFIG_SUPERIO_NUVOTON_NCT6793D) += superio.c +ramstage-$(CONFIG_SUPERIO_NUVOTON_NCT6793D) += ../../common/ssdt.c +ramstage-$(CONFIG_SUPERIO_NUVOTON_NCT6793D) += ../../common/generic.c diff --git a/src/superio/nuvoton/nct6793d/nct6793d.h b/src/superio/nuvoton/nct6793d/nct6793d.h new file mode 100644 index 0000000..a56d41d --- /dev/null +++ b/src/superio/nuvoton/nct6793d/nct6793d.h @@ -0,0 +1,44 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef SUPERIO_NUVOTON_NCT6793D_H +#define SUPERIO_NUVOTON_NCT6793D_H + +/* Logical Device Numbers (LDN). */ +#define NCT6793D_PP 0x01 /* Parallel port */ +#define NCT6793D_SP1 0x02 /* UART A */ +#define NCT6793D_SP2 0x03 /* UART B, IR */ +#define NCT6793D_KBC 0x05 /* Keyboard Controller */ +#define NCT6793D_CIR 0x06 /* Consumer IR */ +#define NCT6793D_GPIO678 0x07 /* GPIO 6, 7 & 8 */ +#define NCT6793D_WDT1_WDTMEM_GPIO01 0x08 /* WDT1, WDT_MEM, GPIO 0 & 1 */ +#define NCT6793D_GPIO2345 0x09 /* GPIO 2, 3, 4 & 5 */ +#define NCT6793D_ACPI 0x0A /* ACPI */ +#define NCT6793D_HWM_FPLED 0x0B /* HW Monitor, Front Panel LED */ +#define NCT6793D_BCLK_WDT2_WDTMEM 0x0D /* BCLK, WDT2, WDT_MEM */ +#define NCT6793D_CIRWUP 0x0E /* CIR Wake-Up */ +#define NCT6793D_GPIO_PP_OD 0x0F /* GPIO Push-Pull/Open-Drain */ +#define NCT6793D_PORT80 0x14 /* Port 80 UART */ +#define NCT6793D_DS 0x16 /* Deep Sleep */ + +/* Virtual LDNs */ +#define NCT6793D_WDT1 ((0 << 8) | NCT6793D_WDT1_WDTMEM_GPIO01) +#define NCT6793D_WDTMEM ((4 << 8) | NCT6793D_WDT1_WDTMEM_GPIO01) +#define NCT6793D_GPIOBASE ((3 << 8) | NCT6793D_WDT1_WDTMEM_GPIO01) +#define NCT6793D_GPIO0 ((1 << 8) | NCT6793D_WDT1_WDTMEM_GPIO01) +#define NCT6793D_GPIO1 ((7 << 8) | NCT6793D_WDT1_WDTMEM_GPIO01) +#define NCT6793D_GPIO2 ((0 << 8) | NCT6793D_GPIO2345) +#define NCT6793D_GPIO3 ((1 << 8) | NCT6793D_GPIO2345) +#define NCT6793D_GPIO4 ((2 << 8) | NCT6793D_GPIO2345) +#define NCT6793D_GPIO5 ((3 << 8) | NCT6793D_GPIO2345) +#define NCT6793D_GPIO6 ((0 << 8) | NCT6793D_GPIO678) +#define NCT6793D_GPIO7 ((1 << 8) | NCT6793D_GPIO678) +#define NCT6793D_GPIO8 ((2 << 8) | NCT6793D_GPIO678) +#define NCT6793D_DS5 ((0 << 8) | NCT6793D_DS) +#define NCT6793D_DS3 ((1 << 8) | NCT6793D_DS) +#define NCT6793D_PCHDSW ((3 << 8) | NCT6793D_DS) +#define NCT6793D_DSWWOPT ((4 << 8) | NCT6793D_DS) +#define NCT6793D_DS3OPT ((5 << 8) | NCT6793D_DS) +#define NCT6793D_DSDSS ((6 << 8) | NCT6793D_DS) +#define NCT6793D_DSPU ((7 << 8) | NCT6793D_DS) + +#endif /* SUPERIO_NUVOTON_NCT6793D_H */ diff --git a/src/superio/nuvoton/nct6793d/superio.c b/src/superio/nuvoton/nct6793d/superio.c new file mode 100644 index 0000000..c4359d5 --- /dev/null +++ b/src/superio/nuvoton/nct6793d/superio.c @@ -0,0 +1,107 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include <device/device.h> +#include <device/pnp.h> +#include <pc80/keyboard.h> +#include <superio/conf_mode.h> +#include <superio/common/ssdt.h> +#include <acpi/acpi.h> +#include "nct6793d.h" + +static void nct6793d_init(struct device *dev) +{ + if (!dev->enabled) + return; + + switch (dev->path.pnp.device) { + case NCT6793D_KBC: + pc_keyboard_init(NO_AUX_DEVICE); + break; + } +} + +#if CONFIG(HAVE_ACPI_TABLES) +/* Provide ACPI HIDs for generic Super I/O SSDT */ +static const char *nct6793d_acpi_hid(const struct device *dev) +{ + if ((dev->path.type != DEVICE_PATH_PNP) || + (dev->path.pnp.port == 0) || + ((dev->path.pnp.device & 0xff) > NCT6793D_DS)) + return NULL; + + switch (dev->path.pnp.device & 0xff) { + case NCT6793D_SP1: /* fallthrough */ + case NCT6793D_SP2: + return ACPI_HID_COM; + case NCT6793D_KBC: + return ACPI_HID_KEYBOARD; + default: + return ACPI_HID_PNP; + } +} +#endif + +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 = nct6793d_init, + .ops_pnp_mode = &pnp_conf_mode_8787_aa, +#if CONFIG(HAVE_ACPI_TABLES) + .acpi_fill_ssdt = superio_common_fill_ssdt_generator, + .acpi_name = superio_common_ldn_acpi_name, + .acpi_hid = nct6793d_acpi_hid, +#endif +}; + +static struct pnp_info pnp_dev_info[] = { + { NULL, NCT6793D_PP, PNP_IO0 | PNP_IRQ0 | PNP_DRQ0, + 0x0ff8, }, + { NULL, NCT6793D_SP1, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT6793D_SP2, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT6793D_KBC, PNP_IO0 | PNP_IO1 | PNP_IRQ0 | PNP_IRQ1, + 0x0fff, 0x0fff, }, + { NULL, NCT6793D_CIR, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT6793D_ACPI}, + { NULL, NCT6793D_HWM_FPLED, PNP_IO0 | PNP_IO1 | PNP_IRQ0, + 0x0ffe, 0x0ffe, }, + { NULL, NCT6793D_BCLK_WDT2_WDTMEM}, + { NULL, NCT6793D_CIRWUP, PNP_IO0 | PNP_IRQ0, + 0x0ff8, }, + { NULL, NCT6793D_GPIO_PP_OD}, + { NULL, NCT6793D_PORT80}, + { NULL, NCT6793D_WDT1}, + { NULL, NCT6793D_WDTMEM}, + { NULL, NCT6793D_GPIOBASE, PNP_IO0, + 0x0ff8, }, + { NULL, NCT6793D_GPIO0}, + { NULL, NCT6793D_GPIO1}, + { NULL, NCT6793D_GPIO2}, + { NULL, NCT6793D_GPIO3}, + { NULL, NCT6793D_GPIO4}, + { NULL, NCT6793D_GPIO5}, + { NULL, NCT6793D_GPIO6}, + { NULL, NCT6793D_GPIO7}, + { NULL, NCT6793D_GPIO8}, + { NULL, NCT6793D_DS5}, + { NULL, NCT6793D_DS3}, + { NULL, NCT6793D_PCHDSW}, + { NULL, NCT6793D_DSWWOPT}, + { NULL, NCT6793D_DS3OPT}, + { NULL, NCT6793D_DSDSS}, + { NULL, NCT6793D_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_nct6793d_ops = { + CHIP_NAME("NUVOTON NCT6793D Super I/O") + .enable_dev = enable_dev, +};
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@2 PS1, Line 2: happycorsair It’d be great if you used name and surname (even pseudonym).
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@11 PS1, Line 11: It's used in ASUS PRIME H310* motherboards (trying to make them run coreboot). Please document, if you have access to the datasheet (name and revision), and if you already know of differences between NCT6791D and NCT6793D, so a copy is warranted.
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@13 PS1, Line 13: happycorsair Ditto.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@11 PS1, Line 11: It's used in ASUS PRIME H310* motherboards (trying to make them run coreboot).
Please document, if you have access to the datasheet (name and revision), and if you already know of […]
At least the Linux kernel seems to treat them the same in `drivers/hwmon/nct6775.c`.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48790
to look at the new patch set (#2).
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
superio/nuvoton/nct6793d: Add initial support
Add initial support for Nuvoton NCT6793D superio chip. Right now it's just a proper renamed copy of NCT6791D code. It's used in ASUS PRIME H310* motherboards (trying to make them run coreboot).
Signed-off-by: Timofey Komarov happycorsair@yandex.ru Change-Id: I5484c4a30f60c77004bc0787ba584f50cd173f3c --- M src/superio/nuvoton/Makefile.inc A src/superio/nuvoton/nct6793d/Kconfig A src/superio/nuvoton/nct6793d/Makefile.inc A src/superio/nuvoton/nct6793d/nct6793d.h A src/superio/nuvoton/nct6793d/superio.c 5 files changed, 162 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/48790/2
happycorsair@yandex.ru has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@2 PS1, Line 2: happycorsair
It’d be great if you used name and surname (even pseudonym).
Done
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@13 PS1, Line 13: happycorsair
Ditto.
Done
happycorsair@yandex.ru has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48790/1//COMMIT_MSG@11 PS1, Line 11: It's used in ASUS PRIME H310* motherboards (trying to make them run coreboot).
At least the Linux kernel seems to treat them the same in `drivers/hwmon/nct6775.c`.
Hi, Paul! I have access to NCT6791D (Date: January 8th, 2016 Revision 1.11) and NCT6793D (Date: June 29th, 2015 Revision 1.0) datasheets. One can find them in Internet, it's not a big deal.
Differences I've found so far: 1) NCT6793 supports four aux fans, NCT6791 supports only three (chapter 8.7). 2) NCT6793 has more LDNs (chapter 24). 3*) NCT6793 provides power fault handling (chapter 21). 4*) NCT6793 has HWM to port 80 feature (chapter 23). 5) NCT6793 has fading led feature (chapter 22). 6) Various small register differences.
Those with * are important, as for me.
That Linux driver deals only with hwmon features.
I don't like copy-paste, I think it's possible to try to make some shared part for various Nuvoton chips. But for now it's not a big problem, I think.
Anyway, I believe, that Kconfig options for NCT6793 and NCT6791 should be separated. Thanks!
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48790 )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Patch Set 2: Code-Review+1
(6 comments)
There are a few differences in the datasheets (commented them inline in `nct6793d.h`). If you are sure that the NCT6793D datasheet is wrong, just mention that in the commit message. If not, better adapt the code.
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... File src/superio/nuvoton/nct6793d/nct6793d.h:
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 17: /* BCLK, WDT2, WDT_MEM */ This seems to have changed, unless the datasheet is incomplete.
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 38: #define NCT6793D_PCHDSW ((3 << 8) | NCT6793D_DS) Reserved in the DS?
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 41: #define NCT6793D_DSDSS ((6 << 8) | NCT6793D_DS) Reserved in the DS?
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... File src/superio/nuvoton/nct6793d/superio.c:
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 13: if (!dev->enabled) : return; : This function is only called for enabled devices.
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 48: pnp_alt_enable I wonder if it really needs the `alt` enable. Would be nice if you could compare superiotool dumps of cold boots with this set to `pnp_enable` and `pnp_alt_enable`.
https://review.coreboot.org/c/coreboot/+/48790/2/src/superio/nuvoton/nct6793... PS2, Line 60: 0x0ff8, }, The line breaks here seem unnecessary.
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48790?usp=email )
Change subject: superio/nuvoton/nct6793d: Add initial support ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-
Martin L Roth (Code Review) -
Name of user not set (Code Review)
-
Nico Huber (Code Review)
-
Paul Menzel (Code Review)