* Re: Re: [PATCH v4 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor [not found] ` <20260503-brave-bullfinch-of-innovation-942914@quoll> @ 2026-05-12 9:16 ` Huan He 2026-05-12 14:26 ` Guenter Roeck 0 siblings, 1 reply; 2+ messages in thread From: Huan He @ 2026-05-12 9:16 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree, linux-kernel, ningyu, linmin, pinkesh.vaghela, luyulin Hi Krzysztof, Thank you very much for your detailed review. We appreciate the feedback. > On Thu, Apr 30, 2026 at 02:44:44PM +0800, hehuan1@eswincomputing.com wrote: > > + > > + label: > > + enum: > > + - pvt0 > > + - pvt1 > > No, label is user-visible name. Can be whatever user decides. > > Please read writing bindings - instance IDs are not allowed. Thanks for the clarification. I am planning to update the next revision as follows. Would this be acceptable? YAML: - label: - enum: - - pvt0 - - pvt1 + label: true required: - compatible - reg - clocks - interrupts - - label Driver: static int eic7700_pvt_create_hwmon(struct pvt_hwmon *pvt) { - struct device *dev = pvt->dev; - struct device_node *np = dev->of_node; - const char *node_label; - int type; - const char *names[2] = {"soc_pvt", "ddr_pvt"}; - - if (of_property_read_string(np, "label", &node_label)) { - dev_err(dev, "Missing 'label' property in DTS node\n"); - return -EINVAL; - } - - if (strcmp(node_label, "pvt0") == 0) { - type = 0; - } else if (strcmp(node_label, "pvt1") == 0) { - type = 1; - } else { - dev_err(pvt->dev, "Unsupported label: %s\n", node_label); - return -EINVAL; - } + const char *name = "pvt"; + + of_property_read_string(pvt->dev->of_node, "label", &name); - pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, names[type], + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, name, pvt, &pvt_hwmon_info, NULL); > > > + > > + resets: > > + maxItems: 1 > > + > > + '#thermal-sensor-cells': > > + const: 0 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - interrupts > > + - label > > + - resets > > + > > +unevaluatedProperties: false > > + > > +examples: > > + - | > > + pvt@50b00000 { > > > Node names should be generic. See also an explanation and list of > examples (not exhaustive) in DT specification: > https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > If you cannot find a name matching your device, please check in kernel > sources for similar cases or you can grow the spec (via pull request to > DT spec repo). I will update the example node name from "pvt@..." to the generic "sensor@...". Is this acceptable? Best regards, Huan He ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v4 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor 2026-05-12 9:16 ` Re: [PATCH v4 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor Huan He @ 2026-05-12 14:26 ` Guenter Roeck 0 siblings, 0 replies; 2+ messages in thread From: Guenter Roeck @ 2026-05-12 14:26 UTC (permalink / raw) To: Huan He, Krzysztof Kozlowski Cc: robh, krzk+dt, conor+dt, p.zabel, linux-hwmon, devicetree, linux-kernel, ningyu, linmin, pinkesh.vaghela, luyulin On 5/12/26 02:16, Huan He wrote: > Hi Krzysztof, > > Thank you very much for your detailed review. We appreciate the feedback. > >> On Thu, Apr 30, 2026 at 02:44:44PM +0800, hehuan1@eswincomputing.com wrote: >>> + >>> + label: >>> + enum: >>> + - pvt0 >>> + - pvt1 >> >> No, label is user-visible name. Can be whatever user decides. >> >> Please read writing bindings - instance IDs are not allowed. > > Thanks for the clarification. > I am planning to update the next revision as follows. Would this be > acceptable? > > YAML: > - label: > - enum: > - - pvt0 > - - pvt1 > + label: true > > required: > - compatible > - reg > - clocks > - interrupts > - - label > > Driver: > static int eic7700_pvt_create_hwmon(struct pvt_hwmon *pvt) > { > - struct device *dev = pvt->dev; > - struct device_node *np = dev->of_node; > - const char *node_label; > - int type; > - const char *names[2] = {"soc_pvt", "ddr_pvt"}; > - > - if (of_property_read_string(np, "label", &node_label)) { > - dev_err(dev, "Missing 'label' property in DTS node\n"); > - return -EINVAL; > - } > - > - if (strcmp(node_label, "pvt0") == 0) { > - type = 0; > - } else if (strcmp(node_label, "pvt1") == 0) { > - type = 1; > - } else { > - dev_err(pvt->dev, "Unsupported label: %s\n", node_label); > - return -EINVAL; > - } > + const char *name = "pvt"; > + > + of_property_read_string(pvt->dev->of_node, "label", &name); > > - pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, names[type], > + pvt->hwmon = devm_hwmon_device_register_with_info(pvt->dev, name, > pvt, &pvt_hwmon_info, > NULL); > This would try to register a free-text name for the hwmon device, which would be unacceptable. There are lots of multi-channel devices out there. None of them have those problems. Why do you insist in free-text names instead of using, say, "reg" to distinguish the channels ? Guenter >> >>> + >>> + resets: >>> + maxItems: 1 >>> + >>> + '#thermal-sensor-cells': >>> + const: 0 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - clocks >>> + - interrupts >>> + - label >>> + - resets >>> + >>> +unevaluatedProperties: false >>> + >>> +examples: >>> + - | >>> + pvt@50b00000 { >> >> >> Node names should be generic. See also an explanation and list of >> examples (not exhaustive) in DT specification: >> https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation >> If you cannot find a name matching your device, please check in kernel >> sources for similar cases or you can grow the spec (via pull request to >> DT spec repo). > > I will update the example node name from "pvt@..." to the generic > "sensor@...". Is this acceptable? > > Best regards, > Huan He ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-12 14:26 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260430064107.1598-1-hehuan1@eswincomputing.com>
[not found] ` <20260430064444.1615-1-hehuan1@eswincomputing.com>
[not found] ` <20260503-brave-bullfinch-of-innovation-942914@quoll>
2026-05-12 9:16 ` Re: [PATCH v4 1/2] dt-bindings: hwmon: Add Eswin EIC7700 PVT sensor Huan He
2026-05-12 14:26 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox