[PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[Greg Kroah-Hartman]

of_device_uevent_modalias() does not modify the device pointer passed to
it, so mark it constant.  In order to properly do this, a number of
busses need to have a modalias function added as they were attempting to
just point to of_device_uevent_modalias instead of their bus-specific
modalias function.  This is fine except if the prototype for a bus and
device type modalias function diverges and then problems could happen.  To
prevent all of that, just wrap the call to of_device_uevent_modalias()
directly for each bus and device type individually.

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Chen-Yu Tsai <wens@csie.org>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Cc: Samuel Holland <samuel@sholland.org>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Liang He <windhl@126.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Lyude Paul <lyude@redhat.com>
Cc: Corentin Labbe <clabbe@baylibre.com>
Cc: Zou Wei <zou_wei@huawei.com>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: dri-devel@lists.freedesktop.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/powerpc/platforms/pseries/ibmebus.c | 7 ++++++-
 drivers/bus/sunxi-rsb.c                  | 7 ++++++-
 drivers/gpu/drm/display/drm_dp_aux_bus.c | 7 ++++++-
 drivers/macintosh/macio_asic.c           | 7 ++++++-
 drivers/of/device.c                      | 4 ++--
 include/linux/of_device.h                | 4 ++--
 6 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/ibmebus.c b/arch/powerpc/platforms/pseries/ibmebus.c
index a870cada7acd..58b798a0e879 100644
--- a/arch/powerpc/platforms/pseries/ibmebus.c
+++ b/arch/powerpc/platforms/pseries/ibmebus.c
@@ -426,9 +426,14 @@ static struct attribute *ibmebus_bus_device_attrs[] = {
 };
 ATTRIBUTE_GROUPS(ibmebus_bus_device);
 
+static int ibmebus_bus_modalias(struct device *dev, struct kobj_uevent_env *env)
+{
+	return of_device_uevent_modalias(dev, env);
+}
+
 struct bus_type ibmebus_bus_type = {
 	.name      = "ibmebus",
-	.uevent    = of_device_uevent_modalias,
+	.uevent    = ibmebus_bus_modalias,
 	.bus_groups = ibmbus_bus_groups,
 	.match     = ibmebus_bus_bus_match,
 	.probe     = ibmebus_bus_device_probe,
diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index 3aa91aed3bf7..a180af11e034 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -172,12 +172,17 @@ static void sunxi_rsb_device_remove(struct device *dev)
 	drv->remove(to_sunxi_rsb_device(dev));
 }
 
+static int sunxi_rsb_device_modalias(struct device *dev, struct kobj_uevent_env *env)
+{
+	return of_device_uevent_modalias(dev, env);
+}
+
 static struct bus_type sunxi_rsb_bus = {
 	.name		= RSB_CTRL_NAME,
 	.match		= sunxi_rsb_device_match,
 	.probe		= sunxi_rsb_device_probe,
 	.remove		= sunxi_rsb_device_remove,
-	.uevent		= of_device_uevent_modalias,
+	.uevent		= sunxi_rsb_device_modalias,
 };
 
 static void sunxi_rsb_dev_release(struct device *dev)
diff --git a/drivers/gpu/drm/display/drm_dp_aux_bus.c b/drivers/gpu/drm/display/drm_dp_aux_bus.c
index f5741b45ca07..e31a0261c53e 100644
--- a/drivers/gpu/drm/display/drm_dp_aux_bus.c
+++ b/drivers/gpu/drm/display/drm_dp_aux_bus.c
@@ -161,9 +161,14 @@ static void dp_aux_ep_dev_release(struct device *dev)
 	kfree(aux_ep_with_data);
 }
 
+static int dp_aux_ep_dev_modalias(struct device *dev, struct kobj_uevent_env *env)
+{
+	return of_device_uevent_modalias(dev, env);
+}
+
 static struct device_type dp_aux_device_type_type = {
 	.groups		= dp_aux_ep_dev_groups,
-	.uevent		= of_device_uevent_modalias,
+	.uevent		= dp_aux_ep_dev_modalias,
 	.release	= dp_aux_ep_dev_release,
 };
 
