On Fri, Sep 3, 2021 at 10:36 AM Samuel Holland <samuel@xxxxxxxxxxxx> wrote: > > On 9/2/21 10:27 AM, Rob Herring wrote: > > On Wed, Sep 01, 2021 at 12:39:45AM -0500, Samuel Holland wrote: > >> For these new SoCs, start requiring a complete list of input clocks. > >> > >> For H616, this means bus, hosc, and pll-32k. For R329, this means ahb, > >> bus, and hosc; and optionally ext-osc32k. > >> > >> I'm not sure how to best represent this in the binding... > >> > >> Signed-off-by: Samuel Holland <samuel@xxxxxxxxxxxx> > >> --- > >> .../bindings/rtc/allwinner,sun6i-a31-rtc.yaml | 55 +++++++++++++++++-- > >> include/dt-bindings/clock/sun50i-rtc.h | 12 ++++ > >> 2 files changed, 61 insertions(+), 6 deletions(-) > >> create mode 100644 include/dt-bindings/clock/sun50i-rtc.h > >> > >> diff --git a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml > >> index beeb90e55727..3e085db1294f 100644 > >> --- a/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml > >> +++ b/Documentation/devicetree/bindings/rtc/allwinner,sun6i-a31-rtc.yaml > >> @@ -26,6 +26,8 @@ properties: > >> - const: allwinner,sun50i-a64-rtc > >> - const: allwinner,sun8i-h3-rtc > >> - const: allwinner,sun50i-h6-rtc > >> + - const: allwinner,sun50i-h616-rtc > >> + - const: allwinner,sun50i-r329-rtc > > > > Can you please make all the single entry cases a single 'enum'. > > > >> > >> reg: > >> maxItems: 1 > >> @@ -37,7 +39,24 @@ properties: > >> - description: RTC Alarm 1 > >> > >> clocks: > >> - maxItems: 1 > >> + minItems: 1 > >> + maxItems: 4 > >> + > >> + clock-names: > >> + minItems: 1 > >> + maxItems: 4 > >> + items: > >> + - anyOf: > > > > This says the first entry is any of these. What about the rest of them? > > Oh, right. The list below is the list of all possible clocks. > > >> + - const: ahb > >> + description: AHB parent for SPI bus clock > > > > The description should go in 'clocks'. > > Will do for v2. > > > The order should be defined as well with the first clock being the > > one that existed previously. > > The only way I know how to further refine the list is with > minItems/maxItems. My problem is that 1) some clocks are only valid for > certain SoCs, and 2) some clocks are optional, depending on how the > board is wired. So there is no single order where the "valid" > combinations are prefixes of the "possible" combinations of clocks. > > Or in other words, how can I say "clocks #1 and #2 from this list are > required, and #4 is optional, but #3 is not allowed"? This says you have up to 4 clocks, but only defines the 1st 2: maxItems: 4 items: - description: 1st clock - description: 2nd clock But I think you will be better off with just defining the range (minItems/maxItems) at the top level and then use if/then schemas. > > Some concrete examples, with the always-required clocks moved to the > beginning: > > H6: > - bus: required > - hosc: required > - ahb: not allowed > - ext-osc32k: optional > - pll-32k: not allowed Is this really 2 different 32k clock inputs to the h/w block? Doesn't seem like it given both are never valid. > > H616: > - bus: required > - hosc: required > - ahb: not allowed > - ext-osc32k: not allowed > - pll-32k: required > > R329: > - bus: required > - hosc: required > - ahb: required > - ext-osc32k: optional > - pll-32k: not allowed > > Should I just move the entire clocks/clock-items properties to if/then > blocks based on the compatible? Probably so. Rob
