On Tue, Apr 09, 2019 at 10:20:22AM -0500, Eddie James wrote: > > On 4/9/19 7:45 AM, Jean Delvare wrote: > >Don't propose PowerPC-only drivers on other architectures, unless > >build-testing. > > > This driver does NOT only run on PowerPC; rather it runs on a BMC processor > connected to a PowerPC processor. BMC will most likely be ARM, but shouldn't > be restricted to that arch only. > > > > > >Also drop configuration symbol SENSORS_OCC which serves no purpose > >that I can see. > > > >Signed-off-by: Jean Delvare <jdelvare@xxxxxxx> > >Cc: Eddie James <eajames@xxxxxxxxxxxxx> > >Cc: Guenter Roeck <linux@xxxxxxxxxxxx> > >--- > >SENSORS_OCC *would* serve a purpose if the common code between the > >POWER8 driver and the POWER9 driver would go in a separate, shared > >module, and occ-p8-hwmon and occ-p9-hwmon would only contain the > >specific code. This would avoid packaging the same code twice in 2 > >separate modules, therefore saving some storage space for ppc > >distributions. > > > Well you'd never have both P8 and P9 enabled at once, so space shouldn't be > an issue. I agree this could be cleaner but I think I was getting duplicate > symbol errors for the compile test and so I did it this way. If this doesn't > lead to errors in the compile test, I'm fine with this (without the change > for PPC only though). > Any common code would have to be in a separate module to avoid duplicate symbols. You'd bundle common.c and sysfs.c into one module and export the functions called from p8/p9 specific code. Something like occ-common-objs := common.o sysfs.o obj-$(CONFIG_SENSORS_OCC) += occ-common.o obj-$(CONFIG_SENSORS_OCC_P8_I2C) += p8_i2c.o obj-$(CONFIG_SENSORS_OCC_P9_SBE) += p9-sbe.o Guenter > > Thanks, > > Eddie > > > > >As far as I can see, this would simply require exporting 2 functions > >(occ_setup and occ_shutdown). Is there any reason why things were not > >done that way in the first place? This would look cleaner to me. > > > > drivers/hwmon/Makefile | 2 +- > > drivers/hwmon/occ/Kconfig | 8 ++------ > > 2 files changed, 3 insertions(+), 7 deletions(-) > > > >--- linux-5.0.orig/drivers/hwmon/occ/Kconfig 2019-03-04 00:21:29.000000000 +0100 > >+++ linux-5.0/drivers/hwmon/occ/Kconfig 2019-04-09 14:08:41.316551071 +0200 > >@@ -5,7 +5,7 @@ > > config SENSORS_OCC_P8_I2C > > tristate "POWER8 OCC through I2C" > > depends on I2C > >- select SENSORS_OCC > >+ depends on POWERPC || COMPILE_TEST > > help > > This option enables support for monitoring sensors provided by the > > On-Chip Controller (OCC) on a POWER8 processor. Communications with > >@@ -17,7 +17,7 @@ config SENSORS_OCC_P8_I2C > > config SENSORS_OCC_P9_SBE > > tristate "POWER9 OCC through SBE" > > depends on FSI_OCC > >- select SENSORS_OCC > >+ depends on POWERPC || COMPILE_TEST > > help > > This option enables support for monitoring sensors provided by the > > On-Chip Controller (OCC) on a POWER9 processor. Communications with > >@@ -25,7 +25,3 @@ config SENSORS_OCC_P9_SBE > > This driver can also be built as a module. If so, the module will be > > called occ-p9-hwmon. > >- > >-config SENSORS_OCC > >- bool "POWER On-Chip Controller" > >- depends on SENSORS_OCC_P8_I2C || SENSORS_OCC_P9_SBE > >--- linux-5.0.orig/drivers/hwmon/Makefile 2019-03-04 00:21:29.000000000 +0100 > >+++ linux-5.0/drivers/hwmon/Makefile 2019-04-09 14:33:49.605510047 +0200 > >@@ -178,7 +178,7 @@ obj-$(CONFIG_SENSORS_WM831X) += wm831x-h > > obj-$(CONFIG_SENSORS_WM8350) += wm8350-hwmon.o > > obj-$(CONFIG_SENSORS_XGENE) += xgene-hwmon.o > >-obj-$(CONFIG_SENSORS_OCC) += occ/ > >+obj-y += occ/ > > obj-$(CONFIG_PMBUS) += pmbus/ > > ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG > > > > >
