lkml.org 
[lkml]   [2021]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles
    On Fri, Sep 3, 2021 at 10:36 AM Samuel Holland <samuel@sholland.org> 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@sholland.org>
    > >> ---
    > >> .../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

    \
     
     \ /
      Last update: 2021-09-07 16:45    [W:4.409 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site