Messages in this thread | | | Date | Sat, 24 Feb 2024 09:43:30 -0600 | Subject | Re: [PATCH] greybus: constify the struct device_type usage | From | Alex Elder <> |
| |
On 2/19/24 6:40 AM, Ricardo B. Marliere wrote: > Since commit aed65af1cc2f ("drivers: make device_type const"), the driver > core can properly handle constant struct device_type. Move the > greybus_hd_type, greybus_module_type, greybus_interface_type, > greybus_control_type, greybus_bundle_type and greybus_svc_type variables to > be constant structures as well, placing it into read-only memory which can > not be modified at runtime. > > Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
This looks good to me. Assuming it compiles cleanly:
Reviewed-by: Alex Elder <elder@linaro.org>
On another subject:
Johan might disagree, but I think it would be nice to make the definitions of the Greybus device types as static (private) and make the is_gb_host_device() etc. functions real functions rather than static inlines in <linux/greybus.h>.
It turns out that all of the is_gb_*() functions are called only from drivers/greybus/core.c; they could all be moved there rather than advertising them in <linux/greybus.h>.
-Alex
> --- > drivers/greybus/bundle.c | 2 +- > drivers/greybus/control.c | 2 +- > drivers/greybus/hd.c | 2 +- > drivers/greybus/interface.c | 2 +- > drivers/greybus/module.c | 2 +- > drivers/greybus/svc.c | 2 +- > include/linux/greybus.h | 12 ++++++------ > 7 files changed, 12 insertions(+), 12 deletions(-) > > diff --git a/drivers/greybus/bundle.c b/drivers/greybus/bundle.c > index 84660729538b..a6e1cca06172 100644 > --- a/drivers/greybus/bundle.c > +++ b/drivers/greybus/bundle.c > @@ -166,7 +166,7 @@ static const struct dev_pm_ops gb_bundle_pm_ops = { > SET_RUNTIME_PM_OPS(gb_bundle_suspend, gb_bundle_resume, gb_bundle_idle) > }; > > -struct device_type greybus_bundle_type = { > +const struct device_type greybus_bundle_type = { > .name = "greybus_bundle", > .release = gb_bundle_release, > .pm = &gb_bundle_pm_ops, > diff --git a/drivers/greybus/control.c b/drivers/greybus/control.c > index 359a25841973..b5cf49d09df2 100644 > --- a/drivers/greybus/control.c > +++ b/drivers/greybus/control.c > @@ -436,7 +436,7 @@ static void gb_control_release(struct device *dev) > kfree(control); > } > > -struct device_type greybus_control_type = { > +const struct device_type greybus_control_type = { > .name = "greybus_control", > .release = gb_control_release, > }; > diff --git a/drivers/greybus/hd.c b/drivers/greybus/hd.c > index 72b21bf2d7d3..e2f3496bddc3 100644 > --- a/drivers/greybus/hd.c > +++ b/drivers/greybus/hd.c > @@ -116,7 +116,7 @@ static void gb_hd_release(struct device *dev) > kfree(hd); > } > > -struct device_type greybus_hd_type = { > +const struct device_type greybus_hd_type = { > .name = "greybus_host_device", > .release = gb_hd_release, > }; > diff --git a/drivers/greybus/interface.c b/drivers/greybus/interface.c > index 9ec949a438ef..a88dc701289c 100644 > --- a/drivers/greybus/interface.c > +++ b/drivers/greybus/interface.c > @@ -765,7 +765,7 @@ static const struct dev_pm_ops gb_interface_pm_ops = { > gb_interface_runtime_idle) > }; > > -struct device_type greybus_interface_type = { > +const struct device_type greybus_interface_type = { > .name = "greybus_interface", > .release = gb_interface_release, > .pm = &gb_interface_pm_ops, > diff --git a/drivers/greybus/module.c b/drivers/greybus/module.c > index 36f77f9e1d74..7f7153a1dd60 100644 > --- a/drivers/greybus/module.c > +++ b/drivers/greybus/module.c > @@ -81,7 +81,7 @@ static void gb_module_release(struct device *dev) > kfree(module); > } > > -struct device_type greybus_module_type = { > +const struct device_type greybus_module_type = { > .name = "greybus_module", > .release = gb_module_release, > }; > diff --git a/drivers/greybus/svc.c b/drivers/greybus/svc.c > index 0d7e749174a4..4256467fcd35 100644 > --- a/drivers/greybus/svc.c > +++ b/drivers/greybus/svc.c > @@ -1305,7 +1305,7 @@ static void gb_svc_release(struct device *dev) > kfree(svc); > } > > -struct device_type greybus_svc_type = { > +const struct device_type greybus_svc_type = { > .name = "greybus_svc", > .release = gb_svc_release, > }; > diff --git a/include/linux/greybus.h b/include/linux/greybus.h > index 18c0fb958b74..5f9791fae3c0 100644 > --- a/include/linux/greybus.h > +++ b/include/linux/greybus.h > @@ -106,12 +106,12 @@ struct dentry *gb_debugfs_get(void); > > extern struct bus_type greybus_bus_type; > > -extern struct device_type greybus_hd_type; > -extern struct device_type greybus_module_type; > -extern struct device_type greybus_interface_type; > -extern struct device_type greybus_control_type; > -extern struct device_type greybus_bundle_type; > -extern struct device_type greybus_svc_type; > +extern const struct device_type greybus_hd_type; > +extern const struct device_type greybus_module_type; > +extern const struct device_type greybus_interface_type; > +extern const struct device_type greybus_control_type; > +extern const struct device_type greybus_bundle_type; > +extern const struct device_type greybus_svc_type; > > static inline int is_gb_host_device(const struct device *dev) > { > > --- > base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d > change-id: 20240219-device_cleanup-greybus-c97c1ef52458 > > Best regards,
| |