diff --git a/drivers/macintosh/macio_asic.c b/drivers/macintosh/macio_asic.c
index 3bc1f374e657..7f2b0107c733 100644
--- a/drivers/macintosh/macio_asic.c
+++ b/drivers/macintosh/macio_asic.c
@@ -128,12 +128,17 @@ static int macio_device_resume(struct device * dev)
 	return 0;
 }
 
+static int macio_device_modalias(struct device *dev, struct kobj_uevent_env *env)
+{
+	return of_device_uevent_modalias(dev, env);
+}
+
 extern const struct attribute_group *macio_dev_groups[];
 
 struct bus_type macio_bus_type = {
        .name	= "macio",
        .match	= macio_bus_match,
-       .uevent = of_device_uevent_modalias,
+       .uevent	= macio_device_modalias,
        .probe	= macio_device_probe,
        .remove	= macio_device_remove,
        .shutdown = macio_device_shutdown,
diff --git a/drivers/of/device.c b/drivers/of/device.c
index c674a13c3055..dda51b7ce597 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -248,7 +248,7 @@ const void *of_device_get_match_data(const struct device *dev)
 }
 EXPORT_SYMBOL(of_device_get_match_data);
 
-static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len)
+static ssize_t of_device_get_modalias(const struct device *dev, char *str, ssize_t len)
 {
 	const char *compat;
 	char *c;
@@ -372,7 +372,7 @@ void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	mutex_unlock(&of_mutex);
 }
 
