| Message ID | 20190410124726.2d7e9d38@endymion (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2,1/2] hwmon: (occ) Move common code to a separate module | expand |
On 4/10/19 5:47 AM, Jean Delvare wrote: > Instead of duplicating the common code into the 2 (binary) drivers, > move the common code to a separate module. This is cleaner. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Eddie James <eajames@linux.ibm.com> > Cc: Guenter Roeck <linux@roeck-us.net> > --- > Eddie, can you please give it a try and confirm it works? Yes, this works well. Reviewed-by: Eddie James <eajames@linux.ibm.com> Tested-by: Eddie James <eajames@linux.ibm.com> > > Note: I kept the module names as they were before, hence the extra > "*-objs :=" statements. They could be removed if we rename the source > files, but that's better done in git directly. I don't mind either way > personally. > > drivers/hwmon/occ/Kconfig | 3 +-- > drivers/hwmon/occ/Makefile | 6 ++++-- > drivers/hwmon/occ/common.c | 7 +++++++ > drivers/hwmon/occ/sysfs.c | 2 ++ > 4 files changed, 14 insertions(+), 4 deletions(-) > > --- linux-5.0.orig/drivers/hwmon/occ/Kconfig 2019-04-10 11:30:05.579537638 +0200 > +++ linux-5.0/drivers/hwmon/occ/Kconfig 2019-04-10 11:31:20.843383376 +0200 > @@ -27,5 +27,4 @@ config SENSORS_OCC_P9_SBE > called occ-p9-hwmon. > > config SENSORS_OCC > - bool "POWER On-Chip Controller" > - depends on SENSORS_OCC_P8_I2C || SENSORS_OCC_P9_SBE > + tristate > --- linux-5.0.orig/drivers/hwmon/occ/Makefile 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/Makefile 2019-04-10 11:33:23.631765535 +0200 > @@ -1,5 +1,7 @@ > -occ-p8-hwmon-objs := common.o sysfs.o p8_i2c.o > -occ-p9-hwmon-objs := common.o sysfs.o p9_sbe.o > +occ-hwmon-common-objs := common.o sysfs.o > +occ-p8-hwmon-objs := p8_i2c.o > +occ-p9-hwmon-objs := p9_sbe.o > > +obj-$(CONFIG_SENSORS_OCC) += occ-hwmon-common.o > obj-$(CONFIG_SENSORS_OCC_P8_I2C) += occ-p8-hwmon.o > obj-$(CONFIG_SENSORS_OCC_P9_SBE) += occ-p9-hwmon.o > --- linux-5.0.orig/drivers/hwmon/occ/common.c 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/common.c 2019-04-10 11:44:53.035573580 +0200 > @@ -1,11 +1,13 @@ > // SPDX-License-Identifier: GPL-2.0 > > #include <linux/device.h> > +#include <linux/export.h> > #include <linux/hwmon.h> > #include <linux/hwmon-sysfs.h> > #include <linux/jiffies.h> > #include <linux/kernel.h> > #include <linux/math64.h> > +#include <linux/module.h> > #include <linux/mutex.h> > #include <linux/sysfs.h> > #include <asm/unaligned.h> > @@ -1096,3 +1098,8 @@ int occ_setup(struct occ *occ, const cha > > return rc; > } > +EXPORT_SYMBOL_GPL(occ_setup); > + > +MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>"); > +MODULE_DESCRIPTION("Common OCC hwmon code"); > +MODULE_LICENSE("GPL"); > --- linux-5.0.orig/drivers/hwmon/occ/sysfs.c 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/sysfs.c 2019-04-10 11:39:38.627003382 +0200 > @@ -12,6 +12,7 @@ > > #include <linux/bitops.h> > #include <linux/device.h> > +#include <linux/export.h> > #include <linux/hwmon-sysfs.h> > #include <linux/kernel.h> > #include <linux/sysfs.h> > @@ -186,3 +187,4 @@ void occ_shutdown(struct occ *occ) > { > sysfs_remove_group(&occ->bus_dev->kobj, &occ_sysfs); > } > +EXPORT_SYMBOL_GPL(occ_shutdown); > >
Jean, On Wed, Apr 10, 2019 at 12:47:26PM +0200, Jean Delvare wrote: > Instead of duplicating the common code into the 2 (binary) drivers, > move the common code to a separate module. This is cleaner. > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > Cc: Eddie James <eajames@linux.ibm.com> > Cc: Guenter Roeck <linux@roeck-us.net> what is the parent release for this patch ? I tried to apply it to mainline and to hwmon-next using git am, but both failed. Thanks, Guenter > --- > Eddie, can you please give it a try and confirm it works? > > Note: I kept the module names as they were before, hence the extra > "*-objs :=" statements. They could be removed if we rename the source > files, but that's better done in git directly. I don't mind either way > personally. > > drivers/hwmon/occ/Kconfig | 3 +-- > drivers/hwmon/occ/Makefile | 6 ++++-- > drivers/hwmon/occ/common.c | 7 +++++++ > drivers/hwmon/occ/sysfs.c | 2 ++ > 4 files changed, 14 insertions(+), 4 deletions(-) > > --- linux-5.0.orig/drivers/hwmon/occ/Kconfig 2019-04-10 11:30:05.579537638 +0200 > +++ linux-5.0/drivers/hwmon/occ/Kconfig 2019-04-10 11:31:20.843383376 +0200 > @@ -27,5 +27,4 @@ config SENSORS_OCC_P9_SBE > called occ-p9-hwmon. > > config SENSORS_OCC > - bool "POWER On-Chip Controller" > - depends on SENSORS_OCC_P8_I2C || SENSORS_OCC_P9_SBE > + tristate > --- linux-5.0.orig/drivers/hwmon/occ/Makefile 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/Makefile 2019-04-10 11:33:23.631765535 +0200 > @@ -1,5 +1,7 @@ > -occ-p8-hwmon-objs := common.o sysfs.o p8_i2c.o > -occ-p9-hwmon-objs := common.o sysfs.o p9_sbe.o > +occ-hwmon-common-objs := common.o sysfs.o > +occ-p8-hwmon-objs := p8_i2c.o > +occ-p9-hwmon-objs := p9_sbe.o > > +obj-$(CONFIG_SENSORS_OCC) += occ-hwmon-common.o > obj-$(CONFIG_SENSORS_OCC_P8_I2C) += occ-p8-hwmon.o > obj-$(CONFIG_SENSORS_OCC_P9_SBE) += occ-p9-hwmon.o > --- linux-5.0.orig/drivers/hwmon/occ/common.c 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/common.c 2019-04-10 11:44:53.035573580 +0200 > @@ -1,11 +1,13 @@ > // SPDX-License-Identifier: GPL-2.0 > > #include <linux/device.h> > +#include <linux/export.h> > #include <linux/hwmon.h> > #include <linux/hwmon-sysfs.h> > #include <linux/jiffies.h> > #include <linux/kernel.h> > #include <linux/math64.h> > +#include <linux/module.h> > #include <linux/mutex.h> > #include <linux/sysfs.h> > #include <asm/unaligned.h> > @@ -1096,3 +1098,8 @@ int occ_setup(struct occ *occ, const cha > > return rc; > } > +EXPORT_SYMBOL_GPL(occ_setup); > + > +MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>"); > +MODULE_DESCRIPTION("Common OCC hwmon code"); > +MODULE_LICENSE("GPL"); > --- linux-5.0.orig/drivers/hwmon/occ/sysfs.c 2019-03-04 00:21:29.000000000 +0100 > +++ linux-5.0/drivers/hwmon/occ/sysfs.c 2019-04-10 11:39:38.627003382 +0200 > @@ -12,6 +12,7 @@ > > #include <linux/bitops.h> > #include <linux/device.h> > +#include <linux/export.h> > #include <linux/hwmon-sysfs.h> > #include <linux/kernel.h> > #include <linux/sysfs.h> > @@ -186,3 +187,4 @@ void occ_shutdown(struct occ *occ) > { > sysfs_remove_group(&occ->bus_dev->kobj, &occ_sysfs); > } > +EXPORT_SYMBOL_GPL(occ_shutdown); > > > -- > Jean Delvare > SUSE L3 Support
On Wed, Apr 10, 2019 at 11:21:47AM -0700, Guenter Roeck wrote: > Jean, > > On Wed, Apr 10, 2019 at 12:47:26PM +0200, Jean Delvare wrote: > > Instead of duplicating the common code into the 2 (binary) drivers, > > move the common code to a separate module. This is cleaner. > > > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > Cc: Eddie James <eajames@linux.ibm.com> > > Cc: Guenter Roeck <linux@roeck-us.net> > > what is the parent release for this patch ? I tried to apply it to mainline > and to hwmon-next using git am, but both failed. > Never mind, found it - v5.0. Both patches applied to hwmon-next. Thanks, Guenter > Thanks, > Guenter > > > --- > > Eddie, can you please give it a try and confirm it works? > > > > Note: I kept the module names as they were before, hence the extra > > "*-objs :=" statements. They could be removed if we rename the source > > files, but that's better done in git directly. I don't mind either way > > personally. > > > > drivers/hwmon/occ/Kconfig | 3 +-- > > drivers/hwmon/occ/Makefile | 6 ++++-- > > drivers/hwmon/occ/common.c | 7 +++++++ > > drivers/hwmon/occ/sysfs.c | 2 ++ > > 4 files changed, 14 insertions(+), 4 deletions(-) > > > > --- linux-5.0.orig/drivers/hwmon/occ/Kconfig 2019-04-10 11:30:05.579537638 +0200 > > +++ linux-5.0/drivers/hwmon/occ/Kconfig 2019-04-10 11:31:20.843383376 +0200 > > @@ -27,5 +27,4 @@ config SENSORS_OCC_P9_SBE > > called occ-p9-hwmon. > > > > config SENSORS_OCC > > - bool "POWER On-Chip Controller" > > - depends on SENSORS_OCC_P8_I2C || SENSORS_OCC_P9_SBE > > + tristate > > --- linux-5.0.orig/drivers/hwmon/occ/Makefile 2019-03-04 00:21:29.000000000 +0100 > > +++ linux-5.0/drivers/hwmon/occ/Makefile 2019-04-10 11:33:23.631765535 +0200 > > @@ -1,5 +1,7 @@ > > -occ-p8-hwmon-objs := common.o sysfs.o p8_i2c.o > > -occ-p9-hwmon-objs := common.o sysfs.o p9_sbe.o > > +occ-hwmon-common-objs := common.o sysfs.o > > +occ-p8-hwmon-objs := p8_i2c.o > > +occ-p9-hwmon-objs := p9_sbe.o > > > > +obj-$(CONFIG_SENSORS_OCC) += occ-hwmon-common.o > > obj-$(CONFIG_SENSORS_OCC_P8_I2C) += occ-p8-hwmon.o > > obj-$(CONFIG_SENSORS_OCC_P9_SBE) += occ-p9-hwmon.o > > --- linux-5.0.orig/drivers/hwmon/occ/common.c 2019-03-04 00:21:29.000000000 +0100 > > +++ linux-5.0/drivers/hwmon/occ/common.c 2019-04-10 11:44:53.035573580 +0200 > > @@ -1,11 +1,13 @@ > > // SPDX-License-Identifier: GPL-2.0 > > > > #include <linux/device.h> > > +#include <linux/export.h> > > #include <linux/hwmon.h> > > #include <linux/hwmon-sysfs.h> > > #include <linux/jiffies.h> > > #include <linux/kernel.h> > > #include <linux/math64.h> > > +#include <linux/module.h> > > #include <linux/mutex.h> > > #include <linux/sysfs.h> > > #include <asm/unaligned.h> > > @@ -1096,3 +1098,8 @@ int occ_setup(struct occ *occ, const cha > > > > return rc; > > } > > +EXPORT_SYMBOL_GPL(occ_setup); > > + > > +MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>"); > > +MODULE_DESCRIPTION("Common OCC hwmon code"); > > +MODULE_LICENSE("GPL"); > > --- linux-5.0.orig/drivers/hwmon/occ/sysfs.c 2019-03-04 00:21:29.000000000 +0100 > > +++ linux-5.0/drivers/hwmon/occ/sysfs.c 2019-04-10 11:39:38.627003382 +0200 > > @@ -12,6 +12,7 @@ > > > > #include <linux/bitops.h> > > #include <linux/device.h> > > +#include <linux/export.h> > > #include <linux/hwmon-sysfs.h> > > #include <linux/kernel.h> > > #include <linux/sysfs.h> > > @@ -186,3 +187,4 @@ void occ_shutdown(struct occ *occ) > > { > > sysfs_remove_group(&occ->bus_dev->kobj, &occ_sysfs); > > } > > +EXPORT_SYMBOL_GPL(occ_shutdown); > > > > > > -- > > Jean Delvare > > SUSE L3 Support
On Wed, 10 Apr 2019 13:02:32 -0500, Eddie James wrote: > On 4/10/19 5:47 AM, Jean Delvare wrote: > > Instead of duplicating the common code into the 2 (binary) drivers, > > move the common code to a separate module. This is cleaner. > > > > Signed-off-by: Jean Delvare <jdelvare@suse.de> > > Cc: Eddie James <eajames@linux.ibm.com> > > Cc: Guenter Roeck <linux@roeck-us.net> > > --- > > Eddie, can you please give it a try and confirm it works? > > > Yes, this works well. > > Reviewed-by: Eddie James <eajames@linux.ibm.com> > > Tested-by: Eddie James <eajames@linux.ibm.com> Thanks. Can you please send a patch on top of that updating the help texts in Kconfig to explain that the drivers are for the BMC and not for the PowerPC host OS, as discussed before? Thanks,
--- linux-5.0.orig/drivers/hwmon/occ/Kconfig 2019-04-10 11:30:05.579537638 +0200 +++ linux-5.0/drivers/hwmon/occ/Kconfig 2019-04-10 11:31:20.843383376 +0200 @@ -27,5 +27,4 @@ config SENSORS_OCC_P9_SBE called occ-p9-hwmon. config SENSORS_OCC - bool "POWER On-Chip Controller" - depends on SENSORS_OCC_P8_I2C || SENSORS_OCC_P9_SBE + tristate --- linux-5.0.orig/drivers/hwmon/occ/Makefile 2019-03-04 00:21:29.000000000 +0100 +++ linux-5.0/drivers/hwmon/occ/Makefile 2019-04-10 11:33:23.631765535 +0200 @@ -1,5 +1,7 @@ -occ-p8-hwmon-objs := common.o sysfs.o p8_i2c.o -occ-p9-hwmon-objs := common.o sysfs.o p9_sbe.o +occ-hwmon-common-objs := common.o sysfs.o +occ-p8-hwmon-objs := p8_i2c.o +occ-p9-hwmon-objs := p9_sbe.o +obj-$(CONFIG_SENSORS_OCC) += occ-hwmon-common.o obj-$(CONFIG_SENSORS_OCC_P8_I2C) += occ-p8-hwmon.o obj-$(CONFIG_SENSORS_OCC_P9_SBE) += occ-p9-hwmon.o --- linux-5.0.orig/drivers/hwmon/occ/common.c 2019-03-04 00:21:29.000000000 +0100 +++ linux-5.0/drivers/hwmon/occ/common.c 2019-04-10 11:44:53.035573580 +0200 @@ -1,11 +1,13 @@ // SPDX-License-Identifier: GPL-2.0 #include <linux/device.h> +#include <linux/export.h> #include <linux/hwmon.h> #include <linux/hwmon-sysfs.h> #include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/math64.h> +#include <linux/module.h> #include <linux/mutex.h> #include <linux/sysfs.h> #include <asm/unaligned.h> @@ -1096,3 +1098,8 @@ int occ_setup(struct occ *occ, const cha return rc; } +EXPORT_SYMBOL_GPL(occ_setup); + +MODULE_AUTHOR("Eddie James <eajames@linux.ibm.com>"); +MODULE_DESCRIPTION("Common OCC hwmon code"); +MODULE_LICENSE("GPL"); --- linux-5.0.orig/drivers/hwmon/occ/sysfs.c 2019-03-04 00:21:29.000000000 +0100 +++ linux-5.0/drivers/hwmon/occ/sysfs.c 2019-04-10 11:39:38.627003382 +0200 @@ -12,6 +12,7 @@ #include <linux/bitops.h> #include <linux/device.h> +#include <linux/export.h> #include <linux/hwmon-sysfs.h> #include <linux/kernel.h> #include <linux/sysfs.h> @@ -186,3 +187,4 @@ void occ_shutdown(struct occ *occ) { sysfs_remove_group(&occ->bus_dev->kobj, &occ_sysfs); } +EXPORT_SYMBOL_GPL(occ_shutdown);
Instead of duplicating the common code into the 2 (binary) drivers, move the common code to a separate module. This is cleaner. Signed-off-by: Jean Delvare <jdelvare@suse.de> Cc: Eddie James <eajames@linux.ibm.com> Cc: Guenter Roeck <linux@roeck-us.net> --- Eddie, can you please give it a try and confirm it works? Note: I kept the module names as they were before, hence the extra "*-objs :=" statements. They could be removed if we rename the source files, but that's better done in git directly. I don't mind either way personally. drivers/hwmon/occ/Kconfig | 3 +-- drivers/hwmon/occ/Makefile | 6 ++++-- drivers/hwmon/occ/common.c | 7 +++++++ drivers/hwmon/occ/sysfs.c | 2 ++ 4 files changed, 14 insertions(+), 4 deletions(-)