linux-kernel - Re: [PATCH v4 1/2] dt-bindings: clock: fixed-clock: Add nvmem support

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] [day] [month] [year] [list]

Message-ID: <1e87be02-4bea-0b84-a0ee-457c56d4cfd9@topic.nl>
Date:   Thu, 22 Jun 2023 13:41:11 +0200
From:   Mike Looijmans <mike.looijmans@...ic.nl>
To:     Rob Herring <robh@...nel.org>
CC:     devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
        Conor Dooley <conor+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/2] dt-bindings: clock: fixed-clock: Add nvmem support

On 08-06-2023 00:41, Rob Herring wrote:
> On Mon, Jun 05, 2023 at 04:01:48PM +0200, Mike Looijmans wrote:
>> Add bindings for a fixed-rate clock that retrieves its rate from an
>> NVMEM provider. This allows to store clock settings in EEPROM or EFUSE
>> or similar device.
>>
>> Component shortages lead to boards being shipped with different clock
>> crystals, based on what was available at the time. The clock frequency
>> was written to EEPROM at production time. Systems can adapt to a wide
>> range of input frequencies using the clock framework, but this required
>> us to patch the devicetree at runtime or use some custom driver. This
>> provides a more generic solution.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>>
>> ---
>>
>> Changes in v4:
>> Use proper "if" block and add example dts
>>
>> Changes in v3:
>> Modify fixed-clock instead of introducing nvmem-clock
>>
>> Changes in v2:
>> Changed "fixed-clock" into "nvmem-clock" in dts example
>> Add minItems:1 to nvmem-cell-names
>>
>>   .../bindings/clock/fixed-clock.yaml           | 42 ++++++++++++++++++-
>>   1 file changed, 41 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
>> index b0a4fb8256e2..71a5791da438 100644
>> --- a/Documentation/devicetree/bindings/clock/fixed-clock.yaml
>> +++ b/Documentation/devicetree/bindings/clock/fixed-clock.yaml
>> @@ -12,7 +12,9 @@ maintainers:
>>   
>>   properties:
>>     compatible:
>> -    const: fixed-clock
>> +    enum:
>> +      - fixed-clock
>> +      - fixed-clock-nvmem
>>   
>>     "#clock-cells":
>>       const: 0
>> @@ -26,11 +28,41 @@ properties:
>>     clock-output-names:
>>       maxItems: 1
>>   
>> +  nvmem-cells:
>> +    minItems: 1
>> +    maxItems: 2
>> +    description:
>> +      Reads clock-frequency and/or clock-accuracy from an NVMEM provider in
>> +      binary native integer format. The size of the NVMEM cell can be 1, 2, 4
>> +      or 8 bytes. If the contents of the nvmem are all zeroes or all 0xff, the
>> +      value reverts to the one given in the property.
>> +
>> +  nvmem-cell-names:
>> +    minItems: 1
>> +    items:
>> +      - const: clock-frequency
>> +      - const: clock-accuracy
>> +
>>   required:
>>     - compatible
>>     - "#clock-cells"
>>     - clock-frequency
>>   
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: fixed-clock-nvmem
>> +    then:
>> +      required:
>> +        - nvmem-cells
>> +        - nvmem-cell-names
>> +    else:
>> +      properties:
>> +        nvmem-cells: false
>> +        nvmem-cell-names: false
>> +
>>   additionalProperties: false
>>   
>>   examples:
>> @@ -41,4 +73,12 @@ examples:
>>         clock-frequency = <1000000000>;
>>         clock-accuracy = <100>;
>>       };
>> +  - |
>> +    clock {
>> +      compatible = "fixed-clock-nvmem";
>> +      #clock-cells = <0>;
>> +      clock-frequency = <48000000>;
> 
> If the freq comes from nvmem, why is this needed?

Not needed, but it's a nice default in case the NVMEM is still 'blank'. With a 
separate schema (see next comment), something like "clock-frequency-default" 
would be more appropriate.

> I think this should probably be a separate schema file as the only
> other thing shared is #clock-cells.

Yep, I implemented it this way as suggested by Krysztof.

I don't care much either way, I'll send a v5 with a separate yaml file, if 
Krysztof agrees.


-- 
Mike Looijmans
System Expert

TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands

T: +31 (0) 499 33 69 69
E: mike.looijmans@...ic.nl
W: www.topic.nl