diff mbox series

[2/2] dt-bindings: hwmon: Add description for new hwmon driver hs300x

Message ID 20230711140637.4909-2-andre.werner@systec-electronic.com (mailing list archive)
State Superseded
Headers show
Series [1/2] hwmon: (hs300x) Add driver for Renesas HS300x | expand

Commit Message

werneazc July 11, 2023, 2:06 p.m. UTC 
This is the initial description.

Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
---
 .../devicetree/bindings/hwmon/hs300x.yaml     | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/hs300x.yaml

Comments

Krzysztof Kozlowski July 11, 2023, 4:28 p.m. UTC | #1 
On 11/07/2023 16:06, Andre Werner wrote:
> This is the initial description.
> 
> Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
> ---
>  .../devicetree/bindings/hwmon/hs300x.yaml     | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/hs300x.yaml

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC. It might happen, that command when run on an older
kernel, gives you outdated entries. Therefore please be sure you base
your patches on recent Linux kernel.

You missed at least DT list (maybe more), so this won't be tested by
automated tooling. Performing review on untested code might be a waste
of time, thus I will skip this patch entirely till you follow the
process allowing the patch to be tested.

Please kindly resend and include all necessary To/Cc entries.

> 
> diff --git a/Documentation/devicetree/bindings/hwmon/hs300x.yaml b/Documentation/devicetree/bindings/hwmon/hs300x.yaml
> new file mode 100644
> index 000000000000..7a9b3ee3738b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/hs300x.yaml

Filename like compatible

> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +

Drop

> +$id: http://devicetree.org/schemas/hwmon/hs300x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas HS300x humidity and temperature sensor
> +
> +maintainers:
> +  - Andre Werner (andre.werner@systec-electronic.com)
> +
> +description: |
> +  This driver implements support for the Renesas HS300x chips, a humidity

Bindings are for hardware, not drivers.

> +  and temperature family. Temperature is measured in degrees celsius, relative
> +  humidity is expressed as a percentage. In the sysfs interface, all values are
> +  scaled by 1000, i.e. the value for 31.5 degrees celsius is 31500.

sysfs is not hardware related, drop.

> +
> +  The device communicates with the I2C protocol. Sensors can have the I2C
> +  address 0x44 by default.
> +
> +  The driver does not support the sensor's configuration possibilities.
> +
> +  The driver is able to be read out using lmsensors.
> +
> +  Datasheets:
> +  https://www.renesas.com/us/en/document/dst/hs300x-datasheet?r=417401
> +
> +

Just one blank line.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - hs3001
> +      - hs3002
> +      - hs3003
> +      - hs3004

Eh, so this was not tested... That's not correct compatible, missing
vendor prefix.

> +
> +  reg:
> +    maxItems: 1
> +

Missing several properties.

> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          humidity: hs3002@44 {

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


> +              compatible = "hs3002";
> +              reg = <0x44>;
> +          };
> +    };

Best regards,
Krzysztof
Guenter Roeck July 11, 2023, 5:41 p.m. UTC | #2 
On Tue, Jul 11, 2023 at 06:28:11PM +0200, Krzysztof Kozlowski wrote:
> On 11/07/2023 16:06, Andre Werner wrote:
> > This is the initial description.
> > 
> > Signed-off-by: Andre Werner <andre.werner@systec-electronic.com>
> > ---
> >  .../devicetree/bindings/hwmon/hs300x.yaml     | 58 +++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/hwmon/hs300x.yaml
> 
> Please use scripts/get_maintainers.pl to get a list of necessary people
> and lists to CC. It might happen, that command when run on an older
> kernel, gives you outdated entries. Therefore please be sure you base
> your patches on recent Linux kernel.
> 
> You missed at least DT list (maybe more), so this won't be tested by
> automated tooling. Performing review on untested code might be a waste
> of time, thus I will skip this patch entirely till you follow the
> process allowing the patch to be tested.
> 
> Please kindly resend and include all necessary To/Cc entries.
> 

The chips should just be added to trivial-devices.yaml.

> > 
> > diff --git a/Documentation/devicetree/bindings/hwmon/hs300x.yaml b/Documentation/devicetree/bindings/hwmon/hs300x.yaml
> > new file mode 100644
> > index 000000000000..7a9b3ee3738b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/hs300x.yaml
> 
> Filename like compatible
> 
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> 
> Drop
> 
> > +$id: http://devicetree.org/schemas/hwmon/hs300x.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Renesas HS300x humidity and temperature sensor
> > +
> > +maintainers:
> > +  - Andre Werner (andre.werner@systec-electronic.com)
> > +
> > +description: |
> > +  This driver implements support for the Renesas HS300x chips, a humidity
> 
> Bindings are for hardware, not drivers.
> 
> > +  and temperature family. Temperature is measured in degrees celsius, relative
> > +  humidity is expressed as a percentage. In the sysfs interface, all values are
> > +  scaled by 1000, i.e. the value for 31.5 degrees celsius is 31500.
> 
> sysfs is not hardware related, drop.
> 
> > +
> > +  The device communicates with the I2C protocol. Sensors can have the I2C
> > +  address 0x44 by default.
> > +
> > +  The driver does not support the sensor's configuration possibilities.
> > +
> > +  The driver is able to be read out using lmsensors.
> > +
> > +  Datasheets:
> > +  https://www.renesas.com/us/en/document/dst/hs300x-datasheet?r=417401
> > +
> > +
> 
> Just one blank line.
> 
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - hs3001
> > +      - hs3002
> > +      - hs3003
> > +      - hs3004
> 
> Eh, so this was not tested... That's not correct compatible, missing
> vendor prefix.
> 

Also, from programming perspective, the chips are all the same.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> 
> Missing several properties.
> 
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +
> > +          humidity: hs3002@44 {
> 
> 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
> 
> 
> > +              compatible = "hs3002";
> > +              reg = <0x44>;
> > +          };
> > +    };
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/hs300x.yaml b/Documentation/devicetree/bindings/hwmon/hs300x.yaml
new file mode 100644
index 000000000000..7a9b3ee3738b
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/hs300x.yaml
@@ -0,0 +1,58 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/hwmon/hs300x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas HS300x humidity and temperature sensor
+
+maintainers:
+  - Andre Werner (andre.werner@systec-electronic.com)
+
+description: |
+  This driver implements support for the Renesas HS300x chips, a humidity
+  and temperature family. Temperature is measured in degrees celsius, relative
+  humidity is expressed as a percentage. In the sysfs interface, all values are
+  scaled by 1000, i.e. the value for 31.5 degrees celsius is 31500.
+
+  The device communicates with the I2C protocol. Sensors can have the I2C
+  address 0x44 by default.
+
+  The driver does not support the sensor's configuration possibilities.
+
+  The driver is able to be read out using lmsensors.
+
+  Datasheets:
+  https://www.renesas.com/us/en/document/dst/hs300x-datasheet?r=417401
+
+
+
+properties:
+  compatible:
+    enum:
+      - hs3001
+      - hs3002
+      - hs3003
+      - hs3004
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          humidity: hs3002@44 {
+              compatible = "hs3002";
+              reg = <0x44>;
+          };
+    };