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;
+
 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
-- 
2.43.0

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

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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>