From: Samuel Holland <samuel@sholland.org> To: Rob Herring <robh@kernel.org> Cc: Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles Date: Fri, 3 Sep 2021 10:36:48 -0500 [thread overview] Message-ID: <53d6d018-93bf-9bfc-e296-a232105306de@sholland.org> (raw) In-Reply-To: <YTDtelCx5If3J5cM@robh.at.kernel.org> 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"? 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 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? >> + - const: bus >> + description: AHB/APB bus clock for register access >> + - const: ext-osc32k >> + description: External 32768 Hz oscillator input >> + - const: hosc >> + description: 24 MHz oscillator input >> + - const: pll-32k >> + description: 32 kHz clock divided from a PLL >> >> clock-output-names: >> minItems: 1 >> @@ -85,6 +104,9 @@ allOf: >> enum: >> - allwinner,sun8i-h3-rtc >> - allwinner,sun50i-h5-rtc >> + - allwinner,sun50i-h6-rtc >> + - allwinner,sun50i-h616-rtc >> + - allwinner,sun50i-r329-rtc >> >> then: >> properties: >> @@ -96,13 +118,35 @@ allOf: >> properties: >> compatible: >> contains: >> - const: allwinner,sun50i-h6-rtc >> + enum: >> + - allwinner,sun50i-h616-rtc >> + - allwinner,sun50i-r329-rtc >> >> then: >> + clocks: >> + minItems: 3 # bus, hosc, and (pll-32k [H616] or ahb [R329]) >> + >> + clock-names: >> + minItems: 3 >> + >> + required: >> + - clock-names >> + >> + else: >> + required: >> + - clock-output-names >> + >> + - if: >> + properties: clock-names >> + >> + then: >> + required: >> + - clocks # hosc is required >> + >> + else: >> properties: >> - clock-output-names: >> - minItems: 3 >> - maxItems: 3 >> + clocks: >> + maxItems: 1 # only ext-osc32k is allowed >> >> - if: >> properties: >> @@ -127,7 +171,6 @@ required: >> - compatible >> - reg >> - interrupts >> - - clock-output-names >> >> additionalProperties: false >> >> diff --git a/include/dt-bindings/clock/sun50i-rtc.h b/include/dt-bindings/clock/sun50i-rtc.h >> new file mode 100644 >> index 000000000000..d45e3ff4e105 >> --- /dev/null >> +++ b/include/dt-bindings/clock/sun50i-rtc.h >> @@ -0,0 +1,12 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ > > Dual license please. Will do for v2. Regards, Samuel >> + >> +#ifndef _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ >> +#define _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ >> + >> +#define CLK_OSC32K 0 >> +#define CLK_OSC32K_FANOUT 1 >> +#define CLK_IOSC 2 >> + >> +#define CLK_RTC_SPI 8 >> + >> +#endif /* _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ */ >> -- >> 2.31.1 >> >>
WARNING: multiple messages have this Message-ID (diff)
From: Samuel Holland <samuel@sholland.org> To: Rob Herring <robh@kernel.org> Cc: Maxime Ripard <mripard@kernel.org>, Chen-Yu Tsai <wens@csie.org>, Jernej Skrabec <jernej.skrabec@gmail.com>, Michael Turquette <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles Date: Fri, 3 Sep 2021 10:36:48 -0500 [thread overview] Message-ID: <53d6d018-93bf-9bfc-e296-a232105306de@sholland.org> (raw) In-Reply-To: <YTDtelCx5If3J5cM@robh.at.kernel.org> 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"? 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 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? >> + - const: bus >> + description: AHB/APB bus clock for register access >> + - const: ext-osc32k >> + description: External 32768 Hz oscillator input >> + - const: hosc >> + description: 24 MHz oscillator input >> + - const: pll-32k >> + description: 32 kHz clock divided from a PLL >> >> clock-output-names: >> minItems: 1 >> @@ -85,6 +104,9 @@ allOf: >> enum: >> - allwinner,sun8i-h3-rtc >> - allwinner,sun50i-h5-rtc >> + - allwinner,sun50i-h6-rtc >> + - allwinner,sun50i-h616-rtc >> + - allwinner,sun50i-r329-rtc >> >> then: >> properties: >> @@ -96,13 +118,35 @@ allOf: >> properties: >> compatible: >> contains: >> - const: allwinner,sun50i-h6-rtc >> + enum: >> + - allwinner,sun50i-h616-rtc >> + - allwinner,sun50i-r329-rtc >> >> then: >> + clocks: >> + minItems: 3 # bus, hosc, and (pll-32k [H616] or ahb [R329]) >> + >> + clock-names: >> + minItems: 3 >> + >> + required: >> + - clock-names >> + >> + else: >> + required: >> + - clock-output-names >> + >> + - if: >> + properties: clock-names >> + >> + then: >> + required: >> + - clocks # hosc is required >> + >> + else: >> properties: >> - clock-output-names: >> - minItems: 3 >> - maxItems: 3 >> + clocks: >> + maxItems: 1 # only ext-osc32k is allowed >> >> - if: >> properties: >> @@ -127,7 +171,6 @@ required: >> - compatible >> - reg >> - interrupts >> - - clock-output-names >> >> additionalProperties: false >> >> diff --git a/include/dt-bindings/clock/sun50i-rtc.h b/include/dt-bindings/clock/sun50i-rtc.h >> new file mode 100644 >> index 000000000000..d45e3ff4e105 >> --- /dev/null >> +++ b/include/dt-bindings/clock/sun50i-rtc.h >> @@ -0,0 +1,12 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ > > Dual license please. Will do for v2. Regards, Samuel >> + >> +#ifndef _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ >> +#define _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ >> + >> +#define CLK_OSC32K 0 >> +#define CLK_OSC32K_FANOUT 1 >> +#define CLK_IOSC 2 >> + >> +#define CLK_RTC_SPI 8 >> + >> +#endif /* _DT_BINDINGS_CLK_SUN50I_RTC_CCU_H_ */ >> -- >> 2.31.1 >> >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-09-03 15:36 UTC|newest] Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-09-01 5:39 [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 1/7] dt-bindings: rtc: sun6i: Add H616 and R329 compatibles Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 12:06 ` Rob Herring 2021-09-01 12:06 ` Rob Herring 2021-09-02 15:27 ` Rob Herring 2021-09-02 15:27 ` Rob Herring 2021-09-03 15:36 ` Samuel Holland [this message] 2021-09-03 15:36 ` Samuel Holland 2021-09-07 14:44 ` Rob Herring 2021-09-07 14:44 ` Rob Herring 2021-09-07 14:44 ` Rob Herring 2021-09-08 2:26 ` Samuel Holland 2021-09-08 2:26 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 2/7] clk: sunxi-ng: div: Add macro using CLK_HW_INIT_FW_NAME Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 3/7] clk: sunxi-ng: mux: Add macro using CLK_HW_INIT_PARENTS_DATA Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 4/7] clk: sunxi-ng: mux: Allow muxes to have keys Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 5/7] clk: sunxi-ng: Add support for the sun50i RTC clocks Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 6/7] [DO NOT MERGE] clk: sunxi-ng: Add support for H6 Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-03 14:51 ` Maxime Ripard 2021-09-03 14:51 ` Maxime Ripard 2021-09-03 15:07 ` Samuel Holland 2021-09-03 15:07 ` Samuel Holland 2021-09-01 5:39 ` [RFC PATCH 7/7] [DO NOT MERGE] clk: sunxi-ng: Add support for T5 Samuel Holland 2021-09-01 5:39 ` Samuel Holland 2021-09-03 14:50 ` [RFC PATCH 0/7] clk: sunxi-ng: Add a RTC CCU driver Maxime Ripard 2021-09-03 14:50 ` Maxime Ripard 2021-09-03 15:21 ` Samuel Holland 2021-09-03 15:21 ` Samuel Holland 2021-09-09 8:45 ` Maxime Ripard 2021-09-09 8:45 ` Maxime Ripard 2021-09-28 7:46 ` Samuel Holland 2021-09-28 7:46 ` Samuel Holland 2021-09-28 9:06 ` Maxime Ripard 2021-09-28 9:06 ` Maxime Ripard 2021-09-29 3:54 ` Samuel Holland 2021-09-29 3:54 ` Samuel Holland 2021-10-25 15:54 ` Maxime Ripard 2021-10-25 15:54 ` Maxime Ripard
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=53d6d018-93bf-9bfc-e296-a232105306de@sholland.org \ --to=samuel@sholland.org \ --cc=devicetree@vger.kernel.org \ --cc=jernej.skrabec@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-clk@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-sunxi@lists.linux.dev \ --cc=mripard@kernel.org \ --cc=mturquette@baylibre.com \ --cc=robh@kernel.org \ --cc=sboyd@kernel.org \ --cc=wens@csie.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.