* [PATCH] sfc: Convert to use hwmon_device_register_with_groups
@ 2013-11-27 3:30 Guenter Roeck
2013-11-27 3:34 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2013-11-27 3:30 UTC (permalink / raw)
To: netdev; +Cc: linux-net-drivers, lm-sensors, Guenter Roeck
Simplify the code. Avoid race conditions caused by attributes
being created after hwmon device registration. Implicitly
(through hwmon API) add mandatory 'name' sysfs attribute.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Compile tested only.
drivers/net/ethernet/sfc/mcdi.h | 2 +
drivers/net/ethernet/sfc/mcdi_mon.c | 76 +++++++++++++----------------------
2 files changed, 31 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h
index 656a327..15816ca 100644
--- a/drivers/net/ethernet/sfc/mcdi.h
+++ b/drivers/net/ethernet/sfc/mcdi.h
@@ -75,6 +75,8 @@ struct efx_mcdi_mon {
unsigned long last_update;
struct device *device;
struct efx_mcdi_mon_attribute *attrs;
+ struct attribute_group group;
+ const struct attribute_group *groups[2];
unsigned int n_attrs;
};
diff --git a/drivers/net/ethernet/sfc/mcdi_mon.c b/drivers/net/ethernet/sfc/mcdi_mon.c
index 4cc5d95..ee7ce65 100644
--- a/drivers/net/ethernet/sfc/mcdi_mon.c
+++ b/drivers/net/ethernet/sfc/mcdi_mon.c
@@ -139,13 +139,6 @@ static int efx_mcdi_mon_update(struct efx_nic *efx)
return rc;
}
-static ssize_t efx_mcdi_mon_show_name(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "%s\n", KBUILD_MODNAME);
-}
-
static int efx_mcdi_mon_get_entry(struct device *dev, unsigned int index,
efx_dword_t *entry)
{
@@ -263,7 +256,7 @@ static ssize_t efx_mcdi_mon_show_label(struct device *dev,
efx_mcdi_sensor_type[mon_attr->type].label);
}
-static int
+static void
efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
ssize_t (*reader)(struct device *,
struct device_attribute *, char *),
@@ -272,7 +265,6 @@ efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
{
struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
struct efx_mcdi_mon_attribute *attr = &hwmon->attrs[hwmon->n_attrs];
- int rc;
strlcpy(attr->name, name, sizeof(attr->name));
attr->index = index;
@@ -286,10 +278,7 @@ efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
attr->dev_attr.attr.name = attr->name;
attr->dev_attr.attr.mode = S_IRUGO;
attr->dev_attr.show = reader;
- rc = device_create_file(&efx->pci_dev->dev, &attr->dev_attr);
- if (rc == 0)
- ++hwmon->n_attrs;
- return rc;
+ hwmon->group.attrs[hwmon->n_attrs++] = &attr->dev_attr.attr;
}
int efx_mcdi_mon_probe(struct efx_nic *efx)
@@ -338,26 +327,22 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
efx_mcdi_mon_update(efx);
/* Allocate space for the maximum possible number of
- * attributes for this set of sensors: name of the driver plus
+ * attributes for this set of sensors:
* value, min, max, crit, alarm and label for each sensor.
*/
- n_attrs = 1 + 6 * n_sensors;
+ n_attrs = 6 * n_sensors;
hwmon->attrs = kcalloc(n_attrs, sizeof(*hwmon->attrs), GFP_KERNEL);
if (!hwmon->attrs) {
rc = -ENOMEM;
goto fail;
}
-
- hwmon->device = hwmon_device_register(&efx->pci_dev->dev);
- if (IS_ERR(hwmon->device)) {
- rc = PTR_ERR(hwmon->device);
+ hwmon->group.attrs = kcalloc(n_attrs + 1, sizeof(struct attribute *),
+ GFP_KERNEL);
+ if (!hwmon->group.attrs) {
+ rc = -ENOMEM;
goto fail;
}
- rc = efx_mcdi_mon_add_attr(efx, "name", efx_mcdi_mon_show_name, 0, 0, 0);
- if (rc)
- goto fail;
-
for (i = 0, j = -1, type = -1; ; i++) {
enum efx_hwmon_type hwmon_type;
const char *hwmon_prefix;
@@ -372,7 +357,7 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
page = type / 32;
j = -1;
if (page == n_pages)
- return 0;
+ goto hwmon_register;
MCDI_SET_DWORD(inbuf, SENSOR_INFO_EXT_IN_PAGE,
page);
@@ -453,28 +438,22 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
if (min1 != max1) {
snprintf(name, sizeof(name), "%s%u_input",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_value, i, type, 0);
- if (rc)
- goto fail;
if (hwmon_type != EFX_HWMON_POWER) {
snprintf(name, sizeof(name), "%s%u_min",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, min1);
- if (rc)
- goto fail;
}
snprintf(name, sizeof(name), "%s%u_max",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, max1);
- if (rc)
- goto fail;
if (min2 != max2) {
/* Assume max2 is critical value.
@@ -482,32 +461,38 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
*/
snprintf(name, sizeof(name), "%s%u_crit",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, max2);
- if (rc)
- goto fail;
}
}
snprintf(name, sizeof(name), "%s%u_alarm",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_alarm, i, type, 0);
- if (rc)
- goto fail;
if (type < ARRAY_SIZE(efx_mcdi_sensor_type) &&
efx_mcdi_sensor_type[type].label) {
snprintf(name, sizeof(name), "%s%u_label",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_label, i, type, 0);
- if (rc)
- goto fail;
}
}
+hwmon_register:
+ hwmon->groups[0] = &hwmon->group;
+ hwmon->device = hwmon_device_register_with_groups(&efx->pci_dev->dev,
+ KBUILD_MODNAME, NULL,
+ hwmon->groups);
+ if (IS_ERR(hwmon->device)) {
+ rc = PTR_ERR(hwmon->device);
+ goto fail;
+ }
+
+ return 0;
+
fail:
efx_mcdi_mon_remove(efx);
return rc;
@@ -516,14 +501,11 @@ fail:
void efx_mcdi_mon_remove(struct efx_nic *efx)
{
struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
- unsigned int i;
- for (i = 0; i < hwmon->n_attrs; i++)
- device_remove_file(&efx->pci_dev->dev,
- &hwmon->attrs[i].dev_attr);
- kfree(hwmon->attrs);
if (hwmon->device)
hwmon_device_unregister(hwmon->device);
+ kfree(hwmon->attrs);
+ kfree(hwmon->group.attrs);
efx_nic_free_buffer(efx, &hwmon->dma_buf);
}
--
1.7.9.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] sfc: Convert to use hwmon_device_register_with_groups
@ 2013-11-27 3:33 Guenter Roeck
2013-11-28 2:11 ` Ben Hutchings
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2013-11-27 3:33 UTC (permalink / raw)
To: netdev; +Cc: linux-net-drivers, Ben Hutchings, lm-sensors, Guenter Roeck
Simplify the code. Avoid race conditions caused by attributes
being created after hwmon device registration. Implicitly
(through hwmon API) add mandatory 'name' sysfs attribute.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Compile tested only.
drivers/net/ethernet/sfc/mcdi.h | 2 +
drivers/net/ethernet/sfc/mcdi_mon.c | 76 +++++++++++++----------------------
2 files changed, 31 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h
index 656a327..15816ca 100644
--- a/drivers/net/ethernet/sfc/mcdi.h
+++ b/drivers/net/ethernet/sfc/mcdi.h
@@ -75,6 +75,8 @@ struct efx_mcdi_mon {
unsigned long last_update;
struct device *device;
struct efx_mcdi_mon_attribute *attrs;
+ struct attribute_group group;
+ const struct attribute_group *groups[2];
unsigned int n_attrs;
};
diff --git a/drivers/net/ethernet/sfc/mcdi_mon.c b/drivers/net/ethernet/sfc/mcdi_mon.c
index 4cc5d95..ee7ce65 100644
--- a/drivers/net/ethernet/sfc/mcdi_mon.c
+++ b/drivers/net/ethernet/sfc/mcdi_mon.c
@@ -139,13 +139,6 @@ static int efx_mcdi_mon_update(struct efx_nic *efx)
return rc;
}
-static ssize_t efx_mcdi_mon_show_name(struct device *dev,
- struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "%s\n", KBUILD_MODNAME);
-}
-
static int efx_mcdi_mon_get_entry(struct device *dev, unsigned int index,
efx_dword_t *entry)
{
@@ -263,7 +256,7 @@ static ssize_t efx_mcdi_mon_show_label(struct device *dev,
efx_mcdi_sensor_type[mon_attr->type].label);
}
-static int
+static void
efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
ssize_t (*reader)(struct device *,
struct device_attribute *, char *),
@@ -272,7 +265,6 @@ efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
{
struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
struct efx_mcdi_mon_attribute *attr = &hwmon->attrs[hwmon->n_attrs];
- int rc;
strlcpy(attr->name, name, sizeof(attr->name));
attr->index = index;
@@ -286,10 +278,7 @@ efx_mcdi_mon_add_attr(struct efx_nic *efx, const char *name,
attr->dev_attr.attr.name = attr->name;
attr->dev_attr.attr.mode = S_IRUGO;
attr->dev_attr.show = reader;
- rc = device_create_file(&efx->pci_dev->dev, &attr->dev_attr);
- if (rc == 0)
- ++hwmon->n_attrs;
- return rc;
+ hwmon->group.attrs[hwmon->n_attrs++] = &attr->dev_attr.attr;
}
int efx_mcdi_mon_probe(struct efx_nic *efx)
@@ -338,26 +327,22 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
efx_mcdi_mon_update(efx);
/* Allocate space for the maximum possible number of
- * attributes for this set of sensors: name of the driver plus
+ * attributes for this set of sensors:
* value, min, max, crit, alarm and label for each sensor.
*/
- n_attrs = 1 + 6 * n_sensors;
+ n_attrs = 6 * n_sensors;
hwmon->attrs = kcalloc(n_attrs, sizeof(*hwmon->attrs), GFP_KERNEL);
if (!hwmon->attrs) {
rc = -ENOMEM;
goto fail;
}
-
- hwmon->device = hwmon_device_register(&efx->pci_dev->dev);
- if (IS_ERR(hwmon->device)) {
- rc = PTR_ERR(hwmon->device);
+ hwmon->group.attrs = kcalloc(n_attrs + 1, sizeof(struct attribute *),
+ GFP_KERNEL);
+ if (!hwmon->group.attrs) {
+ rc = -ENOMEM;
goto fail;
}
- rc = efx_mcdi_mon_add_attr(efx, "name", efx_mcdi_mon_show_name, 0, 0, 0);
- if (rc)
- goto fail;
-
for (i = 0, j = -1, type = -1; ; i++) {
enum efx_hwmon_type hwmon_type;
const char *hwmon_prefix;
@@ -372,7 +357,7 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
page = type / 32;
j = -1;
if (page == n_pages)
- return 0;
+ goto hwmon_register;
MCDI_SET_DWORD(inbuf, SENSOR_INFO_EXT_IN_PAGE,
page);
@@ -453,28 +438,22 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
if (min1 != max1) {
snprintf(name, sizeof(name), "%s%u_input",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_value, i, type, 0);
- if (rc)
- goto fail;
if (hwmon_type != EFX_HWMON_POWER) {
snprintf(name, sizeof(name), "%s%u_min",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, min1);
- if (rc)
- goto fail;
}
snprintf(name, sizeof(name), "%s%u_max",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, max1);
- if (rc)
- goto fail;
if (min2 != max2) {
/* Assume max2 is critical value.
@@ -482,32 +461,38 @@ int efx_mcdi_mon_probe(struct efx_nic *efx)
*/
snprintf(name, sizeof(name), "%s%u_crit",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_limit,
i, type, max2);
- if (rc)
- goto fail;
}
}
snprintf(name, sizeof(name), "%s%u_alarm",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_alarm, i, type, 0);
- if (rc)
- goto fail;
if (type < ARRAY_SIZE(efx_mcdi_sensor_type) &&
efx_mcdi_sensor_type[type].label) {
snprintf(name, sizeof(name), "%s%u_label",
hwmon_prefix, hwmon_index);
- rc = efx_mcdi_mon_add_attr(
+ efx_mcdi_mon_add_attr(
efx, name, efx_mcdi_mon_show_label, i, type, 0);
- if (rc)
- goto fail;
}
}
+hwmon_register:
+ hwmon->groups[0] = &hwmon->group;
+ hwmon->device = hwmon_device_register_with_groups(&efx->pci_dev->dev,
+ KBUILD_MODNAME, NULL,
+ hwmon->groups);
+ if (IS_ERR(hwmon->device)) {
+ rc = PTR_ERR(hwmon->device);
+ goto fail;
+ }
+
+ return 0;
+
fail:
efx_mcdi_mon_remove(efx);
return rc;
@@ -516,14 +501,11 @@ fail:
void efx_mcdi_mon_remove(struct efx_nic *efx)
{
struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
- unsigned int i;
- for (i = 0; i < hwmon->n_attrs; i++)
- device_remove_file(&efx->pci_dev->dev,
- &hwmon->attrs[i].dev_attr);
- kfree(hwmon->attrs);
if (hwmon->device)
hwmon_device_unregister(hwmon->device);
+ kfree(hwmon->attrs);
+ kfree(hwmon->group.attrs);
efx_nic_free_buffer(efx, &hwmon->dma_buf);
}
--
1.7.9.7
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] sfc: Convert to use hwmon_device_register_with_groups
2013-11-27 3:30 [PATCH] sfc: Convert to use hwmon_device_register_with_groups Guenter Roeck
@ 2013-11-27 3:34 ` Guenter Roeck
0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2013-11-27 3:34 UTC (permalink / raw)
To: netdev; +Cc: linux-net-drivers, lm-sensors, Guenter Roeck
On 11/26/2013 07:30 PM, Guenter Roeck wrote:
> Simplify the code. Avoid race conditions caused by attributes
> being created after hwmon device registration. Implicitly
> (through hwmon API) add mandatory 'name' sysfs attribute.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Compile tested only.
>
Sorry for the noise if you see this patch multiple times. My send script tried to annoy me.
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sfc: Convert to use hwmon_device_register_with_groups
2013-11-27 3:33 Guenter Roeck
@ 2013-11-28 2:11 ` Ben Hutchings
2013-11-28 2:51 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Ben Hutchings @ 2013-11-28 2:11 UTC (permalink / raw)
To: Guenter Roeck; +Cc: netdev, linux-net-drivers, lm-sensors
On Tue, 2013-11-26 at 19:33 -0800, Guenter Roeck wrote:
> Simplify the code. Avoid race conditions caused by attributes
> being created after hwmon device registration. Implicitly
> (through hwmon API) add mandatory 'name' sysfs attribute.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> Compile tested only.
[...]
Very close, but you missed this bit:
--- a/drivers/net/ethernet/sfc/mcdi_mon.c
+++ b/drivers/net/ethernet/sfc/mcdi_mon.c
@@ -142,7 +142,7 @@ static int efx_mcdi_mon_update(struct efx_nic *efx)
static int efx_mcdi_mon_get_entry(struct device *dev, unsigned int index,
efx_dword_t *entry)
{
- struct efx_nic *efx = dev_get_drvdata(dev);
+ struct efx_nic *efx = dev_get_drvdata(dev->parent);
struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
int rc;
---
With that addition, it works for me, and you can add my Reviewed-by.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] sfc: Convert to use hwmon_device_register_with_groups
2013-11-28 2:11 ` Ben Hutchings
@ 2013-11-28 2:51 ` Guenter Roeck
0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2013-11-28 2:51 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, linux-net-drivers, lm-sensors
On 11/27/2013 06:11 PM, Ben Hutchings wrote:
> On Tue, 2013-11-26 at 19:33 -0800, Guenter Roeck wrote:
>> Simplify the code. Avoid race conditions caused by attributes
>> being created after hwmon device registration. Implicitly
>> (through hwmon API) add mandatory 'name' sysfs attribute.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> Compile tested only.
> [...]
>
> Very close, but you missed this bit:
>
> --- a/drivers/net/ethernet/sfc/mcdi_mon.c
> +++ b/drivers/net/ethernet/sfc/mcdi_mon.c
> @@ -142,7 +142,7 @@ static int efx_mcdi_mon_update(struct efx_nic *efx)
> static int efx_mcdi_mon_get_entry(struct device *dev, unsigned int index,
> efx_dword_t *entry)
> {
> - struct efx_nic *efx = dev_get_drvdata(dev);
> + struct efx_nic *efx = dev_get_drvdata(dev->parent);
> struct efx_mcdi_mon *hwmon = efx_mcdi_mon(efx);
> int rc;
>
> ---
>
> With that addition, it works for me, and you can add my Reviewed-by.
>
Thanks a lot!
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-11-28 2:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-27 3:30 [PATCH] sfc: Convert to use hwmon_device_register_with_groups Guenter Roeck
2013-11-27 3:34 ` Guenter Roeck
-- strict thread matches above, loose matches on Subject: below --
2013-11-27 3:33 Guenter Roeck
2013-11-28 2:11 ` Ben Hutchings
2013-11-28 2:51 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).