Re: [PATCH 1/2] i2c: i2c-nct6126: add support for NCT6126D Super I/O

["Niedermayr, BENEDIKT" <benedikt.niedermayr@siemens.com>]

On 9/18/26 14:24, Ilpo Järvinen wrote:
> [Sie erhalten nicht h?ufig E-Mails von ilpo.jarvinen@linux.intel.com. Weitere Informationen, warum dies wichtig ist, finden Sie unter https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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@siemens.com>
>> ---
>>   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@siemens.com>
>> +S:   Maintained
>> +F:   drivers/i2c/busses/i2c-nct6126.c
>> +
>>   NUVOTON NCT6694 MFD DRIVER
>>   M:   Ming Yu <tmyu0@nuvoton.com>
>>   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@siemens.com>
>> + */
>> +
>> +#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?
> 
>> +
>> +/* 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()
> 
>> +
>> +/* 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.
> 
>> +};
>> +
>> +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.
> 
>> +     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.
> 

First of all, thanks for the review!

All comments make sense to me, and I'll send a v2 fixing them (I also 
noticed that "TODO" in the commit message).

Regarding this specific point, I'm not entirely sure. I wasn't 
considering that during implementation because I was leaning heavily on 
the gpio-f7188x.c driver, which implements the GPIO part of this chip. 
It performs the probing/init very similarly. Since the driver must be 
loaded explicitly by the user or by other platform detection mechanisms 
(like the simatic-ipc.c detection driver in this case), I felt safe 
copying this logic.

I'm not sure if using DMI_MATCH tables would be the right choice here? 
While it would restrict write access to only whitelisted platforms, it 
wouldn't scale well as the number of supported platforms increases.

The request_muxed_region() call at least helps lock against other 
drivers already using this region. The critical scenario would be if no 
other driver has gained access to the region yet, but that would 
generally only happen if the user intentionally loaded the driver.

Using ACPI-based detection seems impossible on this platform (no proper 
ACPI table support for this chip). IIRC, these Super I/O chips generally 
aren't suitable for probing via ACPI if they need to be shared across 
different drivers, as ACPI detection would exclusively occupy the I/O 
port region [1].

How should I proceed here?

[1] 
https://lore.kernel.org/linux-watchdog/df8d53db-0056-434d-953b-991025e6cd34@roeck-us.net/


Cheers,
Benedikt

>> +     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.
> 
>> +             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.
> 
>> +             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.
> 
>> +
>> +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.
> 
>> +
>> +     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.
> 
>> +     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?
> 
>> +
>> +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@siemens.com>");
>> +MODULE_DESCRIPTION("SMBus master driver for Nuvoton NCT6126D Super-I/O");
>>
> 
> --
>   i.
>