| Message ID | 20230526143807.10164-1-mike.looijmans@topic.nl |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: clock: Add nvmem-clock | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
Hey Mike, I have no comments about whether this is a good idea or not, I'll leave that Stephen, Krzysztof etc.. On Fri, May 26, 2023 at 04:38:06PM +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@topic.nl> > > --- > > Changes in v2: > Changed "fixed-clock" into "nvmem-clock" in dts example > Add minItems:1 to nvmem-cell-names > > .../bindings/clock/nvmem-clock.yaml | 63 +++++++++++++++++++ > 1 file changed, 63 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/nvmem-clock.yaml > > diff --git a/Documentation/devicetree/bindings/clock/nvmem-clock.yaml b/Documentation/devicetree/bindings/clock/nvmem-clock.yaml > new file mode 100644 > index 000000000000..af96a5e9372d > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/nvmem-clock.yaml > @@ -0,0 +1,63 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/nvmem-clock.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Simple fixed-rate clock source from NVMEM > + > +maintainers: > + - Mike Looijmans <mike.looijmans@topic.nl> > + > +description: > + Provides a clock rate from NVMEM. Typical usage is that the factory places a > + crystal on the board and writes the rate into an EEPROM or EFUSE. If some math > + is required, one can add a fixed-factor clock using this clock as input. > + > +properties: > + compatible: > + const: nvmem-clock > + > + "#clock-cells": > + const: 0 > + > + nvmem-cells: > + minItems: 1 > + maxItems: 2 > + description: > + Reads clock-frequency and optionally 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. How is that size communicated to the driver? Some nvmem magic just takes care of it? > + > + nvmem-cell-names: > + minItems: 1 > + items: > + - const: clock-frequency > + - const: clock-accuracy > + > + clock-accuracy: I think the common pattern is to add a unit suffix for things like this, which I guess would be -ppb? Cheers, Conor. > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + accuracy of clock in ppb (parts per billion). Alternative for providing > + this through nvmem, the nvmem provided value takes precedence. > + > + clock-output-names: > + maxItems: 1 > + > +required: > + - compatible > + - "#clock-cells" > + - nvmem-cells > + - nvmem-cell-names > + > +additionalProperties: false > + > +examples: > + - | > + xtal { > + compatible = "nvmem-clock"; > + #clock-cells = <0>; > + nvmem-cells = <&efuse_xtal_freq>; > + nvmem-cell-names = "clock-frequency"; > + }; > +... > -- > 2.17.1 > > > Met vriendelijke groet / kind regards, > > 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@topicproducts.com > W: www.topic.nl > > Please consider the environment before printing this e-mail
On 26/05/2023 16:38, 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. This does not look like real hardware. I mean, the clock does not fetch its rate from nvmem, right? It's the Linux which does it, so basically you described here driver, not hardware. Extend existing fixed-clock bindings to allow reading frequency via nvmem cells. Best regards, Krzysztof
Resending as my previous email probably got lost. If you got it twice, apologies. On 26/05/2023 16:38, 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. This does not look like real hardware. I mean, the clock does not fetch its rate from nvmem, right? It's the Linux which does it, so basically you described here driver, not hardware. Extend existing fixed-clock bindings to allow reading frequency via nvmem cells. Best regards, Krzysztof
On 31-05-2023 21:27, Krzysztof Kozlowski wrote: > On 26/05/2023 16:38, 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. > This does not look like real hardware. I mean, the clock does not fetch > its rate from nvmem, right? It's the Linux which does it, so basically > you described here driver, not hardware. Right, this just reads a setting from an NVMEM provider. > Extend existing fixed-clock bindings to allow reading frequency via > nvmem cells. I just tried and implemented this, but it does not work. The reason is that the fixed-clock implementation returns "void" in its of_fixed_clk_setup() init function. The nvmem provider returns EPROBE_DEFER because it isn't ready at this early stage, and this error will not be propagated up because of the "void" signature. Thus, it's never retried and the clock just disappears. > Best regards, > Krzysztof >
On 05/06/2023 12:01, Mike Looijmans wrote: > On 31-05-2023 21:27, Krzysztof Kozlowski wrote: >> On 26/05/2023 16:38, 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. >> This does not look like real hardware. I mean, the clock does not fetch >> its rate from nvmem, right? It's the Linux which does it, so basically >> you described here driver, not hardware. > Right, this just reads a setting from an NVMEM provider. >> Extend existing fixed-clock bindings to allow reading frequency via >> nvmem cells. > > I just tried and implemented this, but it does not work. The reason is > that the fixed-clock implementation returns "void" in its > of_fixed_clk_setup() init function. The nvmem provider returns > EPROBE_DEFER because it isn't ready at this early stage, and this error > will not be propagated up because of the "void" signature. Thus, it's > never retried and the clock just disappears. Linux driver problems are not a reason to add bindings for virtual hardware... Best regards, Krzysztof
On 05-06-2023 12:13, Krzysztof Kozlowski wrote: > On 05/06/2023 12:01, Mike Looijmans wrote: >> On 31-05-2023 21:27, Krzysztof Kozlowski wrote: >>> On 26/05/2023 16:38, 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. >>> This does not look like real hardware. I mean, the clock does not fetch >>> its rate from nvmem, right? It's the Linux which does it, so basically >>> you described here driver, not hardware. >> Right, this just reads a setting from an NVMEM provider. >>> Extend existing fixed-clock bindings to allow reading frequency via >>> nvmem cells. >> I just tried and implemented this, but it does not work. The reason is >> that the fixed-clock implementation returns "void" in its >> of_fixed_clk_setup() init function. The nvmem provider returns >> EPROBE_DEFER because it isn't ready at this early stage, and this error >> will not be propagated up because of the "void" signature. Thus, it's >> never retried and the clock just disappears. > Linux driver problems are not a reason to add bindings for virtual > hardware... > Okay, so what are you proposing? The implementation needs a compatible string other than "fixed-clock", because of the way the fixed-clock driver is being loaded (it can never be deferred). So I should add another compatible string to fixed-clock.yaml? That'd make the syntax in fixed-clock.yaml rather awkward, since some properties would only apply to some compatible strings?
diff --git a/Documentation/devicetree/bindings/clock/nvmem-clock.yaml b/Documentation/devicetree/bindings/clock/nvmem-clock.yaml new file mode 100644 index 000000000000..af96a5e9372d --- /dev/null +++ b/Documentation/devicetree/bindings/clock/nvmem-clock.yaml @@ -0,0 +1,63 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/nvmem-clock.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Simple fixed-rate clock source from NVMEM + +maintainers: + - Mike Looijmans <mike.looijmans@topic.nl> + +description: + Provides a clock rate from NVMEM. Typical usage is that the factory places a + crystal on the board and writes the rate into an EEPROM or EFUSE. If some math + is required, one can add a fixed-factor clock using this clock as input. + +properties: + compatible: + const: nvmem-clock + + "#clock-cells": + const: 0 + + nvmem-cells: + minItems: 1 + maxItems: 2 + description: + Reads clock-frequency and optionally 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. + + nvmem-cell-names: + minItems: 1 + items: + - const: clock-frequency + - const: clock-accuracy + + clock-accuracy: + $ref: /schemas/types.yaml#/definitions/uint32 + description: + accuracy of clock in ppb (parts per billion). Alternative for providing + this through nvmem, the nvmem provided value takes precedence. + + clock-output-names: + maxItems: 1 + +required: + - compatible + - "#clock-cells" + - nvmem-cells + - nvmem-cell-names + +additionalProperties: false + +examples: + - | + xtal { + compatible = "nvmem-clock"; + #clock-cells = <0>; + nvmem-cells = <&efuse_xtal_freq>; + nvmem-cell-names = "clock-frequency"; + }; +...
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@topic.nl> --- Changes in v2: Changed "fixed-clock" into "nvmem-clock" in dts example Add minItems:1 to nvmem-cell-names .../bindings/clock/nvmem-clock.yaml | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/nvmem-clock.yaml