| Message ID | 20230622213214.3586530-3-miquel.raynal@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Small of/device cleanup | expand |
On Thu, Jun 22, 2023 at 11:32:14PM +0200, Miquel Raynal wrote: > There is apparently no reasons to open-code of_device_uevent() besides: > - The helper receives a struct device while we want to use the of_node > member of the struct device *parent*. > - of_device_uevent() could not be called by modules because of a missing > EXPORT_SYMBOL*(). > > In practice, the former point is not very constraining, just calling > of_device_uevent(dev->parent, ...) would have made the trick. Yeah, looks like that's correct. I guess I always thought this information would get added to the sysfs node of the struct device * that was passed in, while it actually gets passed to the environment created for the struct device passed into the caller. In other words what we pass to of_device_uevent() here is only ever used as a source of information, so passing dev->parent works. I've also verified this on Tegra30 Beaver just to make sure and it looks like the generated uevent file is identical before and after this patch. Applied to drm-misc, thanks. Thierry
diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c index 4d16a3396c4a..84d042796d2e 100644 --- a/drivers/gpu/host1x/bus.c +++ b/drivers/gpu/host1x/bus.c @@ -338,32 +338,15 @@ static int host1x_device_match(struct device *dev, struct device_driver *drv) return strcmp(dev_name(dev), drv->name) == 0; } +/* + * Note that this is really only needed for backwards compatibility + * with libdrm, which parses this information from sysfs and will + * fail if it can't find the OF_FULLNAME, specifically. + */ static int host1x_device_uevent(const struct device *dev, struct kobj_uevent_env *env) { - struct device_node *np = dev->parent->of_node; - unsigned int count = 0; - struct property *p; - const char *compat; - - /* - * This duplicates most of of_device_uevent(), but the latter cannot - * be called from modules and operates on dev->of_node, which is not - * available in this case. - * - * Note that this is really only needed for backwards compatibility - * with libdrm, which parses this information from sysfs and will - * fail if it can't find the OF_FULLNAME, specifically. - */ - add_uevent_var(env, "OF_NAME=%pOFn", np); - add_uevent_var(env, "OF_FULLNAME=%pOF", np); - - of_property_for_each_string(np, "compatible", p, compat) { - add_uevent_var(env, "OF_COMPATIBLE_%u=%s", count, compat); - count++; - } - - add_uevent_var(env, "OF_COMPATIBLE_N=%u", count); + of_device_uevent(dev->parent, env); return 0; }
There is apparently no reasons to open-code of_device_uevent() besides: - The helper receives a struct device while we want to use the of_node member of the struct device *parent*. - of_device_uevent() could not be called by modules because of a missing EXPORT_SYMBOL*(). In practice, the former point is not very constraining, just calling of_device_uevent(dev->parent, ...) would have made the trick. The latter point is more an observation rather than a real blocking point because nothing prevented of_uevent() (called by the inline function of_device_uevent()) to be exported to modules. In practice, this helper is now exported, so nothing prevent us from using of_device_uevent() anymore. Let's use the core helper directly instead of open-coding it. Cc: Thierry Reding <thierry.reding@gmail.com> Cc: David Airlie <airlied@gmail.com> Cc: Daniel Vetter <daniel@ffwll.ch> Cc: Mikko Perttunen <mperttunen@nvidia.com> Cc: Rob Herring <robh+dt@kernel.org> Cc: Frank Rowand <frowand.list@gmail.com> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- drivers/gpu/host1x/bus.c | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-)