From: Guenter R. <li...@ro...> - 2013-11-23 06:08:21
|
The hwmon subsystem is used by various network drivers to report temperature sensor and other information. Unfortunately, its use is often not correct. Typical errors are that the mandatory name sysfs attribute is not created, that the temperature sensor index starts with 0 instead of 1, and/or that sysfs attributes are created after the hwmon device was created. The following sequence of patches fixes most of the problems. The igb patches have been tested with real hardware; the others are compile tested only. |
From: Guenter R. <li...@ro...> - 2013-11-23 06:08:22
|
Use new hwmon API to simplify code, provide missing mandatory 'name' sysfs attribute, and attach hwmon attributes to hwmon device instead of pci device. Signed-off-by: Guenter Roeck <li...@ro...> --- drivers/net/ethernet/broadcom/tg3.c | 25 ++++++------------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c index a9e0684..369b736 100644 --- a/drivers/net/ethernet/broadcom/tg3.c +++ b/drivers/net/ethernet/broadcom/tg3.c @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir) static ssize_t tg3_show_temp(struct device *dev, struct device_attribute *devattr, char *buf) { - struct pci_dev *pdev = to_pci_dev(dev); - struct net_device *netdev = pci_get_drvdata(pdev); - struct tg3 *tp = netdev_priv(netdev); struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); + struct tg3 *tp = dev_get_drvdata(dev); u32 temperature; spin_lock_bh(&tp->lock); @@ -10650,29 +10648,25 @@ static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, tg3_show_temp, NULL, static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, tg3_show_temp, NULL, TG3_TEMP_MAX_OFFSET); -static struct attribute *tg3_attributes[] = { +static struct attribute *tg3_attrs[] = { &sensor_dev_attr_temp1_input.dev_attr.attr, &sensor_dev_attr_temp1_crit.dev_attr.attr, &sensor_dev_attr_temp1_max.dev_attr.attr, NULL }; - -static const struct attribute_group tg3_group = { - .attrs = tg3_attributes, -}; +ATTRIBUTE_GROUPS(tg3); static void tg3_hwmon_close(struct tg3 *tp) { if (tp->hwmon_dev) { hwmon_device_unregister(tp->hwmon_dev); tp->hwmon_dev = NULL; - sysfs_remove_group(&tp->pdev->dev.kobj, &tg3_group); } } static void tg3_hwmon_open(struct tg3 *tp) { - int i, err; + int i; u32 size = 0; struct pci_dev *pdev = tp->pdev; struct tg3_ocir ocirs[TG3_SD_NUM_RECS]; @@ -10690,18 +10684,11 @@ static void tg3_hwmon_open(struct tg3 *tp) if (!size) return; - /* Register hwmon sysfs hooks */ - err = sysfs_create_group(&pdev->dev.kobj, &tg3_group); - if (err) { - dev_err(&pdev->dev, "Cannot create sysfs group, aborting\n"); - return; - } - - tp->hwmon_dev = hwmon_device_register(&pdev->dev); + tp->hwmon_dev = hwmon_device_register_with_groups(&pdev->dev, "tg3", + tp, tg3_groups); if (IS_ERR(tp->hwmon_dev)) { tp->hwmon_dev = NULL; dev_err(&pdev->dev, "Cannot register hwmon device, aborting\n"); - sysfs_remove_group(&pdev->dev.kobj, &tg3_group); } } -- 1.7.9.7 |
From: Jean D. <kh...@li...> - 2013-11-23 14:33:22
|
Hi Guenter, On Fri, 22 Nov 2013 22:07:57 -0800, Guenter Roeck wrote: > Use new hwmon API to simplify code, provide missing mandatory 'name' > sysfs attribute, and attach hwmon attributes to hwmon device instead > of pci device. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/broadcom/tg3.c | 25 ++++++------------------- > 1 file changed, 6 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index a9e0684..369b736 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir) > static ssize_t tg3_show_temp(struct device *dev, > struct device_attribute *devattr, char *buf) > { > - struct pci_dev *pdev = to_pci_dev(dev); > - struct net_device *netdev = pci_get_drvdata(pdev); > - struct tg3 *tp = netdev_priv(netdev); > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct tg3 *tp = dev_get_drvdata(dev); > u32 temperature; > > spin_lock_bh(&tp->lock); > @@ -10650,29 +10648,25 @@ static SENSOR_DEVICE_ATTR(temp1_crit, S_IRUGO, tg3_show_temp, NULL, > static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO, tg3_show_temp, NULL, > TG3_TEMP_MAX_OFFSET); > > -static struct attribute *tg3_attributes[] = { > +static struct attribute *tg3_attrs[] = { > &sensor_dev_attr_temp1_input.dev_attr.attr, > &sensor_dev_attr_temp1_crit.dev_attr.attr, > &sensor_dev_attr_temp1_max.dev_attr.attr, > NULL > }; > - > -static const struct attribute_group tg3_group = { > - .attrs = tg3_attributes, > -}; > +ATTRIBUTE_GROUPS(tg3); > > static void tg3_hwmon_close(struct tg3 *tp) > { > if (tp->hwmon_dev) { > hwmon_device_unregister(tp->hwmon_dev); > tp->hwmon_dev = NULL; > - sysfs_remove_group(&tp->pdev->dev.kobj, &tg3_group); > } > } > > static void tg3_hwmon_open(struct tg3 *tp) > { > - int i, err; > + int i; > u32 size = 0; > struct pci_dev *pdev = tp->pdev; > struct tg3_ocir ocirs[TG3_SD_NUM_RECS]; > @@ -10690,18 +10684,11 @@ static void tg3_hwmon_open(struct tg3 *tp) > if (!size) > return; > > - /* Register hwmon sysfs hooks */ > - err = sysfs_create_group(&pdev->dev.kobj, &tg3_group); > - if (err) { > - dev_err(&pdev->dev, "Cannot create sysfs group, aborting\n"); > - return; > - } > - > - tp->hwmon_dev = hwmon_device_register(&pdev->dev); > + tp->hwmon_dev = hwmon_device_register_with_groups(&pdev->dev, "tg3", > + tp, tg3_groups); > if (IS_ERR(tp->hwmon_dev)) { > tp->hwmon_dev = NULL; > dev_err(&pdev->dev, "Cannot register hwmon device, aborting\n"); > - sysfs_remove_group(&pdev->dev.kobj, &tg3_group); > } > } > I can't test this, but the changes look good to me. Reviewed-by: Jean Delvare <kh...@li...> -- Jean Delvare |
From: David M. <da...@da...> - 2013-11-28 23:22:37
|
From: Guenter Roeck <li...@ro...> Date: Fri, 22 Nov 2013 22:07:57 -0800 > Use new hwmon API to simplify code, provide missing mandatory 'name' > sysfs attribute, and attach hwmon attributes to hwmon device instead > of pci device. > > Signed-off-by: Guenter Roeck <li...@ro...> Applied, thanks. Jeff Kirsher will pull the rest of this series in via his Intel ethernet driver tree. |
From: Guenter R. <li...@ro...> - 2013-11-23 06:08:24
|
Simplify the code. Attach hwmon sysfs attributes to hwmon device instead of pci device. Avoid race conditions caused by attributes being created after registration and provide mandatory 'name' attribute by using new hwmon API. Other cleanup: Instead of allocating memory for hwmon attributes, move attributes and all other hwmon related data into struct hwmon_buff and allocate the entire structure using devm_kzalloc. Check return value from calls to igb_add_hwmon_attr() one by one instead of logically combining them all together. Signed-off-by: Guenter Roeck <li...@ro...> --- drivers/net/ethernet/intel/igb/igb.h | 8 ++- drivers/net/ethernet/intel/igb/igb_hwmon.c | 100 +++++++++++++--------------- 2 files changed, 53 insertions(+), 55 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 5e9ed89..99c3d33 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -337,8 +337,10 @@ struct hwmon_attr { }; struct hwmon_buff { - struct device *device; - struct hwmon_attr *hwmon_list; + struct attribute_group group; + const struct attribute_group *groups[2]; + struct attribute *attrs[E1000_MAX_SENSORS * 4 + 1]; + struct hwmon_attr hwmon_list[E1000_MAX_SENSORS * 4]; unsigned int n_hwmon; }; #endif @@ -440,7 +442,7 @@ struct igb_adapter { char fw_version[32]; #ifdef CONFIG_IGB_HWMON - struct hwmon_buff igb_hwmon_buff; + struct hwmon_buff *igb_hwmon_buff; bool ets; #endif struct i2c_algo_bit_data i2c_algo; diff --git a/drivers/net/ethernet/intel/igb/igb_hwmon.c b/drivers/net/ethernet/intel/igb/igb_hwmon.c index 58f1ce9..2e7ef2d 100644 --- a/drivers/net/ethernet/intel/igb/igb_hwmon.c +++ b/drivers/net/ethernet/intel/igb/igb_hwmon.c @@ -117,8 +117,8 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter, unsigned int n_attr; struct hwmon_attr *igb_attr; - n_attr = adapter->igb_hwmon_buff.n_hwmon; - igb_attr = &adapter->igb_hwmon_buff.hwmon_list[n_attr]; + n_attr = adapter->igb_hwmon_buff->n_hwmon; + igb_attr = &adapter->igb_hwmon_buff->hwmon_list[n_attr]; switch (type) { case IGB_HWMON_TYPE_LOC: @@ -154,30 +154,16 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter, igb_attr->dev_attr.attr.mode = S_IRUGO; igb_attr->dev_attr.attr.name = igb_attr->name; sysfs_attr_init(&igb_attr->dev_attr.attr); - rc = device_create_file(&adapter->pdev->dev, - &igb_attr->dev_attr); - if (rc == 0) - ++adapter->igb_hwmon_buff.n_hwmon; - return rc; + adapter->igb_hwmon_buff->attrs[n_attr] = &igb_attr->dev_attr.attr; + + ++adapter->igb_hwmon_buff->n_hwmon; + + return 0; } static void igb_sysfs_del_adapter(struct igb_adapter *adapter) { - int i; - - if (adapter == NULL) - return; - - for (i = 0; i < adapter->igb_hwmon_buff.n_hwmon; i++) { - device_remove_file(&adapter->pdev->dev, - &adapter->igb_hwmon_buff.hwmon_list[i].dev_attr); - } - - kfree(adapter->igb_hwmon_buff.hwmon_list); - - if (adapter->igb_hwmon_buff.device) - hwmon_device_unregister(adapter->igb_hwmon_buff.device); } /* called from igb_main.c */ @@ -189,11 +175,11 @@ void igb_sysfs_exit(struct igb_adapter *adapter) /* called from igb_main.c */ int igb_sysfs_init(struct igb_adapter *adapter) { - struct hwmon_buff *igb_hwmon = &adapter->igb_hwmon_buff; + struct hwmon_buff *igb_hwmon; + struct i2c_client *client; + struct device *hwmon_dev; unsigned int i; - int n_attrs; int rc = 0; - struct i2c_client *client = NULL; /* If this method isn't defined we don't support thermals */ if (adapter->hw.mac.ops.init_thermal_sensor_thresh == NULL) @@ -201,34 +187,16 @@ int igb_sysfs_init(struct igb_adapter *adapter) /* Don't create thermal hwmon interface if no sensors present */ rc = (adapter->hw.mac.ops.init_thermal_sensor_thresh(&adapter->hw)); - if (rc) - goto exit; - - /* init i2c_client */ - client = i2c_new_device(&adapter->i2c_adap, &i350_sensor_info); - if (client == NULL) { - dev_info(&adapter->pdev->dev, - "Failed to create new i2c device..\n"); + if (rc) goto exit; - } - adapter->i2c_client = client; - /* Allocation space for max attributes - * max num sensors * values (loc, temp, max, caution) - */ - n_attrs = E1000_MAX_SENSORS * 4; - igb_hwmon->hwmon_list = kcalloc(n_attrs, sizeof(struct hwmon_attr), - GFP_KERNEL); - if (!igb_hwmon->hwmon_list) { + igb_hwmon = devm_kzalloc(&adapter->pdev->dev, sizeof(*igb_hwmon), + GFP_KERNEL); + if (!igb_hwmon) { rc = -ENOMEM; - goto err; - } - - igb_hwmon->device = hwmon_device_register(&adapter->pdev->dev); - if (IS_ERR(igb_hwmon->device)) { - rc = PTR_ERR(igb_hwmon->device); - goto err; + goto exit; } + adapter->igb_hwmon_buff = igb_hwmon; for (i = 0; i < E1000_MAX_SENSORS; i++) { @@ -240,11 +208,39 @@ int igb_sysfs_init(struct igb_adapter *adapter) /* Bail if any hwmon attr struct fails to initialize */ rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_CAUTION); - rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_LOC); - rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_TEMP); - rc |= igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_MAX); if (rc) - goto err; + goto exit; + rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_LOC); + if (rc) + goto exit; + rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_TEMP); + if (rc) + goto exit; + rc = igb_add_hwmon_attr(adapter, i, IGB_HWMON_TYPE_MAX); + if (rc) + goto exit; + } + + /* init i2c_client */ + client = i2c_new_device(&adapter->i2c_adap, &i350_sensor_info); + if (client == NULL) { + dev_info(&adapter->pdev->dev, + "Failed to create new i2c device.\n"); + rc = -ENODEV; + goto exit; + } + adapter->i2c_client = client; + + igb_hwmon->groups[0] = &igb_hwmon->group; + igb_hwmon->group.attrs = igb_hwmon->attrs; + + hwmon_dev = devm_hwmon_device_register_with_groups(&adapter->pdev->dev, + client->name, + igb_hwmon, + igb_hwmon->groups); + if (IS_ERR(hwmon_dev)) { + rc = PTR_ERR(hwmon_dev); + goto err; } goto exit; -- 1.7.9.7 |
From: Jeff K. <jef...@in...> - 2013-11-25 23:16:45
Attachments:
signature.asc
|
On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: > Simplify the code. Attach hwmon sysfs attributes to hwmon device > instead of pci device. Avoid race conditions caused by attributes > being created after registration and provide mandatory 'name' > attribute by using new hwmon API. > > Other cleanup: > > Instead of allocating memory for hwmon attributes, move attributes > and all other hwmon related data into struct hwmon_buff and allocate > the entire structure using devm_kzalloc. > > Check return value from calls to igb_add_hwmon_attr() one by one > instead > of logically combining them all together. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/igb/igb.h | 8 ++- > drivers/net/ethernet/intel/igb/igb_hwmon.c | 100 > +++++++++++++--------------- > 2 files changed, 53 insertions(+), 55 deletions(-) I have added this to my queue, thanks! |
From: Guenter R. <li...@ro...> - 2013-11-23 06:08:26
|
Simplify the code. Attach hwmon sysfs attributes to hwmon device instead of pci device. Avoid race conditions caused by attributes being created after hwmon device registration. Implicitly (through hwmon API) add mandatory 'name' sysfs attribute. Other cleanup: Instead of allocating memory for hwmon attributes, move attributes and all other hwmon related data into struct hwmon_buff and allocate the entire structure using devm_kzalloc. Check return value from calls to igb_add_hwmon_attr() one by one instead of logically combining them all together. Signed-off-by: Guenter Roeck <li...@ro...> --- drivers/net/ethernet/intel/ixgbe/ixgbe.h | 8 ++- drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 76 ++++++++++-------------- 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index f38fc0a..49531cd 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -552,8 +552,10 @@ struct hwmon_attr { }; struct hwmon_buff { - struct device *device; - struct hwmon_attr *hwmon_list; + struct attribute_group group; + const struct attribute_group *groups[2]; + struct attribute *attrs[IXGBE_MAX_SENSORS * 4 + 1]; + struct hwmon_attr hwmon_list[IXGBE_MAX_SENSORS * 4]; unsigned int n_hwmon; }; #endif /* CONFIG_IXGBE_HWMON */ @@ -775,7 +777,7 @@ struct ixgbe_adapter { u32 vferr_refcount; struct kobject *info_kobj; #ifdef CONFIG_IXGBE_HWMON - struct hwmon_buff ixgbe_hwmon_buff; + struct hwmon_buff *ixgbe_hwmon_buff; #endif /* CONFIG_IXGBE_HWMON */ #ifdef CONFIG_DEBUG_FS struct dentry *ixgbe_dbg_adapter; diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c index d118def..3081974 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c @@ -111,8 +111,8 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter, unsigned int n_attr; struct hwmon_attr *ixgbe_attr; - n_attr = adapter->ixgbe_hwmon_buff.n_hwmon; - ixgbe_attr = &adapter->ixgbe_hwmon_buff.hwmon_list[n_attr]; + n_attr = adapter->ixgbe_hwmon_buff->n_hwmon; + ixgbe_attr = &adapter->ixgbe_hwmon_buff->hwmon_list[n_attr]; switch (type) { case IXGBE_HWMON_TYPE_LOC: @@ -147,32 +147,17 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter, ixgbe_attr->dev_attr.store = NULL; ixgbe_attr->dev_attr.attr.mode = S_IRUGO; ixgbe_attr->dev_attr.attr.name = ixgbe_attr->name; + sysfs_attr_init(&ixgbe_attr->dev_attr.attr); - rc = device_create_file(&adapter->pdev->dev, - &ixgbe_attr->dev_attr); + adapter->ixgbe_hwmon_buff->attrs[n_attr] = &ixgbe_attr->dev_attr.attr; - if (rc == 0) - ++adapter->ixgbe_hwmon_buff.n_hwmon; + ++adapter->ixgbe_hwmon_buff->n_hwmon; - return rc; + return 0; } static void ixgbe_sysfs_del_adapter(struct ixgbe_adapter *adapter) { - int i; - - if (adapter == NULL) - return; - - for (i = 0; i < adapter->ixgbe_hwmon_buff.n_hwmon; i++) { - device_remove_file(&adapter->pdev->dev, - &adapter->ixgbe_hwmon_buff.hwmon_list[i].dev_attr); - } - - kfree(adapter->ixgbe_hwmon_buff.hwmon_list); - - if (adapter->ixgbe_hwmon_buff.device) - hwmon_device_unregister(adapter->ixgbe_hwmon_buff.device); } /* called from ixgbe_main.c */ @@ -184,9 +169,9 @@ void ixgbe_sysfs_exit(struct ixgbe_adapter *adapter) /* called from ixgbe_main.c */ int ixgbe_sysfs_init(struct ixgbe_adapter *adapter) { - struct hwmon_buff *ixgbe_hwmon = &adapter->ixgbe_hwmon_buff; + struct hwmon_buff *ixgbe_hwmon; + struct device *hwmon_dev; unsigned int i; - int n_attrs; int rc = 0; /* If this method isn't defined we don't support thermals */ @@ -198,23 +183,13 @@ int ixgbe_sysfs_init(struct ixgbe_adapter *adapter) if (adapter->hw.mac.ops.init_thermal_sensor_thresh(&adapter->hw)) goto exit; - /* - * Allocation space for max attributs - * max num sensors * values (loc, temp, max, caution) - */ - n_attrs = IXGBE_MAX_SENSORS * 4; - ixgbe_hwmon->hwmon_list = kcalloc(n_attrs, sizeof(struct hwmon_attr), - GFP_KERNEL); - if (!ixgbe_hwmon->hwmon_list) { + ixgbe_hwmon = devm_kzalloc(&adapter->pdev->dev, sizeof(*ixgbe_hwmon), + GFP_KERNEL); + if (ixgbe_hwmon == NULL) { rc = -ENOMEM; - goto err; - } - - ixgbe_hwmon->device = hwmon_device_register(&adapter->pdev->dev); - if (IS_ERR(ixgbe_hwmon->device)) { - rc = PTR_ERR(ixgbe_hwmon->device); - goto err; + goto exit; } + adapter->ixgbe_hwmon_buff = ixgbe_hwmon; for (i = 0; i < IXGBE_MAX_SENSORS; i++) { /* @@ -226,17 +201,28 @@ int ixgbe_sysfs_init(struct ixgbe_adapter *adapter) /* Bail if any hwmon attr struct fails to initialize */ rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_CAUTION); - rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_LOC); - rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_TEMP); - rc |= ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_MAX); if (rc) - goto err; + goto exit; + rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_LOC); + if (rc) + goto exit; + rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_TEMP); + if (rc) + goto exit; + rc = ixgbe_add_hwmon_attr(adapter, i, IXGBE_HWMON_TYPE_MAX); + if (rc) + goto exit; } - goto exit; + ixgbe_hwmon->groups[0] = &ixgbe_hwmon->group; + ixgbe_hwmon->group.attrs = ixgbe_hwmon->attrs; -err: - ixgbe_sysfs_del_adapter(adapter); + hwmon_dev = devm_hwmon_device_register_with_groups(&adapter->pdev->dev, + "ixgbe", + ixgbe_hwmon, + ixgbe_hwmon->groups); + if (IS_ERR(hwmon_dev)) + rc = PTR_ERR(hwmon_dev); exit: return rc; } -- 1.7.9.7 |
From: Jeff K. <jef...@in...> - 2013-11-25 23:17:34
Attachments:
signature.asc
|
On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: > Simplify the code. Attach hwmon sysfs attributes to hwmon device > instead of pci device. Avoid race conditions caused by attributes > being created after hwmon device registration. Implicitly > (through hwmon API) add mandatory 'name' sysfs attribute. > > Other cleanup: > > Instead of allocating memory for hwmon attributes, move attributes > and all other hwmon related data into struct hwmon_buff and allocate > the entire structure using devm_kzalloc. > > Check return value from calls to igb_add_hwmon_attr() one by one > instead > of logically combining them all together. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe.h | 8 ++- > drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 76 > ++++++++++-------------- > 2 files changed, 36 insertions(+), 48 deletions(-) I have added this to my queue, thanks! |
From: Guenter R. <li...@ro...> - 2013-11-23 06:08:27
|
Per hwmon ABI, temperature sensor attribute index starts with 1, not 0. Signed-off-by: Guenter Roeck <li...@ro...> --- drivers/net/ethernet/intel/igb/igb_hwmon.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb_hwmon.c b/drivers/net/ethernet/intel/igb/igb_hwmon.c index 2e7ef2d..e0af5bc 100644 --- a/drivers/net/ethernet/intel/igb/igb_hwmon.c +++ b/drivers/net/ethernet/intel/igb/igb_hwmon.c @@ -124,22 +124,22 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter, case IGB_HWMON_TYPE_LOC: igb_attr->dev_attr.show = igb_hwmon_show_location; snprintf(igb_attr->name, sizeof(igb_attr->name), - "temp%u_label", offset); + "temp%u_label", offset + 1); break; case IGB_HWMON_TYPE_TEMP: igb_attr->dev_attr.show = igb_hwmon_show_temp; snprintf(igb_attr->name, sizeof(igb_attr->name), - "temp%u_input", offset); + "temp%u_input", offset + 1); break; case IGB_HWMON_TYPE_CAUTION: igb_attr->dev_attr.show = igb_hwmon_show_cautionthresh; snprintf(igb_attr->name, sizeof(igb_attr->name), - "temp%u_max", offset); + "temp%u_max", offset + 1); break; case IGB_HWMON_TYPE_MAX: igb_attr->dev_attr.show = igb_hwmon_show_maxopthresh; snprintf(igb_attr->name, sizeof(igb_attr->name), - "temp%u_crit", offset); + "temp%u_crit", offset + 1); break; default: rc = -EPERM; -- 1.7.9.7 |
From: Jean D. <kh...@li...> - 2013-11-23 13:29:48
|
On Fri, 22 Nov 2013 22:08:00 -0800, Guenter Roeck wrote: > Per hwmon ABI, temperature sensor attribute index starts with 1, not 0. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/igb/igb_hwmon.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_hwmon.c b/drivers/net/ethernet/intel/igb/igb_hwmon.c > index 2e7ef2d..e0af5bc 100644 > --- a/drivers/net/ethernet/intel/igb/igb_hwmon.c > +++ b/drivers/net/ethernet/intel/igb/igb_hwmon.c > @@ -124,22 +124,22 @@ static int igb_add_hwmon_attr(struct igb_adapter *adapter, > case IGB_HWMON_TYPE_LOC: > igb_attr->dev_attr.show = igb_hwmon_show_location; > snprintf(igb_attr->name, sizeof(igb_attr->name), > - "temp%u_label", offset); > + "temp%u_label", offset + 1); > break; > case IGB_HWMON_TYPE_TEMP: > igb_attr->dev_attr.show = igb_hwmon_show_temp; > snprintf(igb_attr->name, sizeof(igb_attr->name), > - "temp%u_input", offset); > + "temp%u_input", offset + 1); > break; > case IGB_HWMON_TYPE_CAUTION: > igb_attr->dev_attr.show = igb_hwmon_show_cautionthresh; > snprintf(igb_attr->name, sizeof(igb_attr->name), > - "temp%u_max", offset); > + "temp%u_max", offset + 1); > break; > case IGB_HWMON_TYPE_MAX: > igb_attr->dev_attr.show = igb_hwmon_show_maxopthresh; > snprintf(igb_attr->name, sizeof(igb_attr->name), > - "temp%u_crit", offset); > + "temp%u_crit", offset + 1); > break; > default: > rc = -EPERM; Reviewed-by: Jean Delvare <kh...@li...> -- Jean Delvare |
From: Jeff K. <jef...@in...> - 2013-11-25 23:18:04
Attachments:
signature.asc
|
On Fri, 2013-11-22 at 22:08 -0800, Guenter Roeck wrote: > Per hwmon ABI, temperature sensor attribute index starts with 1, not > 0. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/igb/igb_hwmon.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) I have also added this to my queue, thank you. |
From: Guenter R. <li...@ro...> - 2013-11-23 06:08:29
|
Per hwmon ABI, temperature sensor attribute index starts with 1, not 0. Signed-off-by: Guenter Roeck <li...@ro...> --- drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c index 3081974..e74ae36 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c @@ -118,22 +118,22 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter, case IXGBE_HWMON_TYPE_LOC: ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_location; snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), - "temp%u_label", offset); + "temp%u_label", offset + 1); break; case IXGBE_HWMON_TYPE_TEMP: ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_temp; snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), - "temp%u_input", offset); + "temp%u_input", offset + 1); break; case IXGBE_HWMON_TYPE_CAUTION: ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_cautionthresh; snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), - "temp%u_max", offset); + "temp%u_max", offset + 1); break; case IXGBE_HWMON_TYPE_MAX: ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_maxopthresh; snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), - "temp%u_crit", offset); + "temp%u_crit", offset + 1); break; default: rc = -EPERM; -- 1.7.9.7 |
From: Jeff K. <jef...@in...> - 2013-11-25 23:18:34
Attachments:
signature.asc
|
On Fri, 2013-11-22 at 22:08 -0800, Guenter Roeck wrote: > Per hwmon ABI, temperature sensor attribute index starts with 1, not > 0. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) I have also added this to my queue, thank you. |
From: Jean D. <kh...@li...> - 2013-11-23 13:29:37
|
On Fri, 22 Nov 2013 22:08:01 -0800, Guenter Roeck wrote: > Per hwmon ABI, temperature sensor attribute index starts with 1, not 0. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > index 3081974..e74ae36 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > @@ -118,22 +118,22 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter, > case IXGBE_HWMON_TYPE_LOC: > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_location; > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > - "temp%u_label", offset); > + "temp%u_label", offset + 1); > break; > case IXGBE_HWMON_TYPE_TEMP: > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_temp; > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > - "temp%u_input", offset); > + "temp%u_input", offset + 1); > break; > case IXGBE_HWMON_TYPE_CAUTION: > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_cautionthresh; > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > - "temp%u_max", offset); > + "temp%u_max", offset + 1); > break; > case IXGBE_HWMON_TYPE_MAX: > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_maxopthresh; > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > - "temp%u_crit", offset); > + "temp%u_crit", offset + 1); > break; > default: > rc = -EPERM; Reviewed-by: Jean Delvare <kh...@li...> -- Jean Delvare |
From: Guenter R. <li...@ro...> - 2013-11-25 18:08:54
|
On Sat, Nov 23, 2013 at 02:01:58PM +0100, Jean Delvare wrote: > On Fri, 22 Nov 2013 22:08:01 -0800, Guenter Roeck wrote: > > Per hwmon ABI, temperature sensor attribute index starts with 1, not 0. > > > > Signed-off-by: Guenter Roeck <li...@ro...> > > --- > > drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > > index 3081974..e74ae36 100644 > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sysfs.c > > @@ -118,22 +118,22 @@ static int ixgbe_add_hwmon_attr(struct ixgbe_adapter *adapter, > > case IXGBE_HWMON_TYPE_LOC: > > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_location; > > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > > - "temp%u_label", offset); > > + "temp%u_label", offset + 1); > > break; > > case IXGBE_HWMON_TYPE_TEMP: > > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_temp; > > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > > - "temp%u_input", offset); > > + "temp%u_input", offset + 1); > > break; > > case IXGBE_HWMON_TYPE_CAUTION: > > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_cautionthresh; > > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > > - "temp%u_max", offset); > > + "temp%u_max", offset + 1); > > break; > > case IXGBE_HWMON_TYPE_MAX: > > ixgbe_attr->dev_attr.show = ixgbe_hwmon_show_maxopthresh; > > snprintf(ixgbe_attr->name, sizeof(ixgbe_attr->name), > > - "temp%u_crit", offset); > > + "temp%u_crit", offset + 1); > > break; > > default: > > rc = -EPERM; > > Reviewed-by: Jean Delvare <kh...@li...> > Hi Jean, thanks a lot for the reviews! Guenter |
From: Ben H. <bhu...@so...> - 2013-11-23 16:48:21
|
On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: > The hwmon subsystem is used by various network drivers to report temperature > sensor and other information. Unfortunately, its use is often not correct. > Typical errors are that the mandatory name sysfs attribute is not created, > that the temperature sensor index starts with 0 instead of 1, and/or that > sysfs attributes are created after the hwmon device was created. As it happens, I was just looking at what we do in sfc (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create the hwmon device before the attributes. I think I avoided the other bugs though. Ben. > The following sequence of patches fixes most of the problems. > > The igb patches have been tested with real hardware; the others are compile > tested only. -- 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. |
From: Guenter R. <li...@ro...> - 2013-11-23 17:33:54
|
On 11/23/2013 08:48 AM, Ben Hutchings wrote: > On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: >> The hwmon subsystem is used by various network drivers to report temperature >> sensor and other information. Unfortunately, its use is often not correct. >> Typical errors are that the mandatory name sysfs attribute is not created, >> that the temperature sensor index starts with 0 instead of 1, and/or that >> sysfs attributes are created after the hwmon device was created. > > As it happens, I was just looking at what we do in sfc > (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create > the hwmon device before the attributes. I think I avoided the other > bugs though. > Hi Ben, Yes, I know about that one. It concluded that it would be too invasive and risky to try to fix it without access to hardware to test the results. That is why I said "fixes _most_ of the problems". As for why the attributes are created after registration, it was most likely because there was no API available to attach the sysfs attributes to the hwmon device in a clean way. The new APIs fix that. Thanks, Guenter > Ben. > >> The following sequence of patches fixes most of the problems. >> >> The igb patches have been tested with real hardware; the others are compile >> tested only. > |
From: Ben H. <bhu...@so...> - 2013-11-25 17:16:02
|
On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote: > On 11/23/2013 08:48 AM, Ben Hutchings wrote: > > On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: > >> The hwmon subsystem is used by various network drivers to report temperature > >> sensor and other information. Unfortunately, its use is often not correct. > >> Typical errors are that the mandatory name sysfs attribute is not created, > >> that the temperature sensor index starts with 0 instead of 1, and/or that > >> sysfs attributes are created after the hwmon device was created. > > > > As it happens, I was just looking at what we do in sfc > > (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create > > the hwmon device before the attributes. I think I avoided the other > > bugs though. > > > Hi Ben, > > Yes, I know about that one. It concluded that it would be too invasive > and risky to try to fix it without access to hardware to test the results. > That is why I said "fixes _most_ of the problems". > > As for why the attributes are created after registration, it was most likely > because there was no API available to attach the sysfs attributes to > the hwmon device in a clean way. The new APIs fix that. We don't attach them to the hwmon device either, and I would rather not change that yet because lm-sensors 2 is still widely used. 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. |
From: Guenter R. <li...@ro...> - 2013-11-25 17:48:31
|
On 11/25/2013 09:15 AM, Ben Hutchings wrote: > On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote: >> On 11/23/2013 08:48 AM, Ben Hutchings wrote: >>> On Fri, 2013-11-22 at 22:07 -0800, Guenter Roeck wrote: >>>> The hwmon subsystem is used by various network drivers to report temperature >>>> sensor and other information. Unfortunately, its use is often not correct. >>>> Typical errors are that the mandatory name sysfs attribute is not created, >>>> that the temperature sensor index starts with 0 instead of 1, and/or that >>>> sysfs attributes are created after the hwmon device was created. >>> >>> As it happens, I was just looking at what we do in sfc >>> (drivers/net/ethernet/sfc/mcdi_mon.c) and wondering why I made it create >>> the hwmon device before the attributes. I think I avoided the other >>> bugs though. >>> >> Hi Ben, >> >> Yes, I know about that one. It concluded that it would be too invasive >> and risky to try to fix it without access to hardware to test the results. >> That is why I said "fixes _most_ of the problems". >> >> As for why the attributes are created after registration, it was most likely >> because there was no API available to attach the sysfs attributes to >> the hwmon device in a clean way. The new APIs fix that. > > We don't attach them to the hwmon device either, and I would rather not > change that yet because lm-sensors 2 is still widely used. > Hmm .. then there should be no good reason to create the attributes only after hwmon registration. As for lm-sensors 2 ... really ? Seems odd that people would use the latest kernel with 5+ years old versions of applications / libraries. but I guess the world is full of such oddities, so maybe I should not be surprised. Note that the drivers in the hwmon subsystem are expected to move to the new API over time, and some of them already did. New drivers will likely not work with lm-sensors 2. So it may be time for those still using lm-sensors 2 to consider an update, at least if they plan to use those drivers with the latest kernel. Thanks, Guenter |
From: Jean D. <kh...@li...> - 2013-11-25 18:24:06
|
On Mon, 25 Nov 2013 17:15:50 +0000, Ben Hutchings wrote: > On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote: > > Yes, I know about that one. It concluded that it would be too invasive > > and risky to try to fix it without access to hardware to test the results. > > That is why I said "fixes _most_ of the problems". > > > > As for why the attributes are created after registration, it was most likely > > because there was no API available to attach the sysfs attributes to > > the hwmon device in a clean way. The new APIs fix that. > > We don't attach them to the hwmon device either, and I would rather not > change that yet because lm-sensors 2 is still widely used. Mouahahahah. No, seriously, it's not. And lm-sensors 2 doesn't even support your device so this is a totally moot point. -- Jean Delvare |
From: Ben H. <bhu...@so...> - 2013-11-25 18:56:41
|
On Mon, 2013-11-25 at 19:23 +0100, Jean Delvare wrote: > On Mon, 25 Nov 2013 17:15:50 +0000, Ben Hutchings wrote: > > On Sat, 2013-11-23 at 09:07 -0800, Guenter Roeck wrote: > > > Yes, I know about that one. It concluded that it would be too invasive > > > and risky to try to fix it without access to hardware to test the results. > > > That is why I said "fixes _most_ of the problems". > > > > > > As for why the attributes are created after registration, it was most likely > > > because there was no API available to attach the sysfs attributes to > > > the hwmon device in a clean way. The new APIs fix that. > > > > We don't attach them to the hwmon device either, and I would rather not > > change that yet because lm-sensors 2 is still widely used. > > Mouahahahah. > > No, seriously, it's not. RHEL 5 has it, and that is widely used - even with recent mainline kernels, in some cases. > And lm-sensors 2 doesn't even support your > device so this is a totally moot point. I thought it did work with arbitrary devices providing the right attributes, but obviously I misremembered. So there's no reason not to change. Thanks. 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. |
From: Jean D. <kh...@li...> - 2013-11-25 19:34:31
|
On Mon, 25 Nov 2013 18:56:26 +0000, Ben Hutchings wrote: > On Mon, 2013-11-25 at 19:23 +0100, Jean Delvare wrote: > > On Mon, 25 Nov 2013 17:15:50 +0000, Ben Hutchings wrote: > > > We don't attach them to the hwmon device either, and I would rather not > > > change that yet because lm-sensors 2 is still widely used. > > > > Mouahahahah. > > > > No, seriously, it's not. > > RHEL 5 has it, and that is widely used - even with recent mainline > kernels, in some cases. RHEL 5 comes with kernel 2.6.18, which isn't exactly recent. I very much doubt a significant share of users dare to use a brand new kernel on such an old distribution. And if they do, then there are several packages which need to be updated (udev, kernel-firmware...), lm-sensors is only one of them, and the user should be aware of that. > > And lm-sensors 2 doesn't even support your > > device so this is a totally moot point. > > I thought it did work with arbitrary devices providing the right > attributes, but obviously I misremembered. This is how lm-sensors 3 works. But lm-sensors 2 needs explicit support for each and every device. Which is exactly why version 2 sucked and nobody should be using it any longer. > So there's no reason not to change. Thanks. Good to hear :) -- Jean Delvare |
From: Nithin N. S. <ns...@br...> - 2013-11-26 01:53:36
|
On 11/22/2013 10:07 PM, Guenter Roeck wrote: > Use new hwmon API to simplify code, provide missing mandatory 'name' > sysfs attribute, and attach hwmon attributes to hwmon device instead > of pci device. > > Signed-off-by: Guenter Roeck <li...@ro...> > --- > drivers/net/ethernet/broadcom/tg3.c | 25 ++++++------------------- > 1 file changed, 6 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c > index a9e0684..369b736 100644 > --- a/drivers/net/ethernet/broadcom/tg3.c > +++ b/drivers/net/ethernet/broadcom/tg3.c > @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir) > static ssize_t tg3_show_temp(struct device *dev, > struct device_attribute *devattr, char *buf) > { > - struct pci_dev *pdev = to_pci_dev(dev); > - struct net_device *netdev = pci_get_drvdata(pdev); > - struct tg3 *tp = netdev_priv(netdev); > struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct tg3 *tp = dev_get_drvdata(dev); Shouldn't this be struct tg3 *tp = netdev_priv(dev_get_drvdata(dev)); |
From: Guenter R. <li...@ro...> - 2013-11-26 02:47:56
|
On 11/25/2013 05:52 PM, Nithin Nayak Sujir wrote: > > > On 11/22/2013 10:07 PM, Guenter Roeck wrote: >> Use new hwmon API to simplify code, provide missing mandatory 'name' >> sysfs attribute, and attach hwmon attributes to hwmon device instead >> of pci device. >> >> Signed-off-by: Guenter Roeck <li...@ro...> >> --- >> drivers/net/ethernet/broadcom/tg3.c | 25 ++++++------------------- >> 1 file changed, 6 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c >> index a9e0684..369b736 100644 >> --- a/drivers/net/ethernet/broadcom/tg3.c >> +++ b/drivers/net/ethernet/broadcom/tg3.c >> @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, struct tg3_ocir *ocir) >> static ssize_t tg3_show_temp(struct device *dev, >> struct device_attribute *devattr, char *buf) >> { >> - struct pci_dev *pdev = to_pci_dev(dev); >> - struct net_device *netdev = pci_get_drvdata(pdev); >> - struct tg3 *tp = netdev_priv(netdev); >> struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >> + struct tg3 *tp = dev_get_drvdata(dev); > > > Shouldn't this be > struct tg3 *tp = netdev_priv(dev_get_drvdata(dev)); > 'struct tg3 *tp' is attached to the hwmon device in hwmon_device_register_with_groups(), so it can be retrieved with dev_get_drvdata() from there. Keep in mind that 'dev' is no longer the pci device but the hwmon device. Guenter |
From: Nithin N. S. <ns...@br...> - 2013-11-26 17:50:59
|
On 11/25/2013 06:47 PM, Guenter Roeck wrote: > On 11/25/2013 05:52 PM, Nithin Nayak Sujir wrote: >> >> >> On 11/22/2013 10:07 PM, Guenter Roeck wrote: >>> Use new hwmon API to simplify code, provide missing mandatory 'name' >>> sysfs attribute, and attach hwmon attributes to hwmon device instead >>> of pci device. >>> >>> Signed-off-by: Guenter Roeck <li...@ro...> >>> --- >>> drivers/net/ethernet/broadcom/tg3.c | 25 ++++++------------------- >>> 1 file changed, 6 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/broadcom/tg3.c >>> b/drivers/net/ethernet/broadcom/tg3.c >>> index a9e0684..369b736 100644 >>> --- a/drivers/net/ethernet/broadcom/tg3.c >>> +++ b/drivers/net/ethernet/broadcom/tg3.c >>> @@ -10629,10 +10629,8 @@ static void tg3_sd_scan_scratchpad(struct tg3 *tp, >>> struct tg3_ocir *ocir) >>> static ssize_t tg3_show_temp(struct device *dev, >>> struct device_attribute *devattr, char *buf) >>> { >>> - struct pci_dev *pdev = to_pci_dev(dev); >>> - struct net_device *netdev = pci_get_drvdata(pdev); >>> - struct tg3 *tp = netdev_priv(netdev); >>> struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); >>> + struct tg3 *tp = dev_get_drvdata(dev); >> >> >> Shouldn't this be >> struct tg3 *tp = netdev_priv(dev_get_drvdata(dev)); >> > > 'struct tg3 *tp' is attached to the hwmon device in > hwmon_device_register_with_groups(), so it can be retrieved > with dev_get_drvdata() from there. Keep in mind that 'dev' > is no longer the pci device but the hwmon device. > Ah, I see. Acked-by: Nithin Nayak Sujir <ns...@br...> > Guenter > > > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to maj...@vg... > More majordomo info at http://vger.kernel.org/majordomo-info.html > |