Remove the use of devm_add_groups() in favour of dev_groups in platform
driver structure. This will allow for removal of the function as it was
intended in Greg's email[1].
Also move board detection to the init() instead of probe() function so we
don't instantiate the driver if the detection fails.
V2 drops the 3rd patch that removed the probe() function.
[1] Link: https://lore.kernel.org/linux-hwmon/[email protected]/
Joaquín Ignacio Aramendía (2):
hwmon: (oxp-sensors) Move tt_toggle attribute to dev_groups
hwmon: (oxp-sensors) Move board detection to the init function
drivers/hwmon/oxp-sensors.c | 67 +++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 28 deletions(-)
--
2.41.0
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 <[email protected]>
---
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,
};
--
2.41.0
Move detection logic to the start of init() function so we won't
instantiate the driver if the board is not compatible.
Signed-off-by: Joaquín Ignacio Aramendía <[email protected]>
---
drivers/hwmon/oxp-sensors.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/hwmon/oxp-sensors.c b/drivers/hwmon/oxp-sensors.c
index 1e1cc67bcdea..ea9602063eab 100644
--- a/drivers/hwmon/oxp-sensors.c
+++ b/drivers/hwmon/oxp-sensors.c
@@ -434,23 +434,9 @@ static const struct hwmon_chip_info oxp_ec_chip_info = {
/* Initialization logic */
static int oxp_platform_probe(struct platform_device *pdev)
{
- const struct dmi_system_id *dmi_entry;
struct device *dev = &pdev->dev;
struct device *hwdev;
- /*
- * Have to check for AMD processor here because DMI strings are the
- * same between Intel and AMD boards, the only way to tell them apart
- * is the CPU.
- * Intel boards seem to have different EC registers and values to
- * read/write.
- */
- dmi_entry = dmi_first_match(dmi_table);
- if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
- return -ENODEV;
-
- board = (enum oxp_board)(unsigned long)dmi_entry->driver_data;
-
hwdev = devm_hwmon_device_register_with_info(dev, "oxpec", NULL,
&oxp_ec_chip_info, NULL);
@@ -469,6 +455,21 @@ static struct platform_device *oxp_platform_device;
static int __init oxp_platform_init(void)
{
+ const struct dmi_system_id *dmi_entry;
+
+ /*
+ * Have to check for AMD processor here because DMI strings are the
+ * same between Intel and AMD boards, the only way to tell them apart
+ * is the CPU.
+ * Intel boards seem to have different EC registers and values to
+ * read/write.
+ */
+ dmi_entry = dmi_first_match(dmi_table);
+ if (!dmi_entry || boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+ return -ENODEV;
+
+ board = (enum oxp_board)(unsigned long)dmi_entry->driver_data;
+
oxp_platform_device =
platform_create_bundle(&oxp_platform_driver,
oxp_platform_probe, NULL, 0, NULL, 0);
--
2.41.0
On Mon, Jul 17, 2023 at 07:25:16PM -0300, Joaqu?n Ignacio Aramend?a wrote:
> Move detection logic to the start of init() function so we won't
> instantiate the driver if the board is not compatible.
>
> Signed-off-by: Joaqu?n Ignacio Aramend?a <[email protected]>
> ---
> drivers/hwmon/oxp-sensors.c | 29 +++++++++++++++--------------
> 1 file changed, 15 insertions(+), 14 deletions(-)
Reviewed-by: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>
> ---
> drivers/hwmon/oxp-sensors.c | 38 +++++++++++++++++++++++--------------
> 1 file changed, 24 insertions(+), 14 deletions(-)
Reviewed-by: Greg Kroah-Hartman <[email protected]>
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 <[email protected]>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
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,
> };
On Mon, Jul 17, 2023 at 07:25:16PM -0300, Joaqu?n Ignacio Aramend?a wrote:
> Move detection logic to the start of init() function so we won't
> instantiate the driver if the board is not compatible.
>
> Signed-off-by: Joaqu?n Ignacio Aramend?a <[email protected]>
> Reviewed-by: Greg Kroah-Hartman <[email protected]>
Applied.
Thanks,
Guenter