Re: [PATCH] i2c: designware: platdrv: Set class based on dmi
On Wed, Jun 24, 2020 at 01:31:06PM +0200, Ricardo Ribalda Delgado wrote: > On Wed, Jun 24, 2020 at 12:46 PM Andy Shevchenko > wrote: > > > > On Wed, Jun 24, 2020 at 11:12:39AM +0200, Ricardo Ribalda wrote: > > > Current AMD's zen-based APUs use this core for some of its i2c-buses. > > > > > > With this patch we re-enable autodetection of hwmon-alike devices, so > > > lm-sensors will be able to work automatically. > > > > > > It does not affect the boot-time of embedded devices, as the class is > > > set based on the dmi information. > > > > Hmm... Do we really need to have DMI? I mean wouldn't be safe just always > > provide this to be compatible with HWMON class? > > > > I do not care :), I was just trying to follow the logic of > 70fba8302adecfa08a087c6f1dd39537a55f5bd3 > > If it is decided otherwise I can change it, no problem ;) I guess you need to send a kinda revert with a fixes tag. -- With Best Regards, Andy Shevchenko
Re: [PATCH] i2c: designware: platdrv: Set class based on dmi
Hi Andy On Wed, Jun 24, 2020 at 12:46 PM Andy Shevchenko wrote: > > On Wed, Jun 24, 2020 at 11:12:39AM +0200, Ricardo Ribalda wrote: > > Current AMD's zen-based APUs use this core for some of its i2c-buses. > > > > With this patch we re-enable autodetection of hwmon-alike devices, so > > lm-sensors will be able to work automatically. > > > > It does not affect the boot-time of embedded devices, as the class is > > set based on the dmi information. > > Hmm... Do we really need to have DMI? I mean wouldn't be safe just always > provide this to be compatible with HWMON class? > I do not care :), I was just trying to follow the logic of 70fba8302adecfa08a087c6f1dd39537a55f5bd3 If it is decided otherwise I can change it, no problem ;) > ... > > > +static bool dw_i2c_hwmon_bus(void) > > +{ > > + if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) > > + return true; > > + return false; > > +} > > I don't like this. Perhaps for now you may simple use dmi_get_system_info() > directly below. I just realised that if there is no DMI, then we get a NULL, so we need to add that check. With the two ifs, I still prefer to put it on other function like now (check v2). If you still dont like it I will move it into the main function. > > ... > > > + adap->class = dw_i2c_hwmon_bus() ? I2C_CLASS_HWMON > > + : I2C_CLASS_DEPRECATED; > > It's one line. Fixed thanks > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH] i2c: designware: platdrv: Set class based on dmi
On Wed, Jun 24, 2020 at 01:46:55PM +0300, Andy Shevchenko wrote: > On Wed, Jun 24, 2020 at 11:12:39AM +0200, Ricardo Ribalda wrote: > > Current AMD's zen-based APUs use this core for some of its i2c-buses. > > > > With this patch we re-enable autodetection of hwmon-alike devices, so > > lm-sensors will be able to work automatically. > > > > It does not affect the boot-time of embedded devices, as the class is > > set based on the dmi information. > > Hmm... Do we really need to have DMI? I mean wouldn't be safe just always > provide this to be compatible with HWMON class? It is not unsafe. Embedded people might complain about increased boot time whenever a CLASS_HWMON driver scans the addresses it knows. signature.asc Description: PGP signature
Re: [PATCH] i2c: designware: platdrv: Set class based on dmi
Hi Ricardo, I love your patch! Yet something to improve: [auto build test ERROR on wsa/i2c/for-next] [also build test ERROR on v5.8-rc2 next-20200624] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Ricardo-Ribalda/i2c-designware-platdrv-Set-class-based-on-dmi/20200624-171557 base: https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next config: arc-allyesconfig (attached as .config) compiler: arc-elf-gcc (GCC) 9.3.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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All error/warnings (new ones prefixed by >>): drivers/i2c/busses/i2c-designware-platdrv.c: In function 'dw_i2c_hwmon_bus': >> drivers/i2c/busses/i2c-designware-platdrv.c:196:13: error: implicit >> declaration of function 'dmi_get_system_info'; did you mean >> 'acpi_get_system_info'? [-Werror=implicit-function-declaration] 196 | if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) | ^~~ | acpi_get_system_info >> drivers/i2c/busses/i2c-designware-platdrv.c:196:13: warning: passing >> argument 1 of 'strstr' makes pointer from integer without a cast >> [-Wint-conversion] 196 | if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) | ^ | | | int In file included from include/linux/bitmap.h:9, from include/linux/cpumask.h:12, from include/linux/rcupdate.h:31, from include/linux/radix-tree.h:15, from include/linux/idr.h:15, from include/linux/kernfs.h:13, from include/linux/sysfs.h:16, from include/linux/kobject.h:20, from include/linux/of.h:17, from include/linux/irqdomain.h:35, from include/linux/acpi.h:13, from drivers/i2c/busses/i2c-designware-platdrv.c:11: include/linux/string.h:82:15: note: expected 'const char *' but argument is of type 'int' 82 | extern char * strstr(const char *, const char *); | ^~ cc1: some warnings being treated as errors vim +196 drivers/i2c/busses/i2c-designware-platdrv.c 193 194 static bool dw_i2c_hwmon_bus(void) 195 { > 196 if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) 197 return true; 198 return false; 199 } 200 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH] i2c: designware: platdrv: Set class based on dmi
On Wed, Jun 24, 2020 at 11:12:39AM +0200, Ricardo Ribalda wrote: > Current AMD's zen-based APUs use this core for some of its i2c-buses. > > With this patch we re-enable autodetection of hwmon-alike devices, so > lm-sensors will be able to work automatically. > > It does not affect the boot-time of embedded devices, as the class is > set based on the dmi information. Hmm... Do we really need to have DMI? I mean wouldn't be safe just always provide this to be compatible with HWMON class? ... > +static bool dw_i2c_hwmon_bus(void) > +{ > + if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) > + return true; > + return false; > +} I don't like this. Perhaps for now you may simple use dmi_get_system_info() directly below. ... > + adap->class = dw_i2c_hwmon_bus() ? I2C_CLASS_HWMON > + : I2C_CLASS_DEPRECATED; It's one line. -- With Best Regards, Andy Shevchenko
[PATCH] i2c: designware: platdrv: Set class based on dmi
Current AMD's zen-based APUs use this core for some of its i2c-buses. With this patch we re-enable autodetection of hwmon-alike devices, so lm-sensors will be able to work automatically. It does not affect the boot-time of embedded devices, as the class is set based on the dmi information. Signed-off-by: Ricardo Ribalda --- drivers/i2c/busses/i2c-designware-platdrv.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c index c2efc252..f0a02fc3c135 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -191,6 +191,13 @@ static int dw_i2c_plat_request_regs(struct dw_i2c_dev *dev) return ret; } +static bool dw_i2c_hwmon_bus(void) +{ + if (strstr(dmi_get_system_info(DMI_PRODUCT_NAME), "QT5222")) + return true; + return false; +} + static int dw_i2c_plat_probe(struct platform_device *pdev) { struct dw_i2c_platform_data *pdata = dev_get_platdata(>dev); @@ -267,7 +274,8 @@ static int dw_i2c_plat_probe(struct platform_device *pdev) adap = >adapter; adap->owner = THIS_MODULE; - adap->class = I2C_CLASS_DEPRECATED; + adap->class = dw_i2c_hwmon_bus() ? I2C_CLASS_HWMON +: I2C_CLASS_DEPRECATED; ACPI_COMPANION_SET(>dev, ACPI_COMPANION(>dev)); adap->dev.of_node = pdev->dev.of_node; adap->nr = -1; -- 2.27.0