diff mbox series

[v3,1/3] dt-bindings: hwmon: Add Microchip emc2305 yaml schema

Message ID 20250304162136.1963384-2-florin.leotescu@oss.nxp.com (mailing list archive)
State New
Headers show
Series Add OF support for Microchip emc2305 fan controller | expand

Commit Message

Florin Leotescu (OSS) March 4, 2025, 4:21 p.m. UTC
From: Florin Leotescu <florin.leotescu@nxp.com>

Introduce yaml schema for Microchip emc2305 pwm fan controller.

Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
---
 .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml

Comments

Frank Li March 4, 2025, 4:27 p.m. UTC | #1
On Tue, Mar 04, 2025 at 06:21:34PM +0200, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
>
> Introduce yaml schema for Microchip emc2305 pwm fan controller.
>
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> new file mode 100644
> index 000000000000..cac0075a65bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip EMC2305 SMBus compliant PWM fan controller
> +
> +maintainers:
> +  - Michael Shych <michaelsh@nvidia.com>

who is it? look like not Microchip and hwmon maintainer.

> +
> +description: |

Needn't |

> +  Microchip EMC2301/2/3/5 pwm controller which supports up
> +  to five programmable fan control circuits
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,emc2301
> +      - microchip,emc2302
> +      - microchip,emc2303
> +      - microchip,emc2305

According to your driver code look like all compatible with microchip,emc2301

oneOf:
  - enum:
      - microchip,emc2301
  - items:
      - enum:
          - microchip,emc2302
          - microchip,emc2303
          - microchip,emc2305
      - const: microchip,emc2301

So your driver just need one "microchip,emc2301" compatible string.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        emc2301: pwm@2f {
> +            compatible = "microchip,emc2301";
> +            reg = <0x2f>;

If only one reg:
You can go through trivial-devices.yaml

Frank

> +        };
> +    };
> --
> 2.34.1
>
Conor Dooley March 4, 2025, 4:29 p.m. UTC | #2
On Tue, Mar 04, 2025 at 06:21:34PM +0200, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
> 
> Introduce yaml schema for Microchip emc2305 pwm fan controller.
> 
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> new file mode 100644
> index 000000000000..cac0075a65bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip EMC2305 SMBus compliant PWM fan controller
> +
> +maintainers:
> +  - Michael Shych <michaelsh@nvidia.com>
> +
> +description: |
> +  Microchip EMC2301/2/3/5 pwm controller which supports up
> +  to five programmable fan control circuits
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,emc2301
> +      - microchip,emc2302
> +      - microchip,emc2303
> +      - microchip,emc2305

Driver has no match data etc, suggesting all of these are compatible.
Please pick one as a fallback compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg

Missing descriptions of the fans, no?
See max6639 for an example.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        emc2301: pwm@2f {

The emc2301 label here can be dropped, it is not used.
The nodename should be "fan-controller", not pwm here I guess.

Cheers,
Conor.

> +            compatible = "microchip,emc2301";
> +            reg = <0x2f>;
> +        };
> +    };
> -- 
> 2.34.1
>
Krzysztof Kozlowski March 4, 2025, 4:31 p.m. UTC | #3
On 04/03/2025 17:29, Conor Dooley wrote:
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - microchip,emc2301
>> +      - microchip,emc2302
>> +      - microchip,emc2303
>> +      - microchip,emc2305
> 
> Driver has no match data etc, suggesting all of these are compatible.
> Please pick one as a fallback compatible.
> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
> 
> Missing descriptions of the fans, no?
> See max6639 for an example.

Yes and missing $ref to fan-controller schema.

Best regards,
Krzysztof
Krzysztof Kozlowski March 4, 2025, 4:32 p.m. UTC | #4
On 04/03/2025 17:31, Krzysztof Kozlowski wrote:
> On 04/03/2025 17:29, Conor Dooley wrote:
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - microchip,emc2301
>>> +      - microchip,emc2302
>>> +      - microchip,emc2303
>>> +      - microchip,emc2305
>>
>> Driver has no match data etc, suggesting all of these are compatible.
>> Please pick one as a fallback compatible.
>>
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>
>> Missing descriptions of the fans, no?
>> See max6639 for an example.
> 
> Yes and missing $ref to fan-controller schema.
About which I asked at v2...

Best regards,
Krzysztof
Krzysztof Kozlowski March 4, 2025, 4:33 p.m. UTC | #5
On 04/03/2025 17:21, florin.leotescu@oss.nxp.com wrote:
> From: Florin Leotescu <florin.leotescu@nxp.com>
> 
> Introduce yaml schema for Microchip emc2305 pwm fan controller.
> 
> Signed-off-by: Florin Leotescu <florin.leotescu@nxp.com>
> ---
>  .../bindings/hwmon/microchip,emc2305.yaml     | 43 +++++++++++++++++++

I do not see improvements in the subject nor in missing $ref to common
schema for fans.

<form letter>
This is a friendly reminder during the review process.

It looks like you received a tag and forgot to add it.

If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions
of patchset, under or above your Signed-off-by tag, unless patch changed
significantly (e.g. new properties added to the DT bindings). Tag is
"received", when provided in a message replied to you on the mailing
list. Tools like b4 can help here. However, there's no need to repost
patches *only* to add the tags. The upstream maintainer will do that for
tags received on the version they apply.

Please read:
https://elixir.bootlin.com/linux/v6.12-rc3/source/Documentation/process/submitting-patches.rst#L577

If a tag was not added on purpose, please state why and what changed.
</form letter>

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
new file mode 100644
index 000000000000..cac0075a65bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/microchip,emc2305.yaml
@@ -0,0 +1,43 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/microchip,emc2305.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip EMC2305 SMBus compliant PWM fan controller
+
+maintainers:
+  - Michael Shych <michaelsh@nvidia.com>
+
+description: |
+  Microchip EMC2301/2/3/5 pwm controller which supports up
+  to five programmable fan control circuits
+
+properties:
+  compatible:
+    enum:
+      - microchip,emc2301
+      - microchip,emc2302
+      - microchip,emc2303
+      - microchip,emc2305
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        emc2301: pwm@2f {
+            compatible = "microchip,emc2301";
+            reg = <0x2f>;
+        };
+    };