Re: [PATCH 1/2] i2c: i2c-nct6126: add support for NCT6126D Super I/O
From: Niedermayr, BENEDIKT
Date: Mon Sep 28 2026 - 07:06:40 EST
On 9/18/26 14:24, Ilpo Järvinen wrote:
> On Thu, 20 Aug 2026, Benedikt Niedermayr wrote:
>
>> This adds support for an i2c master driver for the NCT6126D Super I/O
>> chips SMBUs controller. The NCT6126D exposes an SMBus master controller
>> inside Logical Device B (the Hardware Monitor / SB-TSI block).
>>
>> The I/O base address is discovered dynamically and is expected to be
>> programmed by the BIOS into LD B CR62h (MSB) and CR63h (LSB).
>>
>> TODO: Explain handling of the CR30h register.
>>
>> The SMBus master controller shares the same CR30h register with the
>> Hardware Monitor (logical device B), which is currently used by the
>> nct6775 driver. The nct6775 driver force overrides the CR30h register
>> to 0x01, if not already set by the bios.
>> This driver in turn is more defensive and will not probe the chip if the
>> CR30h register is not set to 0x01.
>>
>> Signed-off-by: Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx>
>> ---
>> MAINTAINERS | 5 +
>> drivers/i2c/busses/Kconfig | 11 +
>> drivers/i2c/busses/Makefile | 1 +
>> drivers/i2c/busses/i2c-nct6126.c | 473 +++++++++++++++++++++++++++++++
>> 4 files changed, 490 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-nct6126.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 8014b9f8253e..760d7a088629 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19315,6 +19315,11 @@ F: drivers/nubus/
>> F: include/linux/nubus.h
>> F: include/uapi/linux/nubus.h
>>
>> +NUVOTON NCT6126D I2C/SMBUS DRIVER
>> +M: Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx>
>> +S: Maintained
>> +F: drivers/i2c/busses/i2c-nct6126.c
>> +
>> NUVOTON NCT6694 MFD DRIVER
>> M: Ming Yu <tmyu0@xxxxxxxxxxx>
>> S: Supported
>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
>> index d35456994280..9dd265abdfbc 100644
>> --- a/drivers/i2c/busses/Kconfig
>> +++ b/drivers/i2c/busses/Kconfig
>> @@ -258,6 +258,17 @@ config I2C_NFORCE2
>> This driver can also be built as a module. If so, the module
>> will be called i2c-nforce2.
>>
>> +config I2C_NCT6126
>> + tristate "Nuvoton NCT6126D SMBus master"
>> + depends on X86 && HAS_IOPORT
>> + help
>> + If you say yes to this option, support will be included for the
>> + SMBus master controller embedded in the Nuvoton NCT6126D Super-I/O
>> + chip (Logical Device B).
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called i2c-nct6126.
>> +
>> config I2C_NVIDIA_GPU
>> tristate "NVIDIA GPU I2C controller"
>> depends on PCI
>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
>> index 3755c54b3d82..69042afb1f04 100644
>> --- a/drivers/i2c/busses/Makefile
>> +++ b/drivers/i2c/busses/Makefile
>> @@ -20,6 +20,7 @@ obj-$(CONFIG_I2C_I801) += i2c-i801.o
>> obj-$(CONFIG_I2C_ISCH) += i2c-isch.o
>> obj-$(CONFIG_I2C_ISMT) += i2c-ismt.o
>> obj-$(CONFIG_I2C_NFORCE2) += i2c-nforce2.o
>> +obj-$(CONFIG_I2C_NCT6126) += i2c-nct6126.o
>> obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o
>> obj-$(CONFIG_I2C_PIIX4) += i2c-piix4.o
>> obj-$(CONFIG_I2C_SIS5595) += i2c-sis5595.o
>> diff --git a/drivers/i2c/busses/i2c-nct6126.c b/drivers/i2c/busses/i2c-nct6126.c
>> new file mode 100644
>> index 000000000000..ce71883744e8
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-nct6126.c
>> @@ -0,0 +1,473 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * i2c-nct6126 - i2c adapter driver for the Nuvoton NCT6126D Super-I/O chip.
>> + *
>> + * The NCT6126D exposes an SMBus master controller inside Logical Device B
>> + * (the Hardware Monitor / SB-TSI block). Its I/O base address is programmed
>> + * by BIOS into LD B CR62h (MSB) and CR63h (LSB).
>> + *
>> + * Inspired by nct6775-platform.c and gpio-f7188x.c.
>> + *
>> + * Copyright (c) Siemens AG, 2026
>> + *
>> + * Author: Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx>
>> + */
>> +
>> +#define DRVNAME "i2c-nct6126"
>> +#define pr_fmt(fmt) DRVNAME ": " fmt
>> +
>> +#include <linux/delay.h>
>> +#include <linux/i2c.h>
>> +#include <linux/init.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/string.h>
>> +
>> +/*
>> + * Super-I/O configuration space
>> + */
>> +#define SIO_LDSEL 0x07 /* Logical Device Select */
>> +#define SIO_DEVID 0x20 /* CR20/CR21: 16-bit chip ID */
>> +#define SIO_UNLOCK_KEY 0x87 /* Enter extended function mode */
>> +#define SIO_LOCK_KEY 0xAA /* Exit extended function mode */
>> +
>> +/* Accepted chip IDs */
>> +#define SIO_NCT6126D_A_ID 0xD283 /* NCT6126D, A version */
>> +#define SIO_NCT6126D_B_ID 0xD284 /* NCT6126D, B version */
>> +
>> +/* Logical Device B: Hardware Monitor + SB-TSI / SMBus master */
>> +#define SIO_LD_HM_SMBUS 0x0B
>> +#define SIO_LDB_ENABLE 0x30 /* bit0: block active (CR30) */
>> +#define SIO_LDB_SMBUS_BASE 0x62 /* CR62 = MSB, CR63 = LSB */
>> +
>> +/* SMBus base address constraints */
>> +#define SMBUS_BASE_MIN 0x100
>> +#define SMBUS_BASE_MAX 0xFFE
>> +
>> +/*
>> + * SMBus controller register offsets (base + offset)
>> + * All registers are accessed directly at smbus_base + offset.
>> + */
>> +#define NCT6126D_SMWRSIZE 0x01
>> +#define NCT6126D_SMCMD 0x02
>> +#define NCT6126D_SMIDX 0x03
>> +#define NCT6126D_SMCTL 0x04
>> +#define NCT6126D_SMADDR 0x05
>> +#define NCT6126D_ERROR_STS 0x09
>> +#define NCT6126D_SMCTL3 0x0e
>> +#define NCT6126D_SMBUS_REGION_SIZE 0x20
>
> SZ_xx + include for it?
Fixed, see v2.
>
>> +
>> +/* Bit definitions */
>> +#define NCT6126D_FIFO_NULL BIT(0)
>> +#define NCT6126D_FIFO_FULL BIT(1)
>> +#define NCT6126D_ERR_NACK BIT(1)
>> +#define NCT6126D_ERR_BER BIT(2)
>> +#define NCT6126D_ERR_TIMEOUT BIT(4)
>> +#define NCT6126D_ERR_ADNACK BIT(5)
>> +#define NCT6126D_RST BIT(6)
>> +#define NCT6126D_MANUAL_MODE BIT(7)
>> +#define NCT6126D_ACTIVE_MANUAL_MODE BIT(2)
>
> Add include for BIT()
>
Fixed, see v2.
>> +
>> +/* SMBus command codes */
>> +#define NCT6126_CMD_READ_BYTE 0
>> +#define NCT6126_CMD_WRITE_BYTE 8
>> +
>> +/* Timing / retry limits */
>> +#define SMBUS_RW_LOOP_MAX 5
>> +#define SMBUS_FIFO_WAIT_US 100
>> +#define SMBUS_FIFO_MAX_CLEAR_TIME 0xff
>> +
>> +/*
>> + * Super-I/O functions.
>> + */
>> +static inline int nct_superio_inb(int base, int reg)
>> +{
>> + outb(reg, base);
>> + return inb(base + 1);
>> +}
>> +
>> +static int nct_superio_inw(int base, int reg)
>> +{
>> + int val;
>> +
>> + outb(reg++, base);
>> + val = inb(base + 1) << 8;
>> + outb(reg, base);
>> + val |= inb(base + 1);
>> +
>> + return val;
>> +}
>> +
>> +static inline int nct_superio_enter(int base)
>> +{
>> + if (!request_muxed_region(base, 2, DRVNAME)) {
>> + pr_err("SIO config port %#x already in use\n", base);
>> + return -EBUSY;
>> + }
>> +
>> + /* Datasheet 7.1.1: key must be written twice */
>> + outb(SIO_UNLOCK_KEY, base);
>> + outb(SIO_UNLOCK_KEY, base);
>> +
>> + return 0;
>> +}
>> +
>> +static inline void nct_superio_select(int base, int ld)
>> +{
>> + outb(SIO_LDSEL, base);
>> + outb(ld, base + 1);
>> +}
>> +
>> +static inline void nct_superio_exit(int base)
>> +{
>> + outb(SIO_LOCK_KEY, base);
>> + release_region(base, 2);
>> +}
>> +
>> +struct nct6126_sio {
>> + int addr; /* SIO config port: 0x2e or 0x4e */
>> + unsigned long smbus_base; /* from LD B CR62/CR63 */
>> +};
>> +
>> +struct nct6126_smbus {
>> + unsigned long port_addr;
>> + struct i2c_adapter adap;
>
> Inconsistent spacing.
>
Fixed, see v2.
I adapted other areas in the code as well.
>> +};
>> +
>> +static int __init nct6126_find(int addr, struct nct6126_sio *sio)
>> +{
>> + int err;
>> + u16 devid;
>> + u8 enable, msb, lsb;
>> + unsigned long base;
>
> I think I see what you're trying to do here with that extra space, but it
> just lead to inconsistent spacing. Better approach would be to just use
> reverse-xmas tree order without extra spaces.
It was worth a try :-)
Fixed, see v2.
>
>> + err = nct_superio_enter(addr);
>
> So your init func calls this function and then you immediately call
> nct_superio_enter() that performs writes to a random port? On any
> platform? I think that's a non-starter.
>
>> + if (err)
>> + return err;
>> +
>> + /* Verify chip identity */
>> + devid = nct_superio_inw(addr, SIO_DEVID);
>> + if (devid != SIO_NCT6126D_A_ID && devid != SIO_NCT6126D_B_ID) {
>> + err = -ENODEV;
>> + goto out;
>> + }
>> +
>> + /* Select Logical Device B (HM + SB-TSI/SMBus master) */
>> + nct_superio_select(addr, SIO_LD_HM_SMBUS);
>> +
>> + /*
>> + * Check CR30 bit 0: if clear, the block is not decoding its I/O
>> + * range on the LPC bus. Every inb() at the SMBus window would
>> + * return 0xFF. We do NOT write this bit: nct6775 owns CR30 and
>> + * maintains it across suspend/resume.
>> + */
>> + enable = nct_superio_inb(addr, SIO_LDB_ENABLE);
>> + if (!(enable & BIT(0))) {
>
> Please name BIT(0) with a define.
Fixed, see v2.
>
>> + pr_info("LD B inactive (CR30 bit0=0): BIOS has not enabled the HM/SMBus block\n");
>> + err = -ENODEV;
>> + goto out;
>> + }
>> +
>> + /*
>> + * Read the SMBus master base address programmed by BIOS.
>> + * Datasheet 19.7.1, 23.11: LD B CR62h (MSB) / CR63h (LSB).
>> + * BIOS must program this.
>> + */
>> + msb = nct_superio_inb(addr, SIO_LDB_SMBUS_BASE);
>> + lsb = nct_superio_inb(addr, SIO_LDB_SMBUS_BASE + 1);
>> + base = ((unsigned long)msb << 8) | lsb;
>> +
>> + if (base < SMBUS_BASE_MIN || base > SMBUS_BASE_MAX || (base & 1)) {
>
> !IS_ALIGNED() ? Don't forget to add include for it.
>
>> + pr_err("invalid SMBus base %#lx in LD B CR62/63\n", base);
>
> Add include.
Hopefully fixed now correctly by using IS_ALIGNED(base, 2).
See v2.
>
>> + err = -ENXIO;
>> + goto out;
>> + }
>> +
>> + sio->addr = addr;
>> + sio->smbus_base = base;
>> + err = 0;
>> +
>> + pr_info("Found nct6126d at %#x, SMBus base %#lx\n", addr, base);
>
> Success path should be quiet.
Fixed, see v2.
>
>> +
>> +out:
>> + nct_superio_exit(addr);
>> + return err;
>> +}
>> +
>> +/*
>> + * SMBus
>> + */
>> +static int nct_smbus_err_check(u8 err_code)
>> +{
>> + if (err_code & NCT6126D_ERR_ADNACK)
>> + return -ENXIO;
>> + if (err_code & NCT6126D_ERR_TIMEOUT)
>> + return -ETIMEDOUT;
>> + if (err_code & (NCT6126D_ERR_BER | NCT6126D_ERR_NACK))
>> + return -EXDEV;
>> + return 0;
>> +}
>> +
>> +static void nct6126_smbus_init_config(struct nct6126_smbus *priv,
>> + u8 slave_addr, u8 slave_reg)
>> +{
>> + u8 val;
>> +
>> + /* Reset SMBus controller */
>> + val = inb(priv->port_addr + NCT6126D_SMCTL);
>> + val |= NCT6126D_RST;
>> + outb(val, priv->port_addr + NCT6126D_SMCTL);
>> + outb(0, priv->port_addr + NCT6126D_SMCTL);
>> +
>> + /* 7-bit slave address; hardware expects it left-shifted by 1 */
>> + outb(slave_addr << 1, priv->port_addr + NCT6126D_SMADDR);
>> + outb(slave_reg, priv->port_addr + NCT6126D_SMIDX);
>> +}
>> +
>> +static int nct6126_smbus_enable_manual_mode(struct nct6126_smbus *priv)
>> +{
>> + u8 val;
>> +
>> + val = inb(priv->port_addr + NCT6126D_SMCTL);
>> + val |= NCT6126D_MANUAL_MODE;
>> + outb(val, priv->port_addr + NCT6126D_SMCTL);
>> +
>> + val = inb(priv->port_addr + NCT6126D_SMCTL3);
>> + val |= NCT6126D_ACTIVE_MANUAL_MODE;
>> + outb(val, priv->port_addr + NCT6126D_SMCTL3);
>> +
>> + /*
>> + * Wait ~200 us for the slave to respond, then read the error
>> + * status. Per Nuvoton FAE: error must be checked after manual
>> + * mode is activated due to the one-shot enable behaviour.
>> + */
>> + usleep_range(200, 300);
>> + val = inb(priv->port_addr + NCT6126D_ERROR_STS);
>> +
>> + return nct_smbus_err_check(val);
>> +}
>> +
>> +static int nct6126_smbus_write8(struct nct6126_smbus *priv,
>> + u8 slave_addr, u8 slave_reg, u8 *data)
>> +{
>> + int loop_cnt;
>> + int err;
>> +
>> + if (!request_muxed_region(priv->port_addr,
>> + NCT6126D_SMBUS_REGION_SIZE, DRVNAME))
>> + return -EBUSY;
>> +
>> + nct6126_smbus_init_config(priv, slave_addr, slave_reg);
>> +
>> + outb(1, priv->port_addr + NCT6126D_SMWRSIZE);
>> + outb(NCT6126_CMD_WRITE_BYTE, priv->port_addr + NCT6126D_SMCMD);
>> +
>> + loop_cnt = SMBUS_RW_LOOP_MAX;
>> + while ((inb(priv->port_addr + NCT6126D_SMCTL3) & NCT6126D_FIFO_FULL) &&
>> + --loop_cnt)
>> + usleep_range(SMBUS_FIFO_WAIT_US, SMBUS_FIFO_WAIT_US * 2);
>
> linux/iopoll.h would have something for you.
Thanks, for this hint!
Fixed, see v2.
>
>> +
>> + if (loop_cnt == 0) {
>> + err = -ETIMEDOUT;
>> + goto out;
>> + }
>> +
>> + outb(*data, priv->port_addr);
>> + err = nct6126_smbus_enable_manual_mode(priv);
>> +
>> +out:
>> + release_region(priv->port_addr, NCT6126D_SMBUS_REGION_SIZE);
>> + return err;
>> +}
>> +
>> +static int nct6126_smbus_read8(struct nct6126_smbus *priv,
>> + u8 slave_addr, u8 slave_reg, u8 *data)
>> +{
>> + int loop_cnt;
>> + int err;
>> +
>> + if (!request_muxed_region(priv->port_addr,
>> + NCT6126D_SMBUS_REGION_SIZE, DRVNAME))
>> + return -EBUSY;
>> +
>> + nct6126_smbus_init_config(priv, slave_addr, slave_reg);
>> +
>> + outb(0, priv->port_addr + NCT6126D_SMWRSIZE);
>> + outb(NCT6126_CMD_READ_BYTE, priv->port_addr + NCT6126D_SMCMD);
>> +
>> + err = nct6126_smbus_enable_manual_mode(priv);
>> + if (err)
>> + goto out;
>> +
>> + loop_cnt = SMBUS_RW_LOOP_MAX;
>> + while ((inb(priv->port_addr + NCT6126D_SMCTL3) & NCT6126D_FIFO_NULL) &&
>> + --loop_cnt)
>> + usleep_range(SMBUS_FIFO_WAIT_US, SMBUS_FIFO_WAIT_US * 2);
>
> linux/iopoll.h can do this as well.
Fixed, see v2.
>
>> + if (loop_cnt == 0) {
>> + err = -ETIMEDOUT;
>> + goto out;
>> + }
>> +
>> + *data = inb(priv->port_addr);
>> +
>> + /* Drain any residual FIFO entries */
>> + loop_cnt = SMBUS_FIFO_MAX_CLEAR_TIME;
>> + while (!(inb(priv->port_addr + NCT6126D_SMCTL3) & NCT6126D_FIFO_NULL) &&
>> + --loop_cnt)
>> + inb(priv->port_addr);
>> +
>> + err = (loop_cnt == 0) ? -ETIMEDOUT : 0;
>> +
>> +out:
>> + release_region(priv->port_addr, NCT6126D_SMBUS_REGION_SIZE);
>> + return err;
>> +}
>> +
>> +/*
>> + * i2c_algorithm
>> + */
>> +static int nct6126_smbus_xfer(struct i2c_adapter *adap, u16 addr, u16 flags,
>> + char read_write, u8 cmd, int size,
>> + union i2c_smbus_data *data)
>> +{
>> + struct nct6126_smbus *priv = i2c_get_adapdata(adap);
>> +
>> + if (!priv)
>> + return -ENODEV;
>> +
>> + if (read_write == I2C_SMBUS_READ)
>> + return nct6126_smbus_read8(priv, addr, cmd, &data->byte);
>> + else
>> + return nct6126_smbus_write8(priv, addr, cmd, &data->byte);
>> +}
>> +
>> +static u32 nct6126_functionality(struct i2c_adapter *adap)
>> +{
>> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_BYTE | I2C_FUNC_SMBUS_BYTE_DATA;
>> +}
>> +
>> +static const struct i2c_algorithm nct6126_algo = {
>> + .smbus_xfer = nct6126_smbus_xfer,
>> + .functionality = nct6126_functionality,
>> +};
>> +
>> +static int nct6126_smbus_probe(struct platform_device *pdev)
>> +{
>> + struct nct6126_sio *sio = dev_get_platdata(&pdev->dev);
>> + struct nct6126_smbus *priv;
>> + struct i2c_adapter *adap;
>> + int err;
>> +
>> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
>> + if (!priv)
>> + return -ENOMEM;
>> +
>> + priv->port_addr = sio->smbus_base;
>> +
>> + adap = &priv->adap;
>> + adap->owner = THIS_MODULE;
>> + adap->class = I2C_CLASS_HWMON;
>> + adap->algo = &nct6126_algo;
>> + adap->dev.parent = &pdev->dev;
>> + adap->nr = -1;
>> + strscpy(adap->name, DRVNAME, sizeof(adap->name));
>> + i2c_set_adapdata(adap, priv);
>> +
>> + err = i2c_add_adapter(adap);
>> + if (err) {
>> + dev_err(&pdev->dev, "failed to add i2c adapter: %d\n", err);
>> + return err;
>> + }
>> +
>> + platform_set_drvdata(pdev, priv);
>> + return 0;
>> +}
>> +
>> +static void nct6126_smbus_remove(struct platform_device *pdev)
>> +{
>> + struct nct6126_smbus *priv = platform_get_drvdata(pdev);
>> +
>> + i2c_del_adapter(&priv->adap);
>> +}
>> +
>> +static struct platform_driver nct6126_smbus_driver = {
>> + .driver = {
>> + .name = DRVNAME,
>> + },
>> + .probe = nct6126_smbus_probe,
>> + .remove = nct6126_smbus_remove,
>> +};
>> +
>> +#define MAX_PDEVS 1
>> +static struct platform_device *nct6126_pdevs[MAX_PDEVS];
>> +static int nct6126_pdevs_cnt;
>
> Why all this complexity to store one pointer? Why isn't NULL/not
NULL> check adequate for this purpose?
Yes, makes absolutely sense. Fixed, see v2.
>> +
>> +static int __init nct6126_device_add(const struct nct6126_sio *sio)
>> +{
>> + struct platform_device *pdev;
>> + int err;
>> +
>> + pdev = platform_device_alloc(DRVNAME, 0);
>> + if (!pdev)
>> + return -ENOMEM;
>> +
>> + err = platform_device_add_data(pdev, sio, sizeof(*sio));
>> + if (err) {
>> + pr_err("platform data allocation failed\n");
>> + goto err_put;
>> + }
>> +
>> + err = platform_device_add(pdev);
>> + if (err) {
>> + pr_err("platform device registration failed\n");
>> + goto err_put;
>> + }
>> +
>> + nct6126_pdevs[nct6126_pdevs_cnt++] = pdev;
>> + return 0;
>> +
>> +err_put:
>> + platform_device_put(pdev);
>> + return err;
>> +}
>> +
>> +static int __init nct6126_smbus_init(void)
>> +{
>> + struct nct6126_sio sio;
>> + int err;
>> +
>> + if (nct6126_find(0x2e, &sio) && nct6126_find(0x4e, &sio))
>> + return -ENODEV;
>> +
>> + err = platform_driver_register(&nct6126_smbus_driver);
>> + if (err)
>> + return err;
>> +
>> + err = nct6126_device_add(&sio);
>> + if (err)
>> + goto err_unreg_driver;
>> +
>> + return 0;
>> +
>> +err_unreg_driver:
>> + platform_driver_unregister(&nct6126_smbus_driver);
>> + return err;
>> +}
>> +
>> +static void __exit nct6126_smbus_exit(void)
>> +{
>> + while (nct6126_pdevs_cnt > 0)
>> + platform_device_unregister(nct6126_pdevs[--nct6126_pdevs_cnt]);
>> +
>> + platform_driver_unregister(&nct6126_smbus_driver);
>> +}
>> +
>> +module_init(nct6126_smbus_init);
>> +module_exit(nct6126_smbus_exit);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_AUTHOR("Benedikt Niedermayr <benedikt.niedermayr@xxxxxxxxxxx>");
>> +MODULE_DESCRIPTION("SMBus master driver for Nuvoton NCT6126D Super-I/O");
>>
>
> --
> i.
>