| Message ID | 20240525102914.22634-2-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] dt-bindings: hwmon: g762: Convert to yaml schema | expand |
On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote: > On 5/25/24 03:29, Christian Marangi wrote: > > Add support for g761 PWM Fan controller. This is an exact copy of g763 > > with the difference that it does also support an internal clock > > oscillators. > > > > Add required logic to support this additional feature with the property > > gmt,internal-clock and reject invalid schema that define both > > internal-clock property and external clocks. > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > --- > > .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++-- > > 1 file changed, 40 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > index bfefe49f54bf..d6e80392d045 100644 > > --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > @@ -4,13 +4,13 @@ > > $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: GMT G762/G763 PWM Fan controller > > +title: GMT G761/G762/G763 PWM Fan controller > > maintainers: > > - Christian Marangi <ansuelsmth@gmail.com> > > description: | > > - GMT G762/G763 PWM Fan controller. > > + GMT G761/G762/G763 PWM Fan controller. > > If an optional property is not set in DT, then current value is kept > > unmodified (e.g. bootloader installed value). > > @@ -22,6 +22,7 @@ description: | > > properties: > > compatible: > > enum: > > + - gmt,g761 > > - gmt,g762 > > - gmt,g763 > > @@ -48,10 +49,37 @@ properties: > > $ref: /schemas/types.yaml#/definitions/uint32 > > enum: [0, 1, 2] > > + gmt,internal-clock: > > + description: Use the Internal clock instead of externally attached one > > + via the CLK pin. > > + type: boolean > > + > > required: > > - compatible > > - reg > > - - clocks > > + > > +allOf: > > + - if: > > + properties: > > + compatible: > > + contains: > > + enum: > > + - gmt,g762 > > + - gmt,g763 > > + then: > > + properties: > > + gmt,internal-clock: false > > + > > + required: > > + - clocks > > Is the new property even necessary ? The absence of an external clock on G761 > could be used to imply that the internal clock is used. > Well with how the driver works, if a property is not defined, then the value is not set and the one set by the bootloader or from device reset is keept. This conflicts with the logic of no clock = internal. But yes if asked I can drop that, I can totally see your point since the clocks is a required property anyway so it's always set. > > > + > > + - if: > > + required: > > + - gmt,internal-clock > > + > > + then: > > + properties: > > + clocks: false > > additionalProperties: false > > @@ -80,4 +108,13 @@ examples: > > fan_startv = <1>; > > pwm_polarity = <0>; > > }; > > + > > + g761@1e { > > + compatible = "gmt,g761"; > > + reg = <0x1e>; > > + gmt,internal-clock; > > + fan_gear_mode = <0>; > > + fan_startv = <1>; > > + pwm_polarity = <0>; > > + }; > > }; >
On Sat, May 25, 2024 at 07:53:57AM -0700, Guenter Roeck wrote: > On 5/25/24 03:50, Christian Marangi wrote: > > On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote: > > > On 5/25/24 03:29, Christian Marangi wrote: > > > > Add support for g761 PWM Fan controller. This is an exact copy of g763 > > > > with the difference that it does also support an internal clock > > > > oscillators. > > > > > > > > Add required logic to support this additional feature with the property > > > > gmt,internal-clock and reject invalid schema that define both > > > > internal-clock property and external clocks. > > > > > > > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > > > > --- > > > > .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++-- > > > > 1 file changed, 40 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > > > index bfefe49f54bf..d6e80392d045 100644 > > > > --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > > > +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > > > > @@ -4,13 +4,13 @@ > > > > $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml# > > > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: GMT G762/G763 PWM Fan controller > > > > +title: GMT G761/G762/G763 PWM Fan controller > > > > maintainers: > > > > - Christian Marangi <ansuelsmth@gmail.com> > > > > description: | > > > > - GMT G762/G763 PWM Fan controller. > > > > + GMT G761/G762/G763 PWM Fan controller. > > > > If an optional property is not set in DT, then current value is kept > > > > unmodified (e.g. bootloader installed value). > > > > @@ -22,6 +22,7 @@ description: | > > > > properties: > > > > compatible: > > > > enum: > > > > + - gmt,g761 > > > > - gmt,g762 > > > > - gmt,g763 > > > > @@ -48,10 +49,37 @@ properties: > > > > $ref: /schemas/types.yaml#/definitions/uint32 > > > > enum: [0, 1, 2] > > > > + gmt,internal-clock: > > > > + description: Use the Internal clock instead of externally attached one > > > > + via the CLK pin. > > > > + type: boolean > > > > + > > > > required: > > > > - compatible > > > > - reg > > > > - - clocks > > > > + > > > > +allOf: > > > > + - if: > > > > + properties: > > > > + compatible: > > > > + contains: > > > > + enum: > > > > + - gmt,g762 > > > > + - gmt,g763 > > > > + then: > > > > + properties: > > > > + gmt,internal-clock: false > > > > + > > > > + required: > > > > + - clocks > > > > > > Is the new property even necessary ? The absence of an external clock on G761 > > > could be used to imply that the internal clock is used. > > > > > > > Well with how the driver works, if a property is not defined, then the > > value is not set and the one set by the bootloader or from device reset > > is keept. > > > > This conflicts with the logic of no clock = internal. > > > > Not sure I understand the problem. It would be a simple change in the driver > to add "if the chip is G761 and the clock is not provided in DT, use the > internal clock". > Yes sure code wise is pretty easy, my concern is more of losing this info in DT. But anyway ok will drop in V2. Thanks a lot for the review!
On 5/25/24 03:29, Christian Marangi wrote: > Add support for g761 PWM Fan controller. This is an exact copy of g763 > with the difference that it does also support an internal clock > oscillators. > > Add required logic to support this additional feature with the property > gmt,internal-clock and reject invalid schema that define both > internal-clock property and external clocks. > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> > --- > .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++-- > 1 file changed, 40 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > index bfefe49f54bf..d6e80392d045 100644 > --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml > @@ -4,13 +4,13 @@ > $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: GMT G762/G763 PWM Fan controller > +title: GMT G761/G762/G763 PWM Fan controller > > maintainers: > - Christian Marangi <ansuelsmth@gmail.com> > > description: | > - GMT G762/G763 PWM Fan controller. > + GMT G761/G762/G763 PWM Fan controller. > > If an optional property is not set in DT, then current value is kept > unmodified (e.g. bootloader installed value). > @@ -22,6 +22,7 @@ description: | > properties: > compatible: > enum: > + - gmt,g761 > - gmt,g762 > - gmt,g763 > > @@ -48,10 +49,37 @@ properties: > $ref: /schemas/types.yaml#/definitions/uint32 > enum: [0, 1, 2] > > + gmt,internal-clock: > + description: Use the Internal clock instead of externally attached one > + via the CLK pin. > + type: boolean > + > required: > - compatible > - reg > - - clocks > + > +allOf: > + - if: > + properties: > + compatible: > + contains: > + enum: > + - gmt,g762 > + - gmt,g763 > + then: > + properties: > + gmt,internal-clock: false > + > + required: > + - clocks Is the new property even necessary ? The absence of an external clock on G761 could be used to imply that the internal clock is used. Guenter > + > + - if: > + required: > + - gmt,internal-clock > + > + then: > + properties: > + clocks: false > > additionalProperties: false > > @@ -80,4 +108,13 @@ examples: > fan_startv = <1>; > pwm_polarity = <0>; > }; > + > + g761@1e { > + compatible = "gmt,g761"; > + reg = <0x1e>; > + gmt,internal-clock; > + fan_gear_mode = <0>; > + fan_startv = <1>; > + pwm_polarity = <0>; > + }; > };
On 5/25/24 03:50, Christian Marangi wrote: > On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote: >> On 5/25/24 03:29, Christian Marangi wrote: >>> Add support for g761 PWM Fan controller. This is an exact copy of g763 >>> with the difference that it does also support an internal clock >>> oscillators. >>> >>> Add required logic to support this additional feature with the property >>> gmt,internal-clock and reject invalid schema that define both >>> internal-clock property and external clocks. >>> >>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> >>> --- >>> .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++-- >>> 1 file changed, 40 insertions(+), 3 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml >>> index bfefe49f54bf..d6e80392d045 100644 >>> --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml >>> +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml >>> @@ -4,13 +4,13 @@ >>> $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml# >>> $schema: http://devicetree.org/meta-schemas/core.yaml# >>> -title: GMT G762/G763 PWM Fan controller >>> +title: GMT G761/G762/G763 PWM Fan controller >>> maintainers: >>> - Christian Marangi <ansuelsmth@gmail.com> >>> description: | >>> - GMT G762/G763 PWM Fan controller. >>> + GMT G761/G762/G763 PWM Fan controller. >>> If an optional property is not set in DT, then current value is kept >>> unmodified (e.g. bootloader installed value). >>> @@ -22,6 +22,7 @@ description: | >>> properties: >>> compatible: >>> enum: >>> + - gmt,g761 >>> - gmt,g762 >>> - gmt,g763 >>> @@ -48,10 +49,37 @@ properties: >>> $ref: /schemas/types.yaml#/definitions/uint32 >>> enum: [0, 1, 2] >>> + gmt,internal-clock: >>> + description: Use the Internal clock instead of externally attached one >>> + via the CLK pin. >>> + type: boolean >>> + >>> required: >>> - compatible >>> - reg >>> - - clocks >>> + >>> +allOf: >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + enum: >>> + - gmt,g762 >>> + - gmt,g763 >>> + then: >>> + properties: >>> + gmt,internal-clock: false >>> + >>> + required: >>> + - clocks >> >> Is the new property even necessary ? The absence of an external clock on G761 >> could be used to imply that the internal clock is used. >> > > Well with how the driver works, if a property is not defined, then the > value is not set and the one set by the bootloader or from device reset > is keept. > > This conflicts with the logic of no clock = internal. > Not sure I understand the problem. It would be a simple change in the driver to add "if the chip is G761 and the clock is not provided in DT, use the internal clock". Guenter
On 5/25/24 04:12, Christian Marangi wrote: >>> Well with how the driver works, if a property is not defined, then the >>> value is not set and the one set by the bootloader or from device reset >>> is keept. >>> >>> This conflicts with the logic of no clock = internal. >>> >> >> Not sure I understand the problem. It would be a simple change in the driver >> to add "if the chip is G761 and the clock is not provided in DT, use the >> internal clock". >> > > Yes sure code wise is pretty easy, my concern is more of losing this > info in DT. But anyway ok will drop in V2. Thanks a lot for the review! > Not sure I understand. This is added support for a chip which is not currently supported. There should not be any information to lose. What am I missing ? Guenter
diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml index bfefe49f54bf..d6e80392d045 100644 --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml @@ -4,13 +4,13 @@ $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: GMT G762/G763 PWM Fan controller +title: GMT G761/G762/G763 PWM Fan controller maintainers: - Christian Marangi <ansuelsmth@gmail.com> description: | - GMT G762/G763 PWM Fan controller. + GMT G761/G762/G763 PWM Fan controller. If an optional property is not set in DT, then current value is kept unmodified (e.g. bootloader installed value). @@ -22,6 +22,7 @@ description: | properties: compatible: enum: + - gmt,g761 - gmt,g762 - gmt,g763 @@ -48,10 +49,37 @@ properties: $ref: /schemas/types.yaml#/definitions/uint32 enum: [0, 1, 2] + gmt,internal-clock: + description: Use the Internal clock instead of externally attached one + via the CLK pin. + type: boolean + required: - compatible - reg - - clocks + +allOf: + - if: + properties: + compatible: + contains: + enum: + - gmt,g762 + - gmt,g763 + then: + properties: + gmt,internal-clock: false + + required: + - clocks + + - if: + required: + - gmt,internal-clock + + then: + properties: + clocks: false additionalProperties: false @@ -80,4 +108,13 @@ examples: fan_startv = <1>; pwm_polarity = <0>; }; + + g761@1e { + compatible = "gmt,g761"; + reg = <0x1e>; + gmt,internal-clock; + fan_gear_mode = <0>; + fan_startv = <1>; + pwm_polarity = <0>; + }; };
Add support for g761 PWM Fan controller. This is an exact copy of g763 with the difference that it does also support an internal clock oscillators. Add required logic to support this additional feature with the property gmt,internal-clock and reject invalid schema that define both internal-clock property and external clocks. Signed-off-by: Christian Marangi <ansuelsmth@gmail.com> --- .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++-- 1 file changed, 40 insertions(+), 3 deletions(-)