| Message ID | 20220313224020.9005-1-calcprogrammer1@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: New driver for Nuvoton SMBus adapters | expand |
Hi Adam,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v5.17-rc8 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Adam-Honse/i2c-New-driver-for-Nuvoton-SMBus-adapters/20220314-064247
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: powerpc64-randconfig-r035-20220314 (https://download.01.org/0day-ci/archive/20220314/202203140908.OiQgTMB7-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/040a1be0d30ce2c611e30eb923d6f1b0afc44e7d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Adam-Honse/i2c-New-driver-for-Nuvoton-SMBus-adapters/20220314-064247
git checkout 040a1be0d30ce2c611e30eb923d6f1b0afc44e7d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/i2c/busses/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-nct6775.c: In function 'nct6775_access':
>> drivers/i2c/busses/i2c-nct6775.c:218:31: warning: this statement may fall through [-Wimplicit-fallthrough=]
218 | tmp_data.byte = data->byte;
| ~~~~~~~~~~~~~~^~~~~~~~~~~~
drivers/i2c/busses/i2c-nct6775.c:219:9: note: here
219 | case I2C_SMBUS_BYTE:
| ^~~~
At top level:
drivers/i2c/busses/i2c-nct6775.c:90:27: warning: 'nct6775_device_names' defined but not used [-Wunused-const-variable=]
90 | static const char * const nct6775_device_names[] = {
| ^~~~~~~~~~~~~~~~~~~~
vim +218 drivers/i2c/busses/i2c-nct6775.c
194
195 /* Return negative errno on error. */
196 static s32 nct6775_access(struct i2c_adapter *adap, u16 addr,
197 unsigned short flags, char read_write,
198 u8 command, int size, union i2c_smbus_data *data)
199 {
200 struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap);
201 unsigned short nuvoton_nct6793d_smba = adapdata->smba;
202 int i, len, cnt;
203 union i2c_smbus_data tmp_data;
204 int timeout = 0;
205
206 tmp_data.word = 0;
207 cnt = 0;
208 len = 0;
209
210 outb_p(NCT6793D_SOFT_RESET, SMBHSTCTL);
211
212 switch (size) {
213 case I2C_SMBUS_QUICK:
214 outb_p((addr << 1) | read_write,
215 SMBHSTADD);
216 break;
217 case I2C_SMBUS_BYTE_DATA:
> 218 tmp_data.byte = data->byte;
219 case I2C_SMBUS_BYTE:
220 outb_p((addr << 1) | read_write,
221 SMBHSTADD);
222 outb_p(command, SMBHSTIDX);
223 if (read_write == I2C_SMBUS_WRITE) {
224 outb_p(tmp_data.byte, SMBHSTDAT);
225 outb_p(NCT6793D_WRITE_BYTE, SMBHSTCMD);
226 } else {
227 outb_p(NCT6793D_READ_BYTE, SMBHSTCMD);
228 }
229 break;
230 case I2C_SMBUS_WORD_DATA:
231 outb_p((addr << 1) | read_write,
232 SMBHSTADD);
233 outb_p(command, SMBHSTIDX);
234 if (read_write == I2C_SMBUS_WRITE) {
235 outb_p(data->word & 0xff, SMBHSTDAT);
236 outb_p((data->word & 0xff00) >> 8, SMBHSTDAT);
237 outb_p(NCT6793D_WRITE_WORD, SMBHSTCMD);
238 } else {
239 outb_p(NCT6793D_READ_WORD, SMBHSTCMD);
240 }
241 break;
242 case I2C_SMBUS_BLOCK_DATA:
243 outb_p((addr << 1) | read_write,
244 SMBHSTADD);
245 outb_p(command, SMBHSTIDX);
246 if (read_write == I2C_SMBUS_WRITE) {
247 len = data->block[0];
248 if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
249 return -EINVAL;
250 outb_p(len, SMBBLKSZ);
251
252 cnt = 1;
253 if (len >= 4) {
254 for (i = cnt; i <= 4; i++)
255 outb_p(data->block[i], SMBHSTDAT);
256
257 len -= 4;
258 cnt += 4;
259 } else {
260 for (i = cnt; i <= len; i++)
261 outb_p(data->block[i], SMBHSTDAT);
262
263 len = 0;
264 }
265
266 outb_p(NCT6793D_WRITE_BLOCK, SMBHSTCMD);
267 } else {
268 return -EOPNOTSUPP;
269 }
270 break;
271 default:
272 dev_warn(&adap->dev, "Unsupported transaction %d\n", size);
273 return -EOPNOTSUPP;
274 }
275
276 outb_p(NCT6793D_MANUAL_START, SMBHSTCTL);
277
278 while ((size == I2C_SMBUS_BLOCK_DATA) && (len > 0)) {
279 if (read_write == I2C_SMBUS_WRITE) {
280 timeout = 0;
281 while ((inb_p(SMBHSTSTS) & NCT6793D_FIFO_EMPTY) == 0) {
282 if (timeout > MAX_RETRIES)
283 return -ETIMEDOUT;
284
285 usleep_range(250, 500);
286 timeout++;
287 }
288
289 //Load more bytes into FIFO
290 if (len >= 4) {
291 for (i = cnt; i <= (cnt + 4); i++)
292 outb_p(data->block[i], SMBHSTDAT);
293
294 len -= 4;
295 cnt += 4;
296 } else {
297 for (i = cnt; i <= (cnt + len); i++)
298 outb_p(data->block[i], SMBHSTDAT);
299
300 len = 0;
301 }
302 } else {
303 return -EOPNOTSUPP;
304 }
305
306 }
307
308 //wait for manual mode to complete
309 timeout = 0;
310 while ((inb_p(SMBHSTSTS) & NCT6793D_MANUAL_ACTIVE) != 0) {
311 if (timeout > MAX_RETRIES)
312 return -ETIMEDOUT;
313
314 usleep_range(250, 500);
315 timeout++;
316 }
317
318 if ((inb_p(SMBHSTERR) & NCT6793D_NO_ACK) != 0)
319 return -ENXIO;
320
321 else if ((read_write == I2C_SMBUS_WRITE) || (size == I2C_SMBUS_QUICK))
322 return 0;
323
324 switch (size) {
325 case I2C_SMBUS_QUICK:
326 case I2C_SMBUS_BYTE_DATA:
327 data->byte = inb_p(SMBHSTDAT);
328 break;
329 case I2C_SMBUS_WORD_DATA:
330 data->word = inb_p(SMBHSTDAT) + (inb_p(SMBHSTDAT) << 8);
331 break;
332 }
333 return 0;
334 }
335
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Adam, On Sun, 13 Mar 2022 17:40:20 -0500, Adam Honse wrote: > This patch introduces a new driver for the SMBus adapter that is built in > to most Nuvoton Super-IO chips. This SMBus adapter is used for RGB > lighting control on some ASUS motherboards with Intel chipsets. > > The interface's register description is available in the supported > devices' datasheets. Operation of this interface has been verified > with OpenRGB on an ASUS PRIME Z270-A motherboard. > > Signed-off-by: Adam Honse <calcprogrammer1@gmail.com> See my review inline below. > --- > drivers/i2c/busses/Kconfig | 9 + > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-nct6775.c | 608 +++++++++++++++++++++++++++++++ > 3 files changed, 618 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-nct6775.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index 8a6c6ee28556..cf4c5e92738b 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -219,6 +219,15 @@ config I2C_CHT_WC > combined with a FUSB302 Type-C port-controller as such it is advised > to also select CONFIG_TYPEC_FUSB302=m. > > +config I2C_NCT6775 > + tristate "Nuvoton NCT6775 and compatible SMBus controller" > + help > + If you say yes to this option, support will be included for the > + Nuvoton NCT6775 and compatible SMBus controllers. > + > + This driver can also be built as a module. If so, the module > + will be called i2c-nct6775. > + > config I2C_NFORCE2 > tristate "Nvidia nForce2, nForce3 and nForce4" > depends on PCI > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 1d00dce77098..450cde39c7d5 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -17,6 +17,7 @@ obj-$(CONFIG_I2C_CHT_WC) += i2c-cht-wc.o > obj-$(CONFIG_I2C_I801) += i2c-i801.o > obj-$(CONFIG_I2C_ISCH) += i2c-isch.o > obj-$(CONFIG_I2C_ISMT) += i2c-ismt.o > +obj-$(CONFIG_I2C_NCT6775) += i2c-nct6775.o > obj-$(CONFIG_I2C_NFORCE2) += i2c-nforce2.o > obj-$(CONFIG_I2C_NFORCE2_S4985) += i2c-nforce2-s4985.o > obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o > diff --git a/drivers/i2c/busses/i2c-nct6775.c b/drivers/i2c/busses/i2c-nct6775.c > new file mode 100644 > index 000000000000..fdffe30f595b > --- /dev/null > +++ b/drivers/i2c/busses/i2c-nct6775.c > @@ -0,0 +1,608 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * i2c-nct6775 - Driver for the SMBus master functionality of > + * Nuvoton NCT677x Super-I/O chips > + * > + * Copyright (C) 2019 Adam Honse <calcprogrammer1@gmail.com> > + * > + * Derived from nct6775 hwmon driver > + * Copyright (C) 2012 Guenter Roeck <linux@roeck-us.net> > + * > + * 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 <linux/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/jiffies.h> Not needed? > +#include <linux/platform_device.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/hwmon-vid.h> You don't need these 3 hwmon header files. > +#include <linux/err.h> > +#include <linux/mutex.h> Also not needed. > +#include <linux/delay.h> > +#include <linux/ioport.h> > +#include <linux/i2c.h> > +#include <linux/acpi.h> > +#include <linux/bitops.h> Not needed. > +#include <linux/dmi.h> Not needed. > +#include <linux/io.h> > +#include <linux/nospec.h> Why? > + > +#define DRVNAME "i2c-nct6775" > + > +/* Nuvoton SMBus address offsets */ > +#define SMBHSTDAT (0 + nuvoton_nct6793d_smba) > +#define SMBBLKSZ (1 + nuvoton_nct6793d_smba) > +#define SMBHSTCMD (2 + nuvoton_nct6793d_smba) > +#define SMBHSTIDX (3 + nuvoton_nct6793d_smba) > +#define SMBHSTCTL (4 + nuvoton_nct6793d_smba) > +#define SMBHSTADD (5 + nuvoton_nct6793d_smba) > +#define SMBHSTERR (9 + nuvoton_nct6793d_smba) > +#define SMBHSTSTS (0xE + nuvoton_nct6793d_smba) For consistency, you should be using hexadecimal for all the defines. I also believe that (nuvoton_nct6793d_smba + 0) would make more sense than (0 + nuvoton_nct6793d_smba), even though the result is the same. > + > +/* Command register */ > +#define NCT6793D_READ_BYTE 0 It is pretty confusing to prefix these constants with NCT6793D when the driver is named i2c-nct6775. > +#define NCT6793D_READ_WORD 1 > +#define NCT6793D_READ_BLOCK 2 > +#define NCT6793D_BLOCK_WRITE_READ_PROC_CALL 3 NCT6793D_BLOCK_PROC_CALL would be enough. > +#define NCT6793D_PROC_CALL 4 > +#define NCT6793D_WRITE_BYTE 8 > +#define NCT6793D_WRITE_WORD 9 > +#define NCT6793D_WRITE_BLOCK 10 > + > +/* Control register */ > +#define NCT6793D_MANUAL_START 128 > +#define NCT6793D_SOFT_RESET 64 > + > +/* Error register */ > +#define NCT6793D_NO_ACK 32 > + > +/* Status register */ > +#define NCT6793D_FIFO_EMPTY 1 > +#define NCT6793D_FIFO_FULL 2 > +#define NCT6793D_MANUAL_ACTIVE 4 The 6 values above are clearly bit masks, so you should use BIT() instead of decimal value for clarity. > + > +#define NCT6775_LD_SMBUS 0x0B > + > +/* Other settings */ > +#define MAX_RETRIES 400 > + > +enum kinds { nct6106, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793, > + nct6795, nct6796, nct6798 }; > + > +struct nct6775_sio_data { > + int sioreg; > + enum kinds kind; > +}; > + > +/* used to set data->name = nct6775_device_names[data->sio_kind] */ > +static const char * const nct6775_device_names[] = { > + "nct6106", > + "nct6775", > + "nct6776", > + "nct6779", > + "nct6791", > + "nct6792", > + "nct6793", > + "nct6795", > + "nct6796", > + "nct6798", > +}; > + > +static const char * const nct6775_sio_names[] __initconst = { > + "NCT6106D", > + "NCT6775F", > + "NCT6776D/F", > + "NCT6779D", > + "NCT6791D", > + "NCT6792D", > + "NCT6793D", > + "NCT6795D", > + "NCT6796D", > + "NCT6798D", > +}; > + > +#define SIO_REG_LDSEL 0x07 /* Logical device select */ > +#define SIO_REG_DEVID 0x20 /* Device ID (2 bytes) */ > +#define SIO_REG_SMBA 0x62 /* SMBus base address register */ > + > +#define SIO_NCT6106_ID 0xc450 > +#define SIO_NCT6775_ID 0xb470 > +#define SIO_NCT6776_ID 0xc330 > +#define SIO_NCT6779_ID 0xc560 > +#define SIO_NCT6791_ID 0xc800 > +#define SIO_NCT6792_ID 0xc910 > +#define SIO_NCT6793_ID 0xd120 > +#define SIO_NCT6795_ID 0xd350 > +#define SIO_NCT6796_ID 0xd420 > +#define SIO_NCT6798_ID 0xd428 > +#define SIO_ID_MASK 0xFFF0 > + > +static inline void > +superio_outb(int ioreg, int reg, int val) > +{ > + outb(reg, ioreg); > + outb(val, ioreg + 1); > +} > + > +static inline int > +superio_inb(int ioreg, int reg) > +{ > + outb(reg, ioreg); > + return inb(ioreg + 1); > +} > + > +static inline void > +superio_select(int ioreg, int ld) > +{ > + outb(SIO_REG_LDSEL, ioreg); > + outb(ld, ioreg + 1); > +} > + > +static inline int > +superio_enter(int ioreg) > +{ > + /* > + * Try to reserve <ioreg> and <ioreg + 1> for exclusive access. > + */ > + if (!request_muxed_region(ioreg, 2, DRVNAME)) > + return -EBUSY; > + > + outb(0x87, ioreg); > + outb(0x87, ioreg); > + > + return 0; > +} > + > +static inline void > +superio_exit(int ioreg) > +{ > + outb(0xaa, ioreg); > + outb(0x02, ioreg); > + outb(0x02, ioreg + 1); > + release_region(ioreg, 2); > +} > + > +/* > + * ISA constants > + */ > + > +#define IOREGION_ALIGNMENT (~7) > +#define IOREGION_LENGTH 2 > +#define ADDR_REG_OFFSET 0 > +#define DATA_REG_OFFSET 1 The offset defines are never used. > + > +#define NCT6775_REG_BANK 0x4E > +#define NCT6775_REG_CONFIG 0x40 You don't use these anywhere either. > + > +static struct i2c_adapter *nct6775_adapter; You can't possibly have a driver which claims to support 2 platform devices but have a single static pointer to store the SMBus adapter's data. The adapter's data should be stored as per-platform device data instead. > + > +struct i2c_nct6775_adapdata { > + unsigned short smba; > +}; > + > +/* Return negative errno on error. */ > +static s32 nct6775_access(struct i2c_adapter *adap, u16 addr, > + unsigned short flags, char read_write, > + u8 command, int size, union i2c_smbus_data *data) > +{ > + struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap); > + unsigned short nuvoton_nct6793d_smba = adapdata->smba; > + int i, len, cnt; > + union i2c_smbus_data tmp_data; "tmp" is never a good variable name. All local variables are temporary by nature. I don't understand why you need this variable anyway, you already have *data. > + int timeout = 0; > + > + tmp_data.word = 0; > + cnt = 0; > + len = 0; Do you actually need to initialize these variables? Initializing to 0 "just in case" is bad practice as it prevents the compiler from warning you if you end up using an uninitialized variable. > + > + outb_p(NCT6793D_SOFT_RESET, SMBHSTCTL); > + > + switch (size) { > + case I2C_SMBUS_QUICK: Are you sure this is actually supported? There's no define for this command. > + outb_p((addr << 1) | read_write, > + SMBHSTADD); Fits on one line. Same comment several times below. > + break; > + case I2C_SMBUS_BYTE_DATA: > + tmp_data.byte = data->byte; Please use the fallthrough pseudo keyword to make it clear that the lack of break is intentional. > + case I2C_SMBUS_BYTE: > + outb_p((addr << 1) | read_write, > + SMBHSTADD); > + outb_p(command, SMBHSTIDX); > + if (read_write == I2C_SMBUS_WRITE) { > + outb_p(tmp_data.byte, SMBHSTDAT); > + outb_p(NCT6793D_WRITE_BYTE, SMBHSTCMD); > + } else { > + outb_p(NCT6793D_READ_BYTE, SMBHSTCMD); > + } > + break; This look highly suspicious. You can't possibly support two different SMBus transaction types (Receive Byte and Read Byte) with the same command value (NCT6793D_READ_BYTE). Same for Send Byte vs Write Byte. > + case I2C_SMBUS_WORD_DATA: > + outb_p((addr << 1) | read_write, > + SMBHSTADD); > + outb_p(command, SMBHSTIDX); > + if (read_write == I2C_SMBUS_WRITE) { > + outb_p(data->word & 0xff, SMBHSTDAT); > + outb_p((data->word & 0xff00) >> 8, SMBHSTDAT); > + outb_p(NCT6793D_WRITE_WORD, SMBHSTCMD); > + } else { > + outb_p(NCT6793D_READ_WORD, SMBHSTCMD); > + } > + break; > + case I2C_SMBUS_BLOCK_DATA: > + outb_p((addr << 1) | read_write, > + SMBHSTADD); > + outb_p(command, SMBHSTIDX); > + if (read_write == I2C_SMBUS_WRITE) { > + len = data->block[0]; > + if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) > + return -EINVAL; > + outb_p(len, SMBBLKSZ); > + > + cnt = 1; > + if (len >= 4) { > + for (i = cnt; i <= 4; i++) > + outb_p(data->block[i], SMBHSTDAT); > + > + len -= 4; > + cnt += 4; > + } else { > + for (i = cnt; i <= len; i++) > + outb_p(data->block[i], SMBHSTDAT); > + > + len = 0; > + } > + > + outb_p(NCT6793D_WRITE_BLOCK, SMBHSTCMD); > + } else { > + return -EOPNOTSUPP; > + } > + break; > + default: > + dev_warn(&adap->dev, "Unsupported transaction %d\n", size); > + return -EOPNOTSUPP; > + } > + > + outb_p(NCT6793D_MANUAL_START, SMBHSTCTL); > + > + while ((size == I2C_SMBUS_BLOCK_DATA) && (len > 0)) { More parentheses than needed. > + if (read_write == I2C_SMBUS_WRITE) { > + timeout = 0; > + while ((inb_p(SMBHSTSTS) & NCT6793D_FIFO_EMPTY) == 0) { > + if (timeout > MAX_RETRIES) > + return -ETIMEDOUT; > + > + usleep_range(250, 500); > + timeout++; > + } > + > + //Load more bytes into FIFO Please use /* */ for comments. > + if (len >= 4) { > + for (i = cnt; i <= (cnt + 4); i++) > + outb_p(data->block[i], SMBHSTDAT); > + > + len -= 4; > + cnt += 4; > + } else { > + for (i = cnt; i <= (cnt + len); i++) > + outb_p(data->block[i], SMBHSTDAT); > + > + len = 0; > + } > + } else { This can't happen. > + return -EOPNOTSUPP; > + } > + > + } > + > + //wait for manual mode to complete > + timeout = 0; > + while ((inb_p(SMBHSTSTS) & NCT6793D_MANUAL_ACTIVE) != 0) { > + if (timeout > MAX_RETRIES) > + return -ETIMEDOUT; > + > + usleep_range(250, 500); > + timeout++; > + } > + > + if ((inb_p(SMBHSTERR) & NCT6793D_NO_ACK) != 0) > + return -ENXIO; > + > + else if ((read_write == I2C_SMBUS_WRITE) || (size == I2C_SMBUS_QUICK)) No blank line before "else". Or just remove the "else" which isn't needed. Too many parentheses. > + return 0; > + > + switch (size) { > + case I2C_SMBUS_QUICK: Never true. > + case I2C_SMBUS_BYTE_DATA: > + data->byte = inb_p(SMBHSTDAT); > + break; > + case I2C_SMBUS_WORD_DATA: > + data->word = inb_p(SMBHSTDAT) + (inb_p(SMBHSTDAT) << 8); | is preferred here. > + break; > + } > + return 0; > +} > + > +static u32 nct6775_func(struct i2c_adapter *adapter) > +{ > + return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | > + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | > + I2C_FUNC_SMBUS_BLOCK_DATA; > +} > + > +static const struct i2c_algorithm smbus_algorithm = { > + .smbus_xfer = nct6775_access, > + .functionality = nct6775_func, > +}; > + > +static int nct6775_add_adapter(unsigned short smba, const char *name, struct i2c_adapter **padap) IMHO it would make more sense to have this function return the struct i2c_adapter * (and use ERR_PTR to return errors). > +{ > + struct i2c_adapter *adap; > + struct i2c_nct6775_adapdata *adapdata; > + int retval; > + > + adap = kzalloc(sizeof(*adap), GFP_KERNEL); > + if (adap == NULL) > + return -ENOMEM; > + > + adap->owner = THIS_MODULE; > + adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD; Doesn't seem appropriate if the only known usage of this SMBus controller is RGB light control. > + adap->algo = &smbus_algorithm; > + > + adapdata = kzalloc(sizeof(*adapdata), GFP_KERNEL); > + if (adapdata == NULL) { > + kfree(adap); > + return -ENOMEM; > + } > + > + adapdata->smba = smba; > + > + snprintf(adap->name, sizeof(adap->name), > + "SMBus NCT67xx adapter%s at %04x", name, smba); Name is already an empty string, so this could be simplified. Alternatively, set name to the actual device name (available in nct6775_sio_names) and use that instead of NCT67xx. > + > + i2c_set_adapdata(adap, adapdata); > + > + retval = i2c_add_adapter(adap); > + if (retval) { > + kfree(adapdata); > + kfree(adap); > + return retval; > + } > + > + *padap = adap; > + return 0; > +} > + > +static void nct6775_remove_adapter(struct i2c_adapter *adap) > +{ > + struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap); > + > + if (adapdata->smba) { Always true, as far as I can see. > + i2c_del_adapter(adap); > + kfree(adapdata); > + kfree(adap); > + } > +} > + > +/* > + * when Super-I/O functions move to a separate file, the Super-I/O > + * bus will manage the lifetime of the device and this module will only keep > + * track of the nct6775 driver. But since we use platform_device_alloc(), we > + * must keep track of the device Leading capital and trailing dot would be appreciated. > + */ > +static struct platform_device *pdev[2]; > + > +static int nct6775_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct nct6775_sio_data *sio_data = dev_get_platdata(dev); > + struct resource *res; > + > + res = platform_get_resource(pdev, IORESOURCE_IO, 0); > + if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH, > + DRVNAME)) > + return -EBUSY; > + > + switch (sio_data->kind) { > + case nct6791: > + case nct6792: > + case nct6793: > + case nct6795: > + case nct6796: > + case nct6798: > + nct6775_add_adapter(res->start, "", &nct6775_adapter); > + break; I'm confused. The driver is named i2c-nct6775 but the NCT6775 isn't supported? I understand the idea of matching the name of the hwmon driver you used as a based, but it only makes sense if the same devices are supported. All references to unsupported devices should be removed from this driver, there's no reason to make the driver larger. > + default: > + return -ENODEV; > + } > + > + return 0; > +} > + > +static struct platform_driver i2c_nct6775_driver = { > + .driver = { > + .name = DRVNAME, > + }, > + .probe = nct6775_probe, > +}; > + > +static void __exit i2c_nct6775_exit(void) > +{ > + int i; > + > + if (nct6775_adapter) > + nct6775_remove_adapter(nct6775_adapter); That's a weird construct. Unloading the module should remove the platform devices, and the remove callback of the platform device driver should remove the adapter. > + > + for (i = 0; i < ARRAY_SIZE(pdev); i++) { > + if (pdev[i]) > + platform_device_unregister(pdev[i]); > + } > + platform_driver_unregister(&i2c_nct6775_driver); > +} > + > +/* nct6775_find() looks for a '627 in the Super-I/O config space */ This reference to the Winbond W83627HF chipset is way outdated. > +static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data) > +{ > + u16 val; > + int err; > + int addr; > + > + err = superio_enter(sioaddr); > + if (err) > + return err; > + > + val = (superio_inb(sioaddr, SIO_REG_DEVID) << 8) | > + superio_inb(sioaddr, SIO_REG_DEVID + 1); > + > + switch (val & SIO_ID_MASK) { > + case SIO_NCT6106_ID: > + sio_data->kind = nct6106; > + break; > + case SIO_NCT6775_ID: > + sio_data->kind = nct6775; > + break; > + case SIO_NCT6776_ID: > + sio_data->kind = nct6776; > + break; > + case SIO_NCT6779_ID: > + sio_data->kind = nct6779; > + break; > + case SIO_NCT6791_ID: > + sio_data->kind = nct6791; > + break; > + case SIO_NCT6792_ID: > + sio_data->kind = nct6792; > + break; > + case SIO_NCT6793_ID: > + sio_data->kind = nct6793; > + break; > + case SIO_NCT6795_ID: > + sio_data->kind = nct6795; > + break; > + case SIO_NCT6796_ID: > + sio_data->kind = nct6796; > + break; > + case SIO_NCT6798_ID: > + sio_data->kind = nct6798; > + break; > + default: > + if (val != 0xffff) > + pr_debug("unsupported chip ID: 0x%04x\n", val); > + superio_exit(sioaddr); > + return -ENODEV; > + } > + > + /* We have a known chip, find the SMBus I/O address */ > + superio_select(sioaddr, NCT6775_LD_SMBUS); > + val = (superio_inb(sioaddr, SIO_REG_SMBA) << 8) > + | superio_inb(sioaddr, SIO_REG_SMBA + 1); > + addr = val & IOREGION_ALIGNMENT; > + if (addr == 0) { > + pr_err("Refusing to enable a Super-I/O device with a base I/O port 0\n"); > + superio_exit(sioaddr); > + return -ENODEV; > + } > + > + superio_exit(sioaddr); > + pr_info("Found %s or compatible chip at %#x:%#x\n", > + nct6775_sio_names[sio_data->kind], sioaddr, addr); > + sio_data->sioreg = sioaddr; > + > + return addr; > +} > + > +static int __init i2c_nct6775_init(void) > +{ > + int i, err; > + bool found = false; > + int address; > + struct resource res; > + struct nct6775_sio_data sio_data; > + int sioaddr[2] = { 0x2e, 0x4e }; > + > + err = platform_driver_register(&i2c_nct6775_driver); > + if (err) > + return err; > + > + /* > + * initialize sio_data->kind and sio_data->sioreg. > + * > + * when Super-I/O functions move to a separate file, the Super-I/O > + * driver will probe 0x2e and 0x4e and auto-detect the presence of a > + * nct6775 hardware monitor, and call probe() > + */ > + for (i = 0; i < ARRAY_SIZE(pdev); i++) { > + address = nct6775_find(sioaddr[i], &sio_data); > + if (address <= 0) > + continue; > + > + found = true; > + > + pdev[i] = platform_device_alloc(DRVNAME, address); > + if (!pdev[i]) { > + err = -ENOMEM; > + goto exit_device_unregister; > + } > + > + err = platform_device_add_data(pdev[i], &sio_data, > + sizeof(struct nct6775_sio_data)); > + if (err) > + goto exit_device_put; > + > + memset(&res, 0, sizeof(res)); > + res.name = DRVNAME; > + res.start = address; > + res.end = address + IOREGION_LENGTH - 1; > + res.flags = IORESOURCE_IO; > + > + err = acpi_check_resource_conflict(&res); > + if (err) { > + platform_device_put(pdev[i]); > + pdev[i] = NULL; > + continue; > + } > + > + err = platform_device_add_resources(pdev[i], &res, 1); > + if (err) > + goto exit_device_put; > + > + /* platform_device_add calls probe() */ > + err = platform_device_add(pdev[i]); > + if (err) > + goto exit_device_put; > + } > + if (!found) { > + err = -ENODEV; > + goto exit_unregister; > + } > + > + return 0; > + > +exit_device_put: > + platform_device_put(pdev[i]); > +exit_device_unregister: > + while (--i >= 0) { > + if (pdev[i]) > + platform_device_unregister(pdev[i]); > + } > +exit_unregister: > + platform_driver_unregister(&i2c_nct6775_driver); > + return err; > +} > + > +MODULE_AUTHOR("Adam Honse <calcprogrammer1@gmail.com>"); > +MODULE_DESCRIPTION("SMBus driver for NCT6775F and compatible chips"); > +MODULE_LICENSE("GPL"); > + > +module_init(i2c_nct6775_init); > +module_exit(i2c_nct6775_exit);
Hi Adam,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v5.17-rc8 next-20220310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Adam-Honse/i2c-New-driver-for-Nuvoton-SMBus-adapters/20220314-064247
base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20220315/202203150442.qhRCXPnf-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 3e4950d7fa78ac83f33bbf1658e2f49a73719236)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/040a1be0d30ce2c611e30eb923d6f1b0afc44e7d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Adam-Honse/i2c-New-driver-for-Nuvoton-SMBus-adapters/20220314-064247
git checkout 040a1be0d30ce2c611e30eb923d6f1b0afc44e7d
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm64 SHELL=/bin/bash drivers/i2c/busses/ drivers/mfd/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/i2c/busses/i2c-nct6775.c:219:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
case I2C_SMBUS_BYTE:
^
drivers/i2c/busses/i2c-nct6775.c:219:2: note: insert '__attribute__((fallthrough));' to silence this warning
case I2C_SMBUS_BYTE:
^
__attribute__((fallthrough));
drivers/i2c/busses/i2c-nct6775.c:219:2: note: insert 'break;' to avoid fall-through
case I2C_SMBUS_BYTE:
^
break;
>> drivers/i2c/busses/i2c-nct6775.c:90:27: warning: unused variable 'nct6775_device_names' [-Wunused-const-variable]
static const char * const nct6775_device_names[] = {
^
drivers/i2c/busses/i2c-nct6775.c:133:1: warning: unused function 'superio_outb' [-Wunused-function]
superio_outb(int ioreg, int reg, int val)
^
3 warnings generated.
vim +219 drivers/i2c/busses/i2c-nct6775.c
88
89 /* used to set data->name = nct6775_device_names[data->sio_kind] */
> 90 static const char * const nct6775_device_names[] = {
91 "nct6106",
92 "nct6775",
93 "nct6776",
94 "nct6779",
95 "nct6791",
96 "nct6792",
97 "nct6793",
98 "nct6795",
99 "nct6796",
100 "nct6798",
101 };
102
103 static const char * const nct6775_sio_names[] __initconst = {
104 "NCT6106D",
105 "NCT6775F",
106 "NCT6776D/F",
107 "NCT6779D",
108 "NCT6791D",
109 "NCT6792D",
110 "NCT6793D",
111 "NCT6795D",
112 "NCT6796D",
113 "NCT6798D",
114 };
115
116 #define SIO_REG_LDSEL 0x07 /* Logical device select */
117 #define SIO_REG_DEVID 0x20 /* Device ID (2 bytes) */
118 #define SIO_REG_SMBA 0x62 /* SMBus base address register */
119
120 #define SIO_NCT6106_ID 0xc450
121 #define SIO_NCT6775_ID 0xb470
122 #define SIO_NCT6776_ID 0xc330
123 #define SIO_NCT6779_ID 0xc560
124 #define SIO_NCT6791_ID 0xc800
125 #define SIO_NCT6792_ID 0xc910
126 #define SIO_NCT6793_ID 0xd120
127 #define SIO_NCT6795_ID 0xd350
128 #define SIO_NCT6796_ID 0xd420
129 #define SIO_NCT6798_ID 0xd428
130 #define SIO_ID_MASK 0xFFF0
131
132 static inline void
133 superio_outb(int ioreg, int reg, int val)
134 {
135 outb(reg, ioreg);
136 outb(val, ioreg + 1);
137 }
138
139 static inline int
140 superio_inb(int ioreg, int reg)
141 {
142 outb(reg, ioreg);
143 return inb(ioreg + 1);
144 }
145
146 static inline void
147 superio_select(int ioreg, int ld)
148 {
149 outb(SIO_REG_LDSEL, ioreg);
150 outb(ld, ioreg + 1);
151 }
152
153 static inline int
154 superio_enter(int ioreg)
155 {
156 /*
157 * Try to reserve <ioreg> and <ioreg + 1> for exclusive access.
158 */
159 if (!request_muxed_region(ioreg, 2, DRVNAME))
160 return -EBUSY;
161
162 outb(0x87, ioreg);
163 outb(0x87, ioreg);
164
165 return 0;
166 }
167
168 static inline void
169 superio_exit(int ioreg)
170 {
171 outb(0xaa, ioreg);
172 outb(0x02, ioreg);
173 outb(0x02, ioreg + 1);
174 release_region(ioreg, 2);
175 }
176
177 /*
178 * ISA constants
179 */
180
181 #define IOREGION_ALIGNMENT (~7)
182 #define IOREGION_LENGTH 2
183 #define ADDR_REG_OFFSET 0
184 #define DATA_REG_OFFSET 1
185
186 #define NCT6775_REG_BANK 0x4E
187 #define NCT6775_REG_CONFIG 0x40
188
189 static struct i2c_adapter *nct6775_adapter;
190
191 struct i2c_nct6775_adapdata {
192 unsigned short smba;
193 };
194
195 /* Return negative errno on error. */
196 static s32 nct6775_access(struct i2c_adapter *adap, u16 addr,
197 unsigned short flags, char read_write,
198 u8 command, int size, union i2c_smbus_data *data)
199 {
200 struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap);
201 unsigned short nuvoton_nct6793d_smba = adapdata->smba;
202 int i, len, cnt;
203 union i2c_smbus_data tmp_data;
204 int timeout = 0;
205
206 tmp_data.word = 0;
207 cnt = 0;
208 len = 0;
209
210 outb_p(NCT6793D_SOFT_RESET, SMBHSTCTL);
211
212 switch (size) {
213 case I2C_SMBUS_QUICK:
214 outb_p((addr << 1) | read_write,
215 SMBHSTADD);
216 break;
217 case I2C_SMBUS_BYTE_DATA:
218 tmp_data.byte = data->byte;
> 219 case I2C_SMBUS_BYTE:
220 outb_p((addr << 1) | read_write,
221 SMBHSTADD);
222 outb_p(command, SMBHSTIDX);
223 if (read_write == I2C_SMBUS_WRITE) {
224 outb_p(tmp_data.byte, SMBHSTDAT);
225 outb_p(NCT6793D_WRITE_BYTE, SMBHSTCMD);
226 } else {
227 outb_p(NCT6793D_READ_BYTE, SMBHSTCMD);
228 }
229 break;
230 case I2C_SMBUS_WORD_DATA:
231 outb_p((addr << 1) | read_write,
232 SMBHSTADD);
233 outb_p(command, SMBHSTIDX);
234 if (read_write == I2C_SMBUS_WRITE) {
235 outb_p(data->word & 0xff, SMBHSTDAT);
236 outb_p((data->word & 0xff00) >> 8, SMBHSTDAT);
237 outb_p(NCT6793D_WRITE_WORD, SMBHSTCMD);
238 } else {
239 outb_p(NCT6793D_READ_WORD, SMBHSTCMD);
240 }
241 break;
242 case I2C_SMBUS_BLOCK_DATA:
243 outb_p((addr << 1) | read_write,
244 SMBHSTADD);
245 outb_p(command, SMBHSTIDX);
246 if (read_write == I2C_SMBUS_WRITE) {
247 len = data->block[0];
248 if (len == 0 || len > I2C_SMBUS_BLOCK_MAX)
249 return -EINVAL;
250 outb_p(len, SMBBLKSZ);
251
252 cnt = 1;
253 if (len >= 4) {
254 for (i = cnt; i <= 4; i++)
255 outb_p(data->block[i], SMBHSTDAT);
256
257 len -= 4;
258 cnt += 4;
259 } else {
260 for (i = cnt; i <= len; i++)
261 outb_p(data->block[i], SMBHSTDAT);
262
263 len = 0;
264 }
265
266 outb_p(NCT6793D_WRITE_BLOCK, SMBHSTCMD);
267 } else {
268 return -EOPNOTSUPP;
269 }
270 break;
271 default:
272 dev_warn(&adap->dev, "Unsupported transaction %d\n", size);
273 return -EOPNOTSUPP;
274 }
275
276 outb_p(NCT6793D_MANUAL_START, SMBHSTCTL);
277
278 while ((size == I2C_SMBUS_BLOCK_DATA) && (len > 0)) {
279 if (read_write == I2C_SMBUS_WRITE) {
280 timeout = 0;
281 while ((inb_p(SMBHSTSTS) & NCT6793D_FIFO_EMPTY) == 0) {
282 if (timeout > MAX_RETRIES)
283 return -ETIMEDOUT;
284
285 usleep_range(250, 500);
286 timeout++;
287 }
288
289 //Load more bytes into FIFO
290 if (len >= 4) {
291 for (i = cnt; i <= (cnt + 4); i++)
292 outb_p(data->block[i], SMBHSTDAT);
293
294 len -= 4;
295 cnt += 4;
296 } else {
297 for (i = cnt; i <= (cnt + len); i++)
298 outb_p(data->block[i], SMBHSTDAT);
299
300 len = 0;
301 }
302 } else {
303 return -EOPNOTSUPP;
304 }
305
306 }
307
308 //wait for manual mode to complete
309 timeout = 0;
310 while ((inb_p(SMBHSTSTS) & NCT6793D_MANUAL_ACTIVE) != 0) {
311 if (timeout > MAX_RETRIES)
312 return -ETIMEDOUT;
313
314 usleep_range(250, 500);
315 timeout++;
316 }
317
318 if ((inb_p(SMBHSTERR) & NCT6793D_NO_ACK) != 0)
319 return -ENXIO;
320
321 else if ((read_write == I2C_SMBUS_WRITE) || (size == I2C_SMBUS_QUICK))
322 return 0;
323
324 switch (size) {
325 case I2C_SMBUS_QUICK:
326 case I2C_SMBUS_BYTE_DATA:
327 data->byte = inb_p(SMBHSTDAT);
328 break;
329 case I2C_SMBUS_WORD_DATA:
330 data->word = inb_p(SMBHSTDAT) + (inb_p(SMBHSTDAT) << 8);
331 break;
332 }
333 return 0;
334 }
335
---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 8a6c6ee28556..cf4c5e92738b 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -219,6 +219,15 @@ config I2C_CHT_WC combined with a FUSB302 Type-C port-controller as such it is advised to also select CONFIG_TYPEC_FUSB302=m. +config I2C_NCT6775 + tristate "Nuvoton NCT6775 and compatible SMBus controller" + help + If you say yes to this option, support will be included for the + Nuvoton NCT6775 and compatible SMBus controllers. + + This driver can also be built as a module. If so, the module + will be called i2c-nct6775. + config I2C_NFORCE2 tristate "Nvidia nForce2, nForce3 and nForce4" depends on PCI diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 1d00dce77098..450cde39c7d5 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -17,6 +17,7 @@ obj-$(CONFIG_I2C_CHT_WC) += i2c-cht-wc.o obj-$(CONFIG_I2C_I801) += i2c-i801.o obj-$(CONFIG_I2C_ISCH) += i2c-isch.o obj-$(CONFIG_I2C_ISMT) += i2c-ismt.o +obj-$(CONFIG_I2C_NCT6775) += i2c-nct6775.o obj-$(CONFIG_I2C_NFORCE2) += i2c-nforce2.o obj-$(CONFIG_I2C_NFORCE2_S4985) += i2c-nforce2-s4985.o obj-$(CONFIG_I2C_NVIDIA_GPU) += i2c-nvidia-gpu.o diff --git a/drivers/i2c/busses/i2c-nct6775.c b/drivers/i2c/busses/i2c-nct6775.c new file mode 100644 index 000000000000..fdffe30f595b --- /dev/null +++ b/drivers/i2c/busses/i2c-nct6775.c @@ -0,0 +1,608 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * i2c-nct6775 - Driver for the SMBus master functionality of + * Nuvoton NCT677x Super-I/O chips + * + * Copyright (C) 2019 Adam Honse <calcprogrammer1@gmail.com> + * + * Derived from nct6775 hwmon driver + * Copyright (C) 2012 Guenter Roeck <linux@roeck-us.net> + * + * 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 <linux/module.h> +#include <linux/init.h> +#include <linux/slab.h> +#include <linux/jiffies.h> +#include <linux/platform_device.h> +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/hwmon-vid.h> +#include <linux/err.h> +#include <linux/mutex.h> +#include <linux/delay.h> +#include <linux/ioport.h> +#include <linux/i2c.h> +#include <linux/acpi.h> +#include <linux/bitops.h> +#include <linux/dmi.h> +#include <linux/io.h> +#include <linux/nospec.h> + +#define DRVNAME "i2c-nct6775" + +/* Nuvoton SMBus address offsets */ +#define SMBHSTDAT (0 + nuvoton_nct6793d_smba) +#define SMBBLKSZ (1 + nuvoton_nct6793d_smba) +#define SMBHSTCMD (2 + nuvoton_nct6793d_smba) +#define SMBHSTIDX (3 + nuvoton_nct6793d_smba) +#define SMBHSTCTL (4 + nuvoton_nct6793d_smba) +#define SMBHSTADD (5 + nuvoton_nct6793d_smba) +#define SMBHSTERR (9 + nuvoton_nct6793d_smba) +#define SMBHSTSTS (0xE + nuvoton_nct6793d_smba) + +/* Command register */ +#define NCT6793D_READ_BYTE 0 +#define NCT6793D_READ_WORD 1 +#define NCT6793D_READ_BLOCK 2 +#define NCT6793D_BLOCK_WRITE_READ_PROC_CALL 3 +#define NCT6793D_PROC_CALL 4 +#define NCT6793D_WRITE_BYTE 8 +#define NCT6793D_WRITE_WORD 9 +#define NCT6793D_WRITE_BLOCK 10 + +/* Control register */ +#define NCT6793D_MANUAL_START 128 +#define NCT6793D_SOFT_RESET 64 + +/* Error register */ +#define NCT6793D_NO_ACK 32 + +/* Status register */ +#define NCT6793D_FIFO_EMPTY 1 +#define NCT6793D_FIFO_FULL 2 +#define NCT6793D_MANUAL_ACTIVE 4 + +#define NCT6775_LD_SMBUS 0x0B + +/* Other settings */ +#define MAX_RETRIES 400 + +enum kinds { nct6106, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793, + nct6795, nct6796, nct6798 }; + +struct nct6775_sio_data { + int sioreg; + enum kinds kind; +}; + +/* used to set data->name = nct6775_device_names[data->sio_kind] */ +static const char * const nct6775_device_names[] = { + "nct6106", + "nct6775", + "nct6776", + "nct6779", + "nct6791", + "nct6792", + "nct6793", + "nct6795", + "nct6796", + "nct6798", +}; + +static const char * const nct6775_sio_names[] __initconst = { + "NCT6106D", + "NCT6775F", + "NCT6776D/F", + "NCT6779D", + "NCT6791D", + "NCT6792D", + "NCT6793D", + "NCT6795D", + "NCT6796D", + "NCT6798D", +}; + +#define SIO_REG_LDSEL 0x07 /* Logical device select */ +#define SIO_REG_DEVID 0x20 /* Device ID (2 bytes) */ +#define SIO_REG_SMBA 0x62 /* SMBus base address register */ + +#define SIO_NCT6106_ID 0xc450 +#define SIO_NCT6775_ID 0xb470 +#define SIO_NCT6776_ID 0xc330 +#define SIO_NCT6779_ID 0xc560 +#define SIO_NCT6791_ID 0xc800 +#define SIO_NCT6792_ID 0xc910 +#define SIO_NCT6793_ID 0xd120 +#define SIO_NCT6795_ID 0xd350 +#define SIO_NCT6796_ID 0xd420 +#define SIO_NCT6798_ID 0xd428 +#define SIO_ID_MASK 0xFFF0 + +static inline void +superio_outb(int ioreg, int reg, int val) +{ + outb(reg, ioreg); + outb(val, ioreg + 1); +} + +static inline int +superio_inb(int ioreg, int reg) +{ + outb(reg, ioreg); + return inb(ioreg + 1); +} + +static inline void +superio_select(int ioreg, int ld) +{ + outb(SIO_REG_LDSEL, ioreg); + outb(ld, ioreg + 1); +} + +static inline int +superio_enter(int ioreg) +{ + /* + * Try to reserve <ioreg> and <ioreg + 1> for exclusive access. + */ + if (!request_muxed_region(ioreg, 2, DRVNAME)) + return -EBUSY; + + outb(0x87, ioreg); + outb(0x87, ioreg); + + return 0; +} + +static inline void +superio_exit(int ioreg) +{ + outb(0xaa, ioreg); + outb(0x02, ioreg); + outb(0x02, ioreg + 1); + release_region(ioreg, 2); +} + +/* + * ISA constants + */ + +#define IOREGION_ALIGNMENT (~7) +#define IOREGION_LENGTH 2 +#define ADDR_REG_OFFSET 0 +#define DATA_REG_OFFSET 1 + +#define NCT6775_REG_BANK 0x4E +#define NCT6775_REG_CONFIG 0x40 + +static struct i2c_adapter *nct6775_adapter; + +struct i2c_nct6775_adapdata { + unsigned short smba; +}; + +/* Return negative errno on error. */ +static s32 nct6775_access(struct i2c_adapter *adap, u16 addr, + unsigned short flags, char read_write, + u8 command, int size, union i2c_smbus_data *data) +{ + struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap); + unsigned short nuvoton_nct6793d_smba = adapdata->smba; + int i, len, cnt; + union i2c_smbus_data tmp_data; + int timeout = 0; + + tmp_data.word = 0; + cnt = 0; + len = 0; + + outb_p(NCT6793D_SOFT_RESET, SMBHSTCTL); + + switch (size) { + case I2C_SMBUS_QUICK: + outb_p((addr << 1) | read_write, + SMBHSTADD); + break; + case I2C_SMBUS_BYTE_DATA: + tmp_data.byte = data->byte; + case I2C_SMBUS_BYTE: + outb_p((addr << 1) | read_write, + SMBHSTADD); + outb_p(command, SMBHSTIDX); + if (read_write == I2C_SMBUS_WRITE) { + outb_p(tmp_data.byte, SMBHSTDAT); + outb_p(NCT6793D_WRITE_BYTE, SMBHSTCMD); + } else { + outb_p(NCT6793D_READ_BYTE, SMBHSTCMD); + } + break; + case I2C_SMBUS_WORD_DATA: + outb_p((addr << 1) | read_write, + SMBHSTADD); + outb_p(command, SMBHSTIDX); + if (read_write == I2C_SMBUS_WRITE) { + outb_p(data->word & 0xff, SMBHSTDAT); + outb_p((data->word & 0xff00) >> 8, SMBHSTDAT); + outb_p(NCT6793D_WRITE_WORD, SMBHSTCMD); + } else { + outb_p(NCT6793D_READ_WORD, SMBHSTCMD); + } + break; + case I2C_SMBUS_BLOCK_DATA: + outb_p((addr << 1) | read_write, + SMBHSTADD); + outb_p(command, SMBHSTIDX); + if (read_write == I2C_SMBUS_WRITE) { + len = data->block[0]; + if (len == 0 || len > I2C_SMBUS_BLOCK_MAX) + return -EINVAL; + outb_p(len, SMBBLKSZ); + + cnt = 1; + if (len >= 4) { + for (i = cnt; i <= 4; i++) + outb_p(data->block[i], SMBHSTDAT); + + len -= 4; + cnt += 4; + } else { + for (i = cnt; i <= len; i++) + outb_p(data->block[i], SMBHSTDAT); + + len = 0; + } + + outb_p(NCT6793D_WRITE_BLOCK, SMBHSTCMD); + } else { + return -EOPNOTSUPP; + } + break; + default: + dev_warn(&adap->dev, "Unsupported transaction %d\n", size); + return -EOPNOTSUPP; + } + + outb_p(NCT6793D_MANUAL_START, SMBHSTCTL); + + while ((size == I2C_SMBUS_BLOCK_DATA) && (len > 0)) { + if (read_write == I2C_SMBUS_WRITE) { + timeout = 0; + while ((inb_p(SMBHSTSTS) & NCT6793D_FIFO_EMPTY) == 0) { + if (timeout > MAX_RETRIES) + return -ETIMEDOUT; + + usleep_range(250, 500); + timeout++; + } + + //Load more bytes into FIFO + if (len >= 4) { + for (i = cnt; i <= (cnt + 4); i++) + outb_p(data->block[i], SMBHSTDAT); + + len -= 4; + cnt += 4; + } else { + for (i = cnt; i <= (cnt + len); i++) + outb_p(data->block[i], SMBHSTDAT); + + len = 0; + } + } else { + return -EOPNOTSUPP; + } + + } + + //wait for manual mode to complete + timeout = 0; + while ((inb_p(SMBHSTSTS) & NCT6793D_MANUAL_ACTIVE) != 0) { + if (timeout > MAX_RETRIES) + return -ETIMEDOUT; + + usleep_range(250, 500); + timeout++; + } + + if ((inb_p(SMBHSTERR) & NCT6793D_NO_ACK) != 0) + return -ENXIO; + + else if ((read_write == I2C_SMBUS_WRITE) || (size == I2C_SMBUS_QUICK)) + return 0; + + switch (size) { + case I2C_SMBUS_QUICK: + case I2C_SMBUS_BYTE_DATA: + data->byte = inb_p(SMBHSTDAT); + break; + case I2C_SMBUS_WORD_DATA: + data->word = inb_p(SMBHSTDAT) + (inb_p(SMBHSTDAT) << 8); + break; + } + return 0; +} + +static u32 nct6775_func(struct i2c_adapter *adapter) +{ + return I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | + I2C_FUNC_SMBUS_BLOCK_DATA; +} + +static const struct i2c_algorithm smbus_algorithm = { + .smbus_xfer = nct6775_access, + .functionality = nct6775_func, +}; + +static int nct6775_add_adapter(unsigned short smba, const char *name, struct i2c_adapter **padap) +{ + struct i2c_adapter *adap; + struct i2c_nct6775_adapdata *adapdata; + int retval; + + adap = kzalloc(sizeof(*adap), GFP_KERNEL); + if (adap == NULL) + return -ENOMEM; + + adap->owner = THIS_MODULE; + adap->class = I2C_CLASS_HWMON | I2C_CLASS_SPD; + adap->algo = &smbus_algorithm; + + adapdata = kzalloc(sizeof(*adapdata), GFP_KERNEL); + if (adapdata == NULL) { + kfree(adap); + return -ENOMEM; + } + + adapdata->smba = smba; + + snprintf(adap->name, sizeof(adap->name), + "SMBus NCT67xx adapter%s at %04x", name, smba); + + i2c_set_adapdata(adap, adapdata); + + retval = i2c_add_adapter(adap); + if (retval) { + kfree(adapdata); + kfree(adap); + return retval; + } + + *padap = adap; + return 0; +} + +static void nct6775_remove_adapter(struct i2c_adapter *adap) +{ + struct i2c_nct6775_adapdata *adapdata = i2c_get_adapdata(adap); + + if (adapdata->smba) { + i2c_del_adapter(adap); + kfree(adapdata); + kfree(adap); + } +} + +/* + * when Super-I/O functions move to a separate file, the Super-I/O + * bus will manage the lifetime of the device and this module will only keep + * track of the nct6775 driver. But since we use platform_device_alloc(), we + * must keep track of the device + */ +static struct platform_device *pdev[2]; + +static int nct6775_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct nct6775_sio_data *sio_data = dev_get_platdata(dev); + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_IO, 0); + if (!devm_request_region(&pdev->dev, res->start, IOREGION_LENGTH, + DRVNAME)) + return -EBUSY; + + switch (sio_data->kind) { + case nct6791: + case nct6792: + case nct6793: + case nct6795: + case nct6796: + case nct6798: + nct6775_add_adapter(res->start, "", &nct6775_adapter); + break; + default: + return -ENODEV; + } + + return 0; +} + +static struct platform_driver i2c_nct6775_driver = { + .driver = { + .name = DRVNAME, + }, + .probe = nct6775_probe, +}; + +static void __exit i2c_nct6775_exit(void) +{ + int i; + + if (nct6775_adapter) + nct6775_remove_adapter(nct6775_adapter); + + for (i = 0; i < ARRAY_SIZE(pdev); i++) { + if (pdev[i]) + platform_device_unregister(pdev[i]); + } + platform_driver_unregister(&i2c_nct6775_driver); +} + +/* nct6775_find() looks for a '627 in the Super-I/O config space */ +static int __init nct6775_find(int sioaddr, struct nct6775_sio_data *sio_data) +{ + u16 val; + int err; + int addr; + + err = superio_enter(sioaddr); + if (err) + return err; + + val = (superio_inb(sioaddr, SIO_REG_DEVID) << 8) | + superio_inb(sioaddr, SIO_REG_DEVID + 1); + + switch (val & SIO_ID_MASK) { + case SIO_NCT6106_ID: + sio_data->kind = nct6106; + break; + case SIO_NCT6775_ID: + sio_data->kind = nct6775; + break; + case SIO_NCT6776_ID: + sio_data->kind = nct6776; + break; + case SIO_NCT6779_ID: + sio_data->kind = nct6779; + break; + case SIO_NCT6791_ID: + sio_data->kind = nct6791; + break; + case SIO_NCT6792_ID: + sio_data->kind = nct6792; + break; + case SIO_NCT6793_ID: + sio_data->kind = nct6793; + break; + case SIO_NCT6795_ID: + sio_data->kind = nct6795; + break; + case SIO_NCT6796_ID: + sio_data->kind = nct6796; + break; + case SIO_NCT6798_ID: + sio_data->kind = nct6798; + break; + default: + if (val != 0xffff) + pr_debug("unsupported chip ID: 0x%04x\n", val); + superio_exit(sioaddr); + return -ENODEV; + } + + /* We have a known chip, find the SMBus I/O address */ + superio_select(sioaddr, NCT6775_LD_SMBUS); + val = (superio_inb(sioaddr, SIO_REG_SMBA) << 8) + | superio_inb(sioaddr, SIO_REG_SMBA + 1); + addr = val & IOREGION_ALIGNMENT; + if (addr == 0) { + pr_err("Refusing to enable a Super-I/O device with a base I/O port 0\n"); + superio_exit(sioaddr); + return -ENODEV; + } + + superio_exit(sioaddr); + pr_info("Found %s or compatible chip at %#x:%#x\n", + nct6775_sio_names[sio_data->kind], sioaddr, addr); + sio_data->sioreg = sioaddr; + + return addr; +} + +static int __init i2c_nct6775_init(void) +{ + int i, err; + bool found = false; + int address; + struct resource res; + struct nct6775_sio_data sio_data; + int sioaddr[2] = { 0x2e, 0x4e }; + + err = platform_driver_register(&i2c_nct6775_driver); + if (err) + return err; + + /* + * initialize sio_data->kind and sio_data->sioreg. + * + * when Super-I/O functions move to a separate file, the Super-I/O + * driver will probe 0x2e and 0x4e and auto-detect the presence of a + * nct6775 hardware monitor, and call probe() + */ + for (i = 0; i < ARRAY_SIZE(pdev); i++) { + address = nct6775_find(sioaddr[i], &sio_data); + if (address <= 0) + continue; + + found = true; + + pdev[i] = platform_device_alloc(DRVNAME, address); + if (!pdev[i]) { + err = -ENOMEM; + goto exit_device_unregister; + } + + err = platform_device_add_data(pdev[i], &sio_data, + sizeof(struct nct6775_sio_data)); + if (err) + goto exit_device_put; + + memset(&res, 0, sizeof(res)); + res.name = DRVNAME; + res.start = address; + res.end = address + IOREGION_LENGTH - 1; + res.flags = IORESOURCE_IO; + + err = acpi_check_resource_conflict(&res); + if (err) { + platform_device_put(pdev[i]); + pdev[i] = NULL; + continue; + } + + err = platform_device_add_resources(pdev[i], &res, 1); + if (err) + goto exit_device_put; + + /* platform_device_add calls probe() */ + err = platform_device_add(pdev[i]); + if (err) + goto exit_device_put; + } + if (!found) { + err = -ENODEV; + goto exit_unregister; + } + + return 0; + +exit_device_put: + platform_device_put(pdev[i]); +exit_device_unregister: + while (--i >= 0) { + if (pdev[i]) + platform_device_unregister(pdev[i]); + } +exit_unregister: + platform_driver_unregister(&i2c_nct6775_driver); + return err; +} + +MODULE_AUTHOR("Adam Honse <calcprogrammer1@gmail.com>"); +MODULE_DESCRIPTION("SMBus driver for NCT6775F and compatible chips"); +MODULE_LICENSE("GPL"); + +module_init(i2c_nct6775_init); +module_exit(i2c_nct6775_exit);
This patch introduces a new driver for the SMBus adapter that is built in to most Nuvoton Super-IO chips. This SMBus adapter is used for RGB lighting control on some ASUS motherboards with Intel chipsets. The interface's register description is available in the supported devices' datasheets. Operation of this interface has been verified with OpenRGB on an ASUS PRIME Z270-A motherboard. Signed-off-by: Adam Honse <calcprogrammer1@gmail.com> --- drivers/i2c/busses/Kconfig | 9 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-nct6775.c | 608 +++++++++++++++++++++++++++++++ 3 files changed, 618 insertions(+) create mode 100644 drivers/i2c/busses/i2c-nct6775.c