| Message ID | 1421799212-2028-1-git-send-email-vivien.didelot@savoirfairelinux.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On 01/20/2015 04:13 PM, Vivien Didelot wrote: > Set the dsa device as the parent of the hwmon device, in order to link > the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/ > sysfs directory. > > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> > --- > net/dsa/dsa.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c > index 3731714..363102a 100644 > --- a/net/dsa/dsa.c > +++ b/net/dsa/dsa.c > @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index, > hname[j] = '\0'; > scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d", > hname, index); > - ds->hwmon_dev = hwmon_device_register_with_groups(NULL, > + ds->hwmon_dev = hwmon_device_register_with_groups(parent, > ds->hwmon_name, ds, dsa_hwmon_groups); > if (IS_ERR(ds->hwmon_dev)) > ds->hwmon_dev = NULL; > I'll have to look into this again; not sure if this works because there is no 1:1 relationship between dsa device and hwmon device (or switch chip). Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/20/2015 04:13 PM, Vivien Didelot wrote: > Set the dsa device as the parent of the hwmon device, in order to link > the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/ > sysfs directory. > > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> > --- > net/dsa/dsa.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c > index 3731714..363102a 100644 > --- a/net/dsa/dsa.c > +++ b/net/dsa/dsa.c > @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index, > hname[j] = '\0'; > scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d", > hname, index); > - ds->hwmon_dev = hwmon_device_register_with_groups(NULL, > + ds->hwmon_dev = hwmon_device_register_with_groups(parent, > ds->hwmon_name, ds, dsa_hwmon_groups); > if (IS_ERR(ds->hwmon_dev)) > ds->hwmon_dev = NULL; > Looking into my old e-mail, turns out we did not add the parent device because it affected the output of the "sensors" command, and we wanted the device to be handled as 'virtual device' (which implies no parent). That was an explicit part of the patch set (v2 of 'net: dsa: Add support for reporting switch chip temperatures'), compared to v1, which did set the parent device. I would suggest to keep the code as is. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2015-01-20 19:50 GMT-08:00 Guenter Roeck <linux@roeck-us.net>: > On 01/20/2015 04:13 PM, Vivien Didelot wrote: >> >> Set the dsa device as the parent of the hwmon device, in order to link >> the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/ >> sysfs directory. >> >> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> >> --- >> net/dsa/dsa.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c >> index 3731714..363102a 100644 >> --- a/net/dsa/dsa.c >> +++ b/net/dsa/dsa.c >> @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int >> index, >> hname[j] = '\0'; >> scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), >> "%s_dsa%d", >> hname, index); >> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL, >> + ds->hwmon_dev = hwmon_device_register_with_groups(parent, >> ds->hwmon_name, ds, >> dsa_hwmon_groups); >> if (IS_ERR(ds->hwmon_dev)) >> ds->hwmon_dev = NULL; >> > > Looking into my old e-mail, turns out we did not add the parent device > because > it affected the output of the "sensors" command, and we wanted the device > to be handled as 'virtual device' (which implies no parent). That was an > explicit > part of the patch set (v2 of 'net: dsa: Add support for reporting switch > chip > temperatures'), compared to v1, which did set the parent device. > > I would suggest to keep the code as is. Maybe follow-up with a comment adding that above the call to hwmon_device_register_with_groups()? I suspect the intent is clear if you are deep into hwmon devices, but not necessarily for the reader ;) > > Thanks, > Guenter > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/
Hi Guenter, Florian, >>> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL, >>> + ds->hwmon_dev = hwmon_device_register_with_groups(parent, >>> ds->hwmon_name, ds, dsa_hwmon_groups); >>> if (IS_ERR(ds->hwmon_dev)) >>> ds->hwmon_dev = NULL; >> >> Looking into my old e-mail, turns out we did not add the parent >> device because it affected the output of the "sensors" command, and >> we wanted the device to be handled as 'virtual device' (which implies >> no parent). That was an explicit part of the patch set (v2 of 'net: >> dsa: Add support for reporting switch chip temperatures'), compared >> to v1, which did set the parent device. I don't know about the "sensors" output, but I found that convenient from the sysfs side to have a hierarchy of sub-devices logically exposed at the same place, i.e.: # ls /sys/devices/platform/dsa.0 driver hwmon net ... >> I would suggest to keep the code as is. > Maybe follow-up with a comment adding that above the call to > hwmon_device_register_with_groups()? I suspect the intent is clear if > you are deep into hwmon devices, but not necessarily for the reader ;) Thanks, -v -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c index 3731714..363102a 100644 --- a/net/dsa/dsa.c +++ b/net/dsa/dsa.c @@ -347,7 +347,7 @@ dsa_switch_setup(struct dsa_switch_tree *dst, int index, hname[j] = '\0'; scnprintf(ds->hwmon_name, sizeof(ds->hwmon_name), "%s_dsa%d", hname, index); - ds->hwmon_dev = hwmon_device_register_with_groups(NULL, + ds->hwmon_dev = hwmon_device_register_with_groups(parent, ds->hwmon_name, ds, dsa_hwmon_groups); if (IS_ERR(ds->hwmon_dev)) ds->hwmon_dev = NULL;
Set the dsa device as the parent of the hwmon device, in order to link the hwmon subsystem under the corresponding /sys/devices/platform/dsa.X/ sysfs directory. Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> --- net/dsa/dsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)