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

[Krzysztof Kozlowski <krzk@kernel.org>]

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