Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *
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 > 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
Re: [PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *
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 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 *
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 > 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 *
On Wed, Jan 11, 2023 at 5:30 AM Greg Kroah-Hartman 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
[PATCH v2 01/16] of: device: make of_device_uevent_modalias() take a const device *
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
Cc: Nicholas Piggin
Cc: Christophe Leroy
Cc: Chen-Yu Tsai
Cc: Jernej Skrabec
Cc: Samuel Holland
Cc: David Airlie
Cc: Daniel Vetter
Cc: Benjamin Herrenschmidt
Cc: Rob Herring
Cc: Frank Rowand
Cc: Liang He
Cc: Greg Kroah-Hartman
Cc: Thomas Gleixner
Cc: Christophe JAILLET
Cc: Thomas Zimmermann
Cc: Dmitry Baryshkov
Cc: Douglas Anderson
Cc: Lyude Paul
Cc: Corentin Labbe
Cc: Zou Wei
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-ker...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-su...@lists.linux.dev
Cc: dri-de...@lists.freedesktop.org
Cc: devicet...@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman
---
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
