linux-kernel - Re: [PATCH 2/3] dt-bindings: hwmon: g76x: Add support for g761

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Sat, 25 May 2024 12:50:31 +0200
From: Christian Marangi <ansuelsmth@...il.com>
To: Guenter Roeck <linux@...ck-us.net>
Cc: Jean Delvare <jdelvare@...e.com>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-hwmon@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] dt-bindings: hwmon: g76x: Add support for g761

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@...il.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@...il.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>;
> > +        };
> >       };
> 

-- 
	Ansuel