* [PATCH 0/2] hwmon: Add "label" attribute @ 2021-12-21 17:50 Paul Cercueil 2021-12-21 17:50 ` [PATCH 1/2] dt-bindings: hwmon: Introduce common properties Paul Cercueil 2021-12-21 17:50 ` [PATCH 2/2] hwmon: Add "label" attribute Paul Cercueil 0 siblings, 2 replies; 9+ messages in thread From: Paul Cercueil @ 2021-12-21 17:50 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring Cc: linux-hwmon, devicetree, linux-kernel, Paul Cercueil Hi Jean, Guenter, Rob, This patchset adds support for specifying a hwmon device's label from Device Tree. When the "label" device property is present, its value is exported to the userspace via the "label" sysfs attribute. This is useful for userspace to be able to identify an individual device when multiple individual chips are present in the system. Note that this mechanism already exists in IIO. Patch [1/2] adds the "label" property to a new dt-bindings/hwmon/common.yaml file. Patch [2/2] adds the change to the core drivers/hwmon/hwmon.c file. Cheers, -Paul Paul Cercueil (2): dt-bindings: hwmon: Introduce common properties hwmon: Add "label" attribute .../devicetree/bindings/hwmon/common.yaml | 31 +++++++++++++++++++ drivers/hwmon/hwmon.c | 22 ++++++++++++- 2 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/hwmon/common.yaml -- 2.34.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] dt-bindings: hwmon: Introduce common properties 2021-12-21 17:50 [PATCH 0/2] hwmon: Add "label" attribute Paul Cercueil @ 2021-12-21 17:50 ` Paul Cercueil 2021-12-22 19:36 ` Rob Herring 2021-12-21 17:50 ` [PATCH 2/2] hwmon: Add "label" attribute Paul Cercueil 1 sibling, 1 reply; 9+ messages in thread From: Paul Cercueil @ 2021-12-21 17:50 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring Cc: linux-hwmon, devicetree, linux-kernel, Paul Cercueil Introduce a file for common properties of hwmon sensors. As of now it contains only the "label" property, which can contain a descriptive label that allows to uniquely identify a device within the system. Signed-off-by: Paul Cercueil <paul@crapouillou.net> --- .../devicetree/bindings/hwmon/common.yaml | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/common.yaml diff --git a/Documentation/devicetree/bindings/hwmon/common.yaml b/Documentation/devicetree/bindings/hwmon/common.yaml new file mode 100644 index 000000000000..997f74127d8c --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/common.yaml @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/common.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Common properties for hwmon sensors + +maintainers: + - Jean Delvare <jdelvare@suse.com> + - Guenter Roeck <linux@roeck-us.net> + +description: | + This document defines device tree properties common to several hwmon + sensors. It doesn't constitue a device tree binding specification by itself but + is meant to be referenced by device tree bindings. + + When referenced from sensor tree bindings the properties defined in this + document are defined as follows. The sensor tree bindings are responsible for + defining whether each property is required or optional. + +properties: + label: + $ref: /schemas/types.yaml#/definitions/string + description: > + Descriptive label that allows to uniquely identify a device within + the system. + +additionalProperties: true + +... -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Introduce common properties 2021-12-21 17:50 ` [PATCH 1/2] dt-bindings: hwmon: Introduce common properties Paul Cercueil @ 2021-12-22 19:36 ` Rob Herring 2021-12-23 9:19 ` Paul Cercueil 2022-01-05 9:41 ` Paul Cercueil 0 siblings, 2 replies; 9+ messages in thread From: Rob Herring @ 2021-12-22 19:36 UTC (permalink / raw) To: Paul Cercueil Cc: Jean Delvare, Guenter Roeck, linux-hwmon, devicetree, linux-kernel On Tue, Dec 21, 2021 at 05:50:28PM +0000, Paul Cercueil wrote: > Introduce a file for common properties of hwmon sensors. > > As of now it contains only the "label" property, which can contain a > descriptive label that allows to uniquely identify a device within the > system. I don't think we need this. What we need is a global (in dtschema) type definition and then any users just add 'label: true'. > > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > --- > .../devicetree/bindings/hwmon/common.yaml | 31 +++++++++++++++++++ > 1 file changed, 31 insertions(+) > create mode 100644 Documentation/devicetree/bindings/hwmon/common.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/common.yaml b/Documentation/devicetree/bindings/hwmon/common.yaml > new file mode 100644 > index 000000000000..997f74127d8c > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/common.yaml > @@ -0,0 +1,31 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/common.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common properties for hwmon sensors > + > +maintainers: > + - Jean Delvare <jdelvare@suse.com> > + - Guenter Roeck <linux@roeck-us.net> > + > +description: | > + This document defines device tree properties common to several hwmon > + sensors. It doesn't constitue a device tree binding specification by itself but > + is meant to be referenced by device tree bindings. > + > + When referenced from sensor tree bindings the properties defined in this > + document are defined as follows. The sensor tree bindings are responsible for > + defining whether each property is required or optional. > + > +properties: > + label: > + $ref: /schemas/types.yaml#/definitions/string > + description: > > + Descriptive label that allows to uniquely identify a device within > + the system. > + > +additionalProperties: true > + > +... > -- > 2.34.1 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Introduce common properties 2021-12-22 19:36 ` Rob Herring @ 2021-12-23 9:19 ` Paul Cercueil 2021-12-23 10:12 ` Paul Cercueil 2022-01-05 9:41 ` Paul Cercueil 1 sibling, 1 reply; 9+ messages in thread From: Paul Cercueil @ 2021-12-23 9:19 UTC (permalink / raw) To: Rob Herring Cc: Jean Delvare, Guenter Roeck, linux-hwmon, devicetree, linux-kernel Hi Rob, Le mer., déc. 22 2021 at 15:36:23 -0400, Rob Herring <robh@kernel.org> a écrit : > On Tue, Dec 21, 2021 at 05:50:28PM +0000, Paul Cercueil wrote: >> Introduce a file for common properties of hwmon sensors. >> >> As of now it contains only the "label" property, which can contain a >> descriptive label that allows to uniquely identify a device within >> the >> system. > > I don't think we need this. What we need is a global (in dtschema) > type definition and then any users just add 'label: true'. Well, users would also need to set an actual label, otherwise this defeats the point :) Cheers, -Paul > >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> .../devicetree/bindings/hwmon/common.yaml | 31 >> +++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/hwmon/common.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/common.yaml >> b/Documentation/devicetree/bindings/hwmon/common.yaml >> new file mode 100644 >> index 000000000000..997f74127d8c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/common.yaml >> @@ -0,0 +1,31 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/common.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Common properties for hwmon sensors >> + >> +maintainers: >> + - Jean Delvare <jdelvare@suse.com> >> + - Guenter Roeck <linux@roeck-us.net> >> + >> +description: | >> + This document defines device tree properties common to several >> hwmon >> + sensors. It doesn't constitue a device tree binding >> specification by itself but >> + is meant to be referenced by device tree bindings. >> + >> + When referenced from sensor tree bindings the properties defined >> in this >> + document are defined as follows. The sensor tree bindings are >> responsible for >> + defining whether each property is required or optional. >> + >> +properties: >> + label: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: > >> + Descriptive label that allows to uniquely identify a device >> within >> + the system. >> + >> +additionalProperties: true >> + >> +... >> -- >> 2.34.1 >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Introduce common properties 2021-12-23 9:19 ` Paul Cercueil @ 2021-12-23 10:12 ` Paul Cercueil 0 siblings, 0 replies; 9+ messages in thread From: Paul Cercueil @ 2021-12-23 10:12 UTC (permalink / raw) To: Rob Herring Cc: Jean Delvare, Guenter Roeck, linux-hwmon, devicetree, linux-kernel Le jeu., déc. 23 2021 at 09:19:43 +0000, Paul Cercueil <paul@crapouillou.net> a écrit : > Hi Rob, > > Le mer., déc. 22 2021 at 15:36:23 -0400, Rob Herring > <robh@kernel.org> a écrit : >> On Tue, Dec 21, 2021 at 05:50:28PM +0000, Paul Cercueil wrote: >>> Introduce a file for common properties of hwmon sensors. >>> >>> As of now it contains only the "label" property, which can contain >>> a >>> descriptive label that allows to uniquely identify a device within >>> \x7f\x7fthe >>> system. >> >> I don't think we need this. What we need is a global (in dtschema) >> type definition and then any users just add 'label: true'. > > Well, users would also need to set an actual label, otherwise this > defeats the point :) Disregard that, I'm stupid. 'label: true' is for the schema and not the DTS files. -Paul ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] dt-bindings: hwmon: Introduce common properties 2021-12-22 19:36 ` Rob Herring 2021-12-23 9:19 ` Paul Cercueil @ 2022-01-05 9:41 ` Paul Cercueil 1 sibling, 0 replies; 9+ messages in thread From: Paul Cercueil @ 2022-01-05 9:41 UTC (permalink / raw) To: Rob Herring Cc: Jean Delvare, Guenter Roeck, linux-hwmon, devicetree, linux-kernel Hi Rob, Le mer., déc. 22 2021 at 15:36:23 -0400, Rob Herring <robh@kernel.org> a écrit : > On Tue, Dec 21, 2021 at 05:50:28PM +0000, Paul Cercueil wrote: >> Introduce a file for common properties of hwmon sensors. >> >> As of now it contains only the "label" property, which can contain a >> descriptive label that allows to uniquely identify a device within >> the >> system. > > I don't think we need this. What we need is a global (in dtschema) > type definition and then any users just add 'label: true'. I created a PR there: https://github.com/devicetree-org/dt-schema/pull/65 Cheers, -Paul > >> >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> --- >> .../devicetree/bindings/hwmon/common.yaml | 31 >> +++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> create mode 100644 >> Documentation/devicetree/bindings/hwmon/common.yaml >> >> diff --git a/Documentation/devicetree/bindings/hwmon/common.yaml >> b/Documentation/devicetree/bindings/hwmon/common.yaml >> new file mode 100644 >> index 000000000000..997f74127d8c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/hwmon/common.yaml >> @@ -0,0 +1,31 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/hwmon/common.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Common properties for hwmon sensors >> + >> +maintainers: >> + - Jean Delvare <jdelvare@suse.com> >> + - Guenter Roeck <linux@roeck-us.net> >> + >> +description: | >> + This document defines device tree properties common to several >> hwmon >> + sensors. It doesn't constitue a device tree binding >> specification by itself but >> + is meant to be referenced by device tree bindings. >> + >> + When referenced from sensor tree bindings the properties defined >> in this >> + document are defined as follows. The sensor tree bindings are >> responsible for >> + defining whether each property is required or optional. >> + >> +properties: >> + label: >> + $ref: /schemas/types.yaml#/definitions/string >> + description: > >> + Descriptive label that allows to uniquely identify a device >> within >> + the system. >> + >> +additionalProperties: true >> + >> +... >> -- >> 2.34.1 >> >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] hwmon: Add "label" attribute 2021-12-21 17:50 [PATCH 0/2] hwmon: Add "label" attribute Paul Cercueil 2021-12-21 17:50 ` [PATCH 1/2] dt-bindings: hwmon: Introduce common properties Paul Cercueil @ 2021-12-21 17:50 ` Paul Cercueil 2021-12-21 18:17 ` Guenter Roeck 1 sibling, 1 reply; 9+ messages in thread From: Paul Cercueil @ 2021-12-21 17:50 UTC (permalink / raw) To: Jean Delvare, Guenter Roeck, Rob Herring Cc: linux-hwmon, devicetree, linux-kernel, Paul Cercueil, Cosmin Tanislav If a label is defined in the device tree for this device add that to the device specific attributes. This is useful for userspace to be able to identify an individual device when multiple identical chips are present in the system. Signed-off-by: Paul Cercueil <paul@crapouillou.net> Tested-by: Cosmin Tanislav <cosmin.tanislav@analog.com> --- drivers/hwmon/hwmon.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index 3501a3ead4ba..15826260a463 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -71,8 +71,23 @@ name_show(struct device *dev, struct device_attribute *attr, char *buf) } static DEVICE_ATTR_RO(name); +static ssize_t +label_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + const char *label; + int ret; + + ret = device_property_read_string(dev, "label", &label); + if (ret < 0) + return ret; + + return sysfs_emit(buf, "%s\n", label); +} +static DEVICE_ATTR_RO(label); + static struct attribute *hwmon_dev_attrs[] = { &dev_attr_name.attr, + &dev_attr_label.attr, NULL }; @@ -81,7 +96,12 @@ static umode_t hwmon_dev_name_is_visible(struct kobject *kobj, { struct device *dev = kobj_to_dev(kobj); - if (to_hwmon_device(dev)->name == NULL) + if (attr == &dev_attr_name.attr && + to_hwmon_device(dev)->name == NULL) + return 0; + + if (attr == &dev_attr_label.attr && + !device_property_present(dev, "label")) return 0; return attr->mode; -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] hwmon: Add "label" attribute 2021-12-21 17:50 ` [PATCH 2/2] hwmon: Add "label" attribute Paul Cercueil @ 2021-12-21 18:17 ` Guenter Roeck 2021-12-22 14:18 ` Paul Cercueil 0 siblings, 1 reply; 9+ messages in thread From: Guenter Roeck @ 2021-12-21 18:17 UTC (permalink / raw) To: Paul Cercueil, Jean Delvare, Rob Herring Cc: linux-hwmon, devicetree, linux-kernel, Cosmin Tanislav On 12/21/21 9:50 AM, Paul Cercueil wrote: > If a label is defined in the device tree for this device add that > to the device specific attributes. This is useful for userspace to > be able to identify an individual device when multiple identical > chips are present in the system. > This is an ABI change which needs to be documented in Documentation/hwmon/sysfs-interface.rst. > Signed-off-by: Paul Cercueil <paul@crapouillou.net> > Tested-by: Cosmin Tanislav <cosmin.tanislav@analog.com> > --- > drivers/hwmon/hwmon.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c > index 3501a3ead4ba..15826260a463 100644 > --- a/drivers/hwmon/hwmon.c > +++ b/drivers/hwmon/hwmon.c > @@ -71,8 +71,23 @@ name_show(struct device *dev, struct device_attribute *attr, char *buf) > } > static DEVICE_ATTR_RO(name); > > +static ssize_t > +label_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + const char *label; > + int ret; > + > + ret = device_property_read_string(dev, "label", &label); Requires "#include <linux/property.h>". Also, reading and verifying the label each time it is read is excessive. More on that see below. > + if (ret < 0) > + return ret; > + > + return sysfs_emit(buf, "%s\n", label); > +} > +static DEVICE_ATTR_RO(label); > + > static struct attribute *hwmon_dev_attrs[] = { > &dev_attr_name.attr, > + &dev_attr_label.attr, > NULL > }; > > @@ -81,7 +96,12 @@ static umode_t hwmon_dev_name_is_visible(struct kobject *kobj, That function name is no longer appropriate and should be changed to something like hwmon_dev_attr_is_visible. > { > struct device *dev = kobj_to_dev(kobj); > > - if (to_hwmon_device(dev)->name == NULL) > + if (attr == &dev_attr_name.attr && > + to_hwmon_device(dev)->name == NULL) Unnecessary continuation line. > + return 0; > + > + if (attr == &dev_attr_label.attr && > + !device_property_present(dev, "label")) If the property is present but not a string, each read of "label" would return a runtime error. I don't like that. I would suggest to store a pointer to the label in struct hwmon_device, set it during registration (eg by using devm_strdup() if it is defined), and use if (attr == &dev_attr_label.attr && to_hwmon_device(dev)->label == NULL) return 0; to check if it is present. > return 0; > > return attr->mode; > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] hwmon: Add "label" attribute 2021-12-21 18:17 ` Guenter Roeck @ 2021-12-22 14:18 ` Paul Cercueil 0 siblings, 0 replies; 9+ messages in thread From: Paul Cercueil @ 2021-12-22 14:18 UTC (permalink / raw) To: Guenter Roeck Cc: Jean Delvare, Rob Herring, linux-hwmon, devicetree, linux-kernel, Cosmin Tanislav Hi Guenter, Comments noted, thanks. I'll send a V2 later today. Cheers, -Paul Le mar., déc. 21 2021 at 10:17:03 -0800, Guenter Roeck <linux@roeck-us.net> a écrit : > On 12/21/21 9:50 AM, Paul Cercueil wrote: >> If a label is defined in the device tree for this device add that >> to the device specific attributes. This is useful for userspace to >> be able to identify an individual device when multiple identical >> chips are present in the system. >> > > This is an ABI change which needs to be documented in > Documentation/hwmon/sysfs-interface.rst. > >> Signed-off-by: Paul Cercueil <paul@crapouillou.net> >> Tested-by: Cosmin Tanislav <cosmin.tanislav@analog.com> >> --- >> drivers/hwmon/hwmon.c | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c >> index 3501a3ead4ba..15826260a463 100644 >> --- a/drivers/hwmon/hwmon.c >> +++ b/drivers/hwmon/hwmon.c >> @@ -71,8 +71,23 @@ name_show(struct device *dev, struct >> device_attribute *attr, char *buf) >> } >> static DEVICE_ATTR_RO(name); >> \x7f+static ssize_t >> +label_show(struct device *dev, struct device_attribute *attr, char >> *buf) >> +{ >> + const char *label; >> + int ret; >> + >> + ret = device_property_read_string(dev, "label", &label); > > Requires "#include <linux/property.h>". Also, reading and verifying > the label > each time it is read is excessive. More on that see below. > >> + if (ret < 0) >> + return ret; >> + >> + return sysfs_emit(buf, "%s\n", label); >> +} >> +static DEVICE_ATTR_RO(label); >> + >> static struct attribute *hwmon_dev_attrs[] = { >> &dev_attr_name.attr, >> + &dev_attr_label.attr, >> NULL >> }; >> \x7f@@ -81,7 +96,12 @@ static umode_t >> hwmon_dev_name_is_visible(struct kobject *kobj, > > That function name is no longer appropriate and should be changed to > something like hwmon_dev_attr_is_visible. > >> { >> struct device *dev = kobj_to_dev(kobj); >> \x7f- if (to_hwmon_device(dev)->name == NULL) >> + if (attr == &dev_attr_name.attr && >> + to_hwmon_device(dev)->name == NULL) > > Unnecessary continuation line. > >> + return 0; >> + >> + if (attr == &dev_attr_label.attr && >> + !device_property_present(dev, "label")) > > If the property is present but not a string, each read of "label" > would > return a runtime error. I don't like that. I would suggest to store > a pointer to the label in struct hwmon_device, set it during > registration > (eg by using devm_strdup() if it is defined), and use > > if (attr == &dev_attr_label.attr && to_hwmon_device(dev)->label == > NULL) > return 0; > > to check if it is present. > >> return 0; >> \x7f return attr->mode; >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-01-05 9:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-21 17:50 [PATCH 0/2] hwmon: Add "label" attribute Paul Cercueil 2021-12-21 17:50 ` [PATCH 1/2] dt-bindings: hwmon: Introduce common properties Paul Cercueil 2021-12-22 19:36 ` Rob Herring 2021-12-23 9:19 ` Paul Cercueil 2021-12-23 10:12 ` Paul Cercueil 2022-01-05 9:41 ` Paul Cercueil 2021-12-21 17:50 ` [PATCH 2/2] hwmon: Add "label" attribute Paul Cercueil 2021-12-21 18:17 ` Guenter Roeck 2021-12-22 14:18 ` Paul Cercueil
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.