| Message ID | 2023121905-idiom-opossum-1ba3@gregkh (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [ARM] locomo: make locomo_bus_type constant and static | expand |
On Tue, Dec 19, 2023, at 18:33, Greg Kroah-Hartman wrote: > Now that the driver core can properly handle constant struct bus_type, > move the locomo_bus_type variable to be a constant structure as well, > placing it into read-only memory which can not be modified at runtime. > > It's also never used outside of arch/arm/common/locomo.c so make it > static and don't export it as no one is using it. Reviewed-by: Arnd Bergmann <arnd@arndb.de>
On Tue, Dec 19, 2023 at 07:33:06PM +0100, Greg Kroah-Hartman wrote: > Now that the driver core can properly handle constant struct bus_type, > move the locomo_bus_type variable to be a constant structure as well, > placing it into read-only memory which can not be modified at runtime. > > It's also never used outside of arch/arm/common/locomo.c so make it > static and don't export it as no one is using it. > > Cc: Russell King <linux@armlinux.org.uk> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > arch/arm/common/locomo.c | 4 +++- > arch/arm/include/asm/hardware/locomo.h | 2 -- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c > index 70480dd9e96d..6d0c9f7268ba 100644 > --- a/arch/arm/common/locomo.c > +++ b/arch/arm/common/locomo.c > @@ -68,6 +68,8 @@ struct locomo { > #endif > }; > > +static const struct bus_type locomo_bus_type; > + If you move up locomo_bus_type together with its three callbacks before locomo_init_one_child, you don't need the extra declaration here. That would be: diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c index 6d0c9f7268ba..676e98802561 100644 --- a/arch/arm/common/locomo.c +++ b/arch/arm/common/locomo.c @@ -68,8 +68,6 @@ struct locomo { #endif }; -static const struct bus_type locomo_bus_type; - struct locomo_dev_info { unsigned long offset; unsigned long length; @@ -216,6 +214,47 @@ static void locomo_dev_release(struct device *_dev) kfree(dev); } +/* + * LoCoMo "Register Access Bus." + * + * We model this as a regular bus type, and hang devices directly + * off this. + */ +static int locomo_match(struct device *_dev, struct device_driver *_drv) +{ + struct locomo_dev *dev = LOCOMO_DEV(_dev); + struct locomo_driver *drv = LOCOMO_DRV(_drv); + + return dev->devid == drv->devid; +} + +static int locomo_bus_probe(struct device *dev) +{ + struct locomo_dev *ldev = LOCOMO_DEV(dev); + struct locomo_driver *drv = LOCOMO_DRV(dev->driver); + int ret = -ENODEV; + + if (drv->probe) + ret = drv->probe(ldev); + return ret; +} + +static void locomo_bus_remove(struct device *dev) +{ + struct locomo_dev *ldev = LOCOMO_DEV(dev); + struct locomo_driver *drv = LOCOMO_DRV(dev->driver); + + if (drv->remove) + drv->remove(ldev); +} + +static const struct bus_type locomo_bus_type = { + .name = "locomo-bus", + .match = locomo_match, + .probe = locomo_bus_probe, + .remove = locomo_bus_remove, +}; + static int locomo_init_one_child(struct locomo *lchip, struct locomo_dev_info *info) { @@ -810,47 +849,6 @@ void locomo_frontlight_set(struct locomo_dev *dev, int duty, int vr, int bpwf) } EXPORT_SYMBOL(locomo_frontlight_set); -/* - * LoCoMo "Register Access Bus." - * - * We model this as a regular bus type, and hang devices directly - * off this. - */ -static int locomo_match(struct device *_dev, struct device_driver *_drv) -{ - struct locomo_dev *dev = LOCOMO_DEV(_dev); - struct locomo_driver *drv = LOCOMO_DRV(_drv); - - return dev->devid == drv->devid; -} - -static int locomo_bus_probe(struct device *dev) -{ - struct locomo_dev *ldev = LOCOMO_DEV(dev); - struct locomo_driver *drv = LOCOMO_DRV(dev->driver); - int ret = -ENODEV; - - if (drv->probe) - ret = drv->probe(ldev); - return ret; -} - -static void locomo_bus_remove(struct device *dev) -{ - struct locomo_dev *ldev = LOCOMO_DEV(dev); - struct locomo_driver *drv = LOCOMO_DRV(dev->driver); - - if (drv->remove) - drv->remove(ldev); -} - -static const struct bus_type locomo_bus_type = { - .name = "locomo-bus", - .match = locomo_match, - .probe = locomo_bus_probe, - .remove = locomo_bus_remove, -}; - int locomo_driver_register(struct locomo_driver *driver) { driver->drv.bus = &locomo_bus_type; Feel free to squash this into your patch for a v2 if you like it. Or is that better done as a separate change on top of yours? Otherwise looks fine to me. Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> Best regards Uwe
On Tue, Dec 19, 2023 at 10:21:20PM +0100, Uwe Kleine-König wrote: > On Tue, Dec 19, 2023 at 07:33:06PM +0100, Greg Kroah-Hartman wrote: > > Now that the driver core can properly handle constant struct bus_type, > > move the locomo_bus_type variable to be a constant structure as well, > > placing it into read-only memory which can not be modified at runtime. > > > > It's also never used outside of arch/arm/common/locomo.c so make it > > static and don't export it as no one is using it. > > > > Cc: Russell King <linux@armlinux.org.uk> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> > > Cc: Randy Dunlap <rdunlap@infradead.org> > > Cc: linux-arm-kernel@lists.infradead.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > arch/arm/common/locomo.c | 4 +++- > > arch/arm/include/asm/hardware/locomo.h | 2 -- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c > > index 70480dd9e96d..6d0c9f7268ba 100644 > > --- a/arch/arm/common/locomo.c > > +++ b/arch/arm/common/locomo.c > > @@ -68,6 +68,8 @@ struct locomo { > > #endif > > }; > > > > +static const struct bus_type locomo_bus_type; > > + > > If you move up locomo_bus_type together with its three callbacks before > locomo_init_one_child, you don't need the extra declaration here. I was trying to go for "least intrusive and most obvious change possible" here, given that this file hasn't been touched in over a decade by anyone else for anything other than api changes. And even then, it was just a tiny fix, this is a very old driver, what's the odds that it's even used anymore? thanks, greg k-h
diff --git a/arch/arm/common/locomo.c b/arch/arm/common/locomo.c index 70480dd9e96d..6d0c9f7268ba 100644 --- a/arch/arm/common/locomo.c +++ b/arch/arm/common/locomo.c @@ -68,6 +68,8 @@ struct locomo { #endif }; +static const struct bus_type locomo_bus_type; + struct locomo_dev_info { unsigned long offset; unsigned long length; @@ -842,7 +844,7 @@ static void locomo_bus_remove(struct device *dev) drv->remove(ldev); } -struct bus_type locomo_bus_type = { +static const struct bus_type locomo_bus_type = { .name = "locomo-bus", .match = locomo_match, .probe = locomo_bus_probe, diff --git a/arch/arm/include/asm/hardware/locomo.h b/arch/arm/include/asm/hardware/locomo.h index aaaedafef7cc..9fd9ad5d9202 100644 --- a/arch/arm/include/asm/hardware/locomo.h +++ b/arch/arm/include/asm/hardware/locomo.h @@ -158,8 +158,6 @@ #define LOCOMO_LPT_TOH(TOH) ((TOH & 0x7) << 4) #define LOCOMO_LPT_TOL(TOL) ((TOL & 0x7)) -extern struct bus_type locomo_bus_type; - #define LOCOMO_DEVID_KEYBOARD 0 #define LOCOMO_DEVID_FRONTLIGHT 1 #define LOCOMO_DEVID_BACKLIGHT 2
Now that the driver core can properly handle constant struct bus_type, move the locomo_bus_type variable to be a constant structure as well, placing it into read-only memory which can not be modified at runtime. It's also never used outside of arch/arm/common/locomo.c so make it static and don't export it as no one is using it. Cc: Russell King <linux@armlinux.org.uk> Cc: Arnd Bergmann <arnd@arndb.de> Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- arch/arm/common/locomo.c | 4 +++- arch/arm/include/asm/hardware/locomo.h | 2 -- 2 files changed, 3 insertions(+), 3 deletions(-)