Messages in this thread | | | Date | Thu, 11 Apr 2024 08:14:01 +0200 | Subject | Re: [PATCH 02/11] dt-bindings: clock: mobileye,eyeq5-clk: add EyeQ6L and EyeQ6H | From | Krzysztof Kozlowski <> |
| |
On 10/04/2024 19:12, Théo Lebrun wrote: > Add bindings describing EyeQ6L and EyeQ6H clock controllers. > Add constants to index clocks. > > Bindings are conditional for two reasons: > - Some compatibles expose a single clock; they do not take clock cells. > - All compatibles take a PLLs resource, not all take others (aimed at > divider clocks). Those that only take a resource for PLLs do not > require named resources. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../bindings/clock/mobileye,eyeq5-clk.yaml | 103 ++++++++++++++++++--- > MAINTAINERS | 2 + > include/dt-bindings/clock/mobileye,eyeq5-clk.h | 21 +++++ > 3 files changed, 113 insertions(+), 13 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml > index 2d4f2cde1e58..a1651fcce258 100644 > --- a/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml > +++ b/Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml > @@ -4,12 +4,13 @@ > $id: http://devicetree.org/schemas/clock/mobileye,eyeq5-clk.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Mobileye EyeQ5 clock controller > +title: Mobileye EyeQ clock controller > > description: > - The EyeQ5 clock controller handles 10 read-only PLLs derived from the main > - crystal clock. It also exposes one divider clock, a child of one of the PLLs. > - Its registers live in a shared region called OLB. > + EyeQ clock controllers expose read-only PLLs derived from main crystal clock. > + Some also expose divider clocks, children of specific PLLs. Its registers > + live in a shared region called OLB. EyeQ5 and EyeQ6L have a single OLB > + instance while EyeQ6H have seven, leading to seven clock controllers. > > maintainers: > - Grégory Clement <gregory.clement@bootlin.com> > @@ -18,18 +19,23 @@ maintainers: > > properties: > compatible: > - const: mobileye,eyeq5-clk > + enum: > + - mobileye,eyeq5-clk > + - mobileye,eyeq6l-clk > + - mobileye,eyeq6h-central-clk > + - mobileye,eyeq6h-west-clk > + - mobileye,eyeq6h-east-clk > + - mobileye,eyeq6h-south-clk > + - mobileye,eyeq6h-ddr0-clk > + - mobileye,eyeq6h-ddr1-clk > + - mobileye,eyeq6h-acc-clk > > - reg: > - maxItems: 2 > + reg: true
No, you must leave widest constraints here.
> > - reg-names: > - items: > - - const: plls > - - const: ospi > + reg-names: true
No, you must leave widest constraints here.
> > "#clock-cells": > - const: 1 > + enum: [0, 1]
Looks like you squash here quite different devices...
> > clocks: > maxItems: 1 > @@ -43,9 +49,80 @@ properties: > required: > - compatible > - reg > - - reg-names > - "#clock-cells" > - clocks > - clock-names > > +allOf: > + # "mobileye,eyeq5-clk" provides: > + # - PLLs and, > + # - One divider clock related to ospi. > + - if: > + properties: > + compatible: > + const: mobileye,eyeq5-clk > + then: > + properties: > + reg: > + minItems: 2 > + maxItems: 2 > + reg-names: > + minItems: 2 > + maxItems: 2
So any name is now valid? Like "yellow-pony"?
> + items: > + enum: [ plls, ospi ] > + required: > + - reg-names > + > + # "mobileye,eyeq6h-south-clk" provides: > + # - PLLs and, > + # - Four divider clocks related to emmc, ospi and tsu. > + - if: > + properties: > + compatible: > + const: mobileye,eyeq6h-south-clk > + then: > + properties: > + reg: > + minItems: 4 > + maxItems: 4 > + reg-names: > + minItems: 4 > + maxItems: 4 > + items: > + enum: [ plls, emmc, ospi, tsu ] > + required: > + - reg-names > + > + # Other compatibles only provide PLLs. Do not ask for named resources. > + - if: > + not: > + required: > + - reg-names > + then: > + properties: > + reg: > + minItems: 1 > + maxItems: 1
No, just restrict properly reg per variant.
> + reg-names: false
That's redundant. Drop entire if.
> + > + # Some compatibles provide a single clock; they do not take a clock cell. > + - if: > + properties: > + compatible: > + enum: > + - mobileye,eyeq6h-central-clk > + - mobileye,eyeq6h-west-clk > + - mobileye,eyeq6h-east-clk > + - mobileye,eyeq6h-ddr0-clk > + - mobileye,eyeq6h-ddr1-clk > + then: > + properties: > + "#clock-cells": > + const: 0
Wait, so you define device-per-clock? That's a terrible idea. We also discussed it many times and it was rejected many times.
You have one device, not 5.
Best regards, Krzysztof
| |