-int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
+int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env)
 {
 	int sl;
 
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index ab7d557d541d..f4b57614979d 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -36,7 +36,7 @@ extern ssize_t of_device_modalias(struct device *dev, char *str, ssize_t len);
 extern int of_device_request_module(struct device *dev);
 
 extern void of_device_uevent(const struct device *dev, struct kobj_uevent_env *env);
-extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern int of_device_uevent_modalias(const struct device *dev, struct kobj_uevent_env *env);
 
 static inline struct device_node *of_cpu_device_node_get(int cpu)
 {
@@ -83,7 +83,7 @@ static inline int of_device_request_module(struct device *dev)
 	return -ENODEV;
 }
 
-static inline int of_device_uevent_modalias(struct device *dev,
+static inline int of_device_uevent_modalias(const struct device *dev,
 				   struct kobj_uevent_env *env)
 {
 	return -ENODEV;
-- 
2.39.0

Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[Rob Herring]

On Wed, Jan 11, 2023 at 5:30 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> of_device_uevent_modalias() does not modify the device pointer passed to
> it, so mark it constant.  In order to properly do this, a number of
> busses need to have a modalias function added as they were attempting to
> just point to of_device_uevent_modalias instead of their bus-specific
> modalias function.  This is fine except if the prototype for a bus and
> device type modalias function diverges and then problems could happen.  To
> prevent all of that, just wrap the call to of_device_uevent_modalias()
> directly for each bus and device type individually.

Why not just put the wrapper function in the DT code instead of making
4 copies of it?

Rob

Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[Greg Kroah-Hartman]

On Wed, Jan 11, 2023 at 08:54:04AM -0600, Rob Herring wrote:
> On Wed, Jan 11, 2023 at 5:30 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > of_device_uevent_modalias() does not modify the device pointer passed to
> > it, so mark it constant.  In order to properly do this, a number of
> > busses need to have a modalias function added as they were attempting to
> > just point to of_device_uevent_modalias instead of their bus-specific
> > modalias function.  This is fine except if the prototype for a bus and
> > device type modalias function diverges and then problems could happen.  To
> > prevent all of that, just wrap the call to of_device_uevent_modalias()
> > directly for each bus and device type individually.
> 
> Why not just put the wrapper function in the DT code instead of making
> 4 copies of it?

I could, if you think that it would be better there instead of in each
individual bus (like all of the other bus callbacks).  This way each bus
"owns" their implementation :)

thanks,

greg k-h

Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[Dmitry Baryshkov]

On 11/01/2023 17:26, Greg Kroah-Hartman wrote:
> On Wed, Jan 11, 2023 at 08:54:04AM -0600, Rob Herring wrote:
>> On Wed, Jan 11, 2023 at 5:30 AM Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> of_device_uevent_modalias() does not modify the device pointer passed to
>>> it, so mark it constant.  In order to properly do this, a number of
>>> busses need to have a modalias function added as they were attempting to
>>> just point to of_device_uevent_modalias instead of their bus-specific
>>> modalias function.  This is fine except if the prototype for a bus and
>>> device type modalias function diverges and then problems could happen.  To
>>> prevent all of that, just wrap the call to of_device_uevent_modalias()
>>> directly for each bus and device type individually.
>>
>> Why not just put the wrapper function in the DT code instead of making
>> 4 copies of it?
> 
> I could, if you think that it would be better there instead of in each
> individual bus (like all of the other bus callbacks).  This way each bus
> "owns" their implementation :)



I'd vote for the generic wrapper instead of 4 similar wrapper. In the 
end, if of_device_uevent_modalias (or the bus callback) interface 
changes again for whatever reasons, there will be just a single place to 
fix rather than fixing 4 (or more) bus drivers.
-- 
With best wishes
Dmitry

Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *

[Greg Kroah-Hartman]

On Wed, Jan 11, 2023 at 08:54:04AM -0600, Rob Herring wrote:
> On Wed, Jan 11, 2023 at 5:30 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > of_device_uevent_modalias() does not modify the device pointer passed to
> > it, so mark it constant.  In order to properly do this, a number of
> > busses need to have a modalias function added as they were attempting to
> > just point to of_device_uevent_modalias instead of their bus-specific
> > modalias function.  This is fine except if the prototype for a bus and
> > device type modalias function diverges and then problems could happen.  To
> > prevent all of that, just wrap the call to of_device_uevent_modalias()
> > directly for each bus and device type individually.
> 
> Why not just put the wrapper function in the DT code instead of making
> 4 copies of it?

Ok, I looked at doing this today, but in the end, making "4" copies of
this is simpler overall.  To do it your way would require a "const"
version of the function be added to the core, and then convert these 4
busses to use that, and then when the real function is converted to be
const, move all of these functions back over to use that again.

Lots of churn, and then in the end, we still have the mismatch of a
the same function callback being used in two different types of
callbacks (one a bus, one a class).  This way we separate them to make
things much more obvious and self-contained.

So I'll keep this as-is for now, thanks.

greg k-h

[PATCH v2 08/16] vio: move to_vio_dev() to use container_of_const()

[Greg Kroah-Hartman]

The driver core is changing to pass some pointers as const, so move
to_vio_dev() to use container_of_const() to handle this change.
to_vio_dev() now properly keeps the const-ness of the pointer passed
into it, while as before it could be lost.

Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-kernel@vger.kernel.org
Cc: sparclinux@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/powerpc/include/asm/vio.h | 5 +----
 arch/sparc/include/asm/vio.h   | 5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/include/asm/vio.h b/arch/powerpc/include/asm/vio.h
index e7479a4abf96..cc9b787627ad 100644
--- a/arch/powerpc/include/asm/vio.h
+++ b/arch/powerpc/include/asm/vio.h
@@ -161,10 +161,7 @@ static inline struct vio_driver *to_vio_driver(struct device_driver *drv)
 	return container_of(drv, struct vio_driver, driver);
 }
 
-static inline struct vio_dev *to_vio_dev(struct device *dev)
-{
-	return container_of(dev, struct vio_dev, dev);
-}
+#define to_vio_dev(__dev)	container_of_const(__dev, struct vio_dev, dev)
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_VIO_H */
diff --git a/arch/sparc/include/asm/vio.h b/arch/sparc/include/asm/vio.h
index 2d7bdf665fd3..8a0c3c11c9ce 100644
--- a/arch/sparc/include/asm/vio.h
+++ b/arch/sparc/include/asm/vio.h
@@ -488,10 +488,7 @@ static inline struct vio_driver *to_vio_driver(struct device_driver *drv)
 	return container_of(drv, struct vio_driver, driver);
 }
 
-static inline struct vio_dev *to_vio_dev(struct device *dev)
-{
-	return container_of(dev, struct vio_dev, dev);
-}
+#define to_vio_dev(__dev)	container_of_const(__dev, struct vio_dev, dev)
 
 int vio_ldc_send(struct vio_driver_state *vio, void *data, int len);
 void vio_link_state_change(struct vio_driver_state *vio, int event);
-- 
2.39.0