| Message ID | 20230717222526.229984-2-samsagax@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (oxp-sensors) Refactor probe() and init() and remove devm_add_groups() | expand |
On Mon, Jul 17, 2023 at 07:25:15PM -0300, Joaquín Ignacio Aramendía wrote: > A driver should not be manually adding groups in its probe function (it will > race with userspace), so replace the call to devm_device_add_groups() to use > the platform dev_groups callback instead. > > This will allow for removal of the devm_device_add_groups() function. > > Signed-off-by: Joaquín Ignacio Aramendía <samsagax@gmail.com> > --- > drivers/hwmon/oxp-sensors.c | 38 +++++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 14 deletions(-) Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Mon, Jul 17, 2023 at 07:25:15PM -0300, Joaquín Ignacio Aramendía wrote: > A driver should not be manually adding groups in its probe function (it will > race with userspace), so replace the call to devm_device_add_groups() to use > the platform dev_groups callback instead. > > This will allow for removal of the devm_device_add_groups() function. > > Signed-off-by: Joaquín Ignacio Aramendía <samsagax@gmail.com> > Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Applied. Thanks, Guenter > --- > drivers/hwmon/oxp-sensors.c | 38 +++++++++++++++++++++++-------------- > 1 file changed, 24 insertions(+), 14 deletions(-) > > diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c > index e1a907cae820..1e1cc67bcdea 100644 > --- a/drivers/hwmon/oxp-sensors.c > +++ b/drivers/hwmon/oxp-sensors.c > @@ -220,6 +220,20 @@ static int tt_toggle_disable(void) > } > > /* Callbacks for turbo toggle attribute */ > +static umode_t tt_toggle_is_visible(struct kobject *kobj, > + struct attribute *attr, int n) > +{ > + switch (board) { > + case aok_zoe_a1: > + case oxp_mini_amd_a07: > + case oxp_mini_amd_pro: > + return attr->mode; > + default: > + break; > + } > + return 0; > +} > + > static ssize_t tt_toggle_store(struct device *dev, > struct device_attribute *attr, const char *buf, > size_t count) > @@ -396,7 +410,15 @@ static struct attribute *oxp_ec_attrs[] = { > NULL > }; > > -ATTRIBUTE_GROUPS(oxp_ec); > +static struct attribute_group oxp_ec_attribute_group = { > + .is_visible = tt_toggle_is_visible, > + .attrs = oxp_ec_attrs, > +}; > + > +static const struct attribute_group *oxp_ec_groups[] = { > + &oxp_ec_attribute_group, > + NULL > +}; > > static const struct hwmon_ops oxp_ec_hwmon_ops = { > .is_visible = oxp_ec_hwmon_is_visible, > @@ -415,7 +437,6 @@ static int oxp_platform_probe(struct platform_device *pdev) > const struct dmi_system_id *dmi_entry; > struct device *dev = &pdev->dev; > struct device *hwdev; > - int ret; > > /* > * Have to check for AMD processor here because DMI strings are the > @@ -430,18 +451,6 @@ static int oxp_platform_probe(struct platform_device *pdev) > > board = (enum oxp_board)(unsigned long)dmi_entry->driver_data; > > - switch (board) { > - case aok_zoe_a1: > - case oxp_mini_amd_a07: > - case oxp_mini_amd_pro: > - ret = devm_device_add_groups(dev, oxp_ec_groups); > - if (ret) > - return ret; > - break; > - default: > - break; > - } > - > hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL, > &oxp_ec_chip_info, NULL); > > @@ -451,6 +460,7 @@ static int oxp_platform_probe(struct platform_device *pdev) > static struct platform_driver oxp_platform_driver = { > .driver = { > .name = "oxp-platform", > + .dev_groups = oxp_ec_groups, > }, > .probe = oxp_platform_probe, > };
diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c index e1a907cae820..1e1cc67bcdea 100644 --- a/drivers/hwmon/oxp-sensors.c +++ b/drivers/hwmon/oxp-sensors.c @@ -220,6 +220,20 @@ static int tt_toggle_disable(void) } /* Callbacks for turbo toggle attribute */ +static umode_t tt_toggle_is_visible(struct kobject *kobj, + struct attribute *attr, int n) +{ + switch (board) { + case aok_zoe_a1: + case oxp_mini_amd_a07: + case oxp_mini_amd_pro: + return attr->mode; + default: + break; + } + return 0; +} + static ssize_t tt_toggle_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) @@ -396,7 +410,15 @@ static struct attribute *oxp_ec_attrs[] = { NULL }; -ATTRIBUTE_GROUPS(oxp_ec); +static struct attribute_group oxp_ec_attribute_group = { + .is_visible = tt_toggle_is_visible, + .attrs = oxp_ec_attrs, +}; + +static const struct attribute_group *oxp_ec_groups[] = { + &oxp_ec_attribute_group, + NULL +}; static const struct hwmon_ops oxp_ec_hwmon_ops = { .is_visible = oxp_ec_hwmon_is_visible, @@ -415,7 +437,6 @@ static int oxp_platform_probe(struct platform_device *pdev) const struct dmi_system_id *dmi_entry; struct device *dev = &pdev->dev; struct device *hwdev; - int ret; /* * Have to check for AMD processor here because DMI strings are the @@ -430,18 +451,6 @@ static int oxp_platform_probe(struct platform_device *pdev) board = (enum oxp_board)(unsigned long)dmi_entry->driver_data; - switch (board) { - case aok_zoe_a1: - case oxp_mini_amd_a07: - case oxp_mini_amd_pro: - ret = devm_device_add_groups(dev, oxp_ec_groups); - if (ret) - return ret; - break; - default: - break; - } - hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL, &oxp_ec_chip_info, NULL); @@ -451,6 +460,7 @@ static int oxp_platform_probe(struct platform_device *pdev) static struct platform_driver oxp_platform_driver = { .driver = { .name = "oxp-platform", + .dev_groups = oxp_ec_groups, }, .probe = oxp_platform_probe, };
A driver should not be manually adding groups in its probe function (it will race with userspace), so replace the call to devm_device_add_groups() to use the platform dev_groups callback instead. This will allow for removal of the devm_device_add_groups() function. Signed-off-by: Joaquín Ignacio Aramendía <samsagax@gmail.com> --- drivers/hwmon/oxp-sensors.c | 38 +++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-)