On 05/06/2023 16:00, Mike Looijmans wrote: > On 05-06-2023 15:38, Krzysztof Kozlowski wrote: >> On 05/06/2023 15:34, 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@xxxxxxxx> >>> >>> --- >>> >>> 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 | 25 ++++++++++++++++++- >>> 1 file changed, 24 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/fixed-clock.yaml b/Documentation/devicetree/bindings/clock/fixed-clock.yaml >>> index b0a4fb8256e2..23e4df96d3b0 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 >> Do you even need new compatible? Isn't this the same clock from the >> hardware point of view? > > I need a new compatible because a "fixed-clock" only loads at init time. > It registers using CLK_OF_DECLARE, which requires the clock to register > early. NVMEM providers are typical devices like I2C EEPROMs that won't > be available at that point, hence I needed to create a clock that > registers as a regular clock driver and can handle deferral and similar. Ah, ok. > > >> >>> "#clock-cells": >>> const: 0 >>> @@ -33,6 +35,27 @@ required: >>> >>> additionalProperties: false >>> >> Put it under allOf. Entire block should be before additionalProperties >> (just like in example-schema). >> >>> +if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: fixed-clock-nvmem >>> + >>> +then: >>> + properties: >>> + nvmem-cells: >>> + maxItems: 2 >> Anyway, I don't think you tested it. Provide a DTS user of this. I don't >> think it works and such user would point to mistakes. >> >> Properties should be defined in top-level properties:, not in >> allOf:if:then. In allOf:if:then you only narrow them. > > Ah, got it. Added an example dts block to the document, this revealed > the issues, indeed didn't test the bindings. > > Fixed it into an "allOf" to properly narrow the properties. The real DTS user would be also welcomed. Best regards, Krzysztof