| Message ID | 20231214105243.3707730-3-tudor.ambarus@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | GS101 Oriole: CMU_PERIC0 support and USI updates | expand |
On Thu, Dec 14, 2023 at 4:52 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > > Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) > clock management unit. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > .../bindings/clock/google,gs101-clock.yaml | 25 +++++- > include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ > 2 files changed, 109 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > index 3eebc03a309b..ba54c13c55bc 100644 > --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > @@ -30,14 +30,15 @@ properties: > - google,gs101-cmu-top > - google,gs101-cmu-apm > - google,gs101-cmu-misc > + - google,gs101-cmu-peric0 > > clocks: > minItems: 1 > - maxItems: 2 > + maxItems: 3 > > clock-names: > minItems: 1 > - maxItems: 2 > + maxItems: 3 > > "#clock-cells": > const: 1 > @@ -88,6 +89,26 @@ allOf: > - const: dout_cmu_misc_bus > - const: dout_cmu_misc_sss > > + - if: > + properties: > + compatible: > + contains: > + const: google,gs101-cmu-peric0 > + > + then: > + properties: > + clocks: > + items: > + - description: External reference clock (24.576 MHz) > + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) > + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) > + > + clock-names: > + items: > + - const: oscclk > + - const: dout_cmu_peric0_bus > + - const: dout_cmu_peric0_ip > + > additionalProperties: false > > examples: > diff --git a/include/dt-bindings/clock/google,gs101.h b/include/dt-bindings/clock/google,gs101.h > index 21adec22387c..7d7a896416a7 100644 > --- a/include/dt-bindings/clock/google,gs101.h > +++ b/include/dt-bindings/clock/google,gs101.h > @@ -389,4 +389,90 @@ > #define CLK_GOUT_MISC_WDT_CLUSTER1_PCLK 73 > #define CLK_GOUT_MISC_XIU_D_MISC_ACLK 74 > > +/* CMU_PERIC0 */ This comments looks off here. Other than than, LGTM: Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org> > +/* CMU_PERIC0 MUX */ > +#define CLK_MOUT_PERIC0_BUS_USER 1 > +#define CLK_MOUT_PERIC0_I3C_USER 2 > +#define CLK_MOUT_PERIC0_USI0_UART_USER 3 > +#define CLK_MOUT_PERIC0_USI14_USI_USER 4 > +#define CLK_MOUT_PERIC0_USI1_USI_USER 5 > +#define CLK_MOUT_PERIC0_USI2_USI_USER 6 > +#define CLK_MOUT_PERIC0_USI3_USI_USER 7 > +#define CLK_MOUT_PERIC0_USI4_USI_USER 8 > +#define CLK_MOUT_PERIC0_USI5_USI_USER 9 > +#define CLK_MOUT_PERIC0_USI6_USI_USER 10 > +#define CLK_MOUT_PERIC0_USI7_USI_USER 11 > +#define CLK_MOUT_PERIC0_USI8_USI_USER 12 > + > +/* CMU_PERIC0 Dividers */ > +#define CLK_DOUT_PERIC0_I3C 13 > +#define CLK_DOUT_PERIC0_USI0_UART 14 > +#define CLK_DOUT_PERIC0_USI14_USI 15 > +#define CLK_DOUT_PERIC0_USI1_USI 16 > +#define CLK_DOUT_PERIC0_USI2_USI 17 > +#define CLK_DOUT_PERIC0_USI3_USI 18 > +#define CLK_DOUT_PERIC0_USI4_USI 19 > +#define CLK_DOUT_PERIC0_USI5_USI 20 > +#define CLK_DOUT_PERIC0_USI6_USI 21 > +#define CLK_DOUT_PERIC0_USI7_USI 22 > +#define CLK_DOUT_PERIC0_USI8_USI 23 > + > +/* CMU_PERIC0 Gates */ > +#define CLK_GOUT_PERIC0_IP 24 > +#define CLK_GOUT_PERIC0_PERIC0_CMU_PERIC0_PCLK 25 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_OSCCLK_CLK 26 > +#define CLK_GOUT_PERIC0_D_TZPC_PERIC0_PCLK 27 > +#define CLK_GOUT_PERIC0_GPC_PERIC0_PCLK 28 > +#define CLK_GOUT_PERIC0_GPIO_PERIC0_PCLK 29 > +#define CLK_GOUT_PERIC0_LHM_AXI_P_PERIC0_I_CLK 30 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0 31 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_1 32 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_10 33 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_11 34 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_12 35 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_13 36 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_14 37 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_15 38 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_2 39 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_3 40 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_4 41 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_5 42 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_6 43 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7 44 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_8 45 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_9 46 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_0 47 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_1 48 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_10 49 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_11 50 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_12 51 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_13 52 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_14 53 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_15 54 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_2 55 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_3 56 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_4 57 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_5 58 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_6 59 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_7 60 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_8 61 > +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_9 62 > +#define CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_0 63 > +#define CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_2 64 > +#define CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0 65 > +#define CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_2 66 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_BUSP_CLK 67 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_I3C_CLK 68 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI0_UART_CLK 69 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI14_USI_CLK 70 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI1_USI_CLK 71 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI2_USI_CLK 72 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI3_USI_CLK 73 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI4_USI_CLK 74 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI5_USI_CLK 75 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI6_USI_CLK 76 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI7_USI_CLK 77 > +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK 78 > +#define CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK 79 > + > #endif /* _DT_BINDINGS_CLOCK_GOOGLE_GS101_H */ > -- > 2.43.0.472.g3155946c3a-goog >
On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: > Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) > clock management unit. > > Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > --- > .../bindings/clock/google,gs101-clock.yaml | 25 +++++- > include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ > 2 files changed, 109 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > index 3eebc03a309b..ba54c13c55bc 100644 > --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > @@ -30,14 +30,15 @@ properties: > - google,gs101-cmu-top > - google,gs101-cmu-apm > - google,gs101-cmu-misc > + - google,gs101-cmu-peric0 > > clocks: > minItems: 1 > - maxItems: 2 > + maxItems: 3 > > clock-names: > minItems: 1 > - maxItems: 2 > + maxItems: 3 > > "#clock-cells": > const: 1 > @@ -88,6 +89,26 @@ allOf: > - const: dout_cmu_misc_bus > - const: dout_cmu_misc_sss > > + - if: > + properties: > + compatible: > + contains: > + const: google,gs101-cmu-peric0 > + > + then: > + properties: > + clocks: > + items: > + - description: External reference clock (24.576 MHz) > + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) > + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) > + > + clock-names: > + items: > + - const: oscclk > + - const: dout_cmu_peric0_bus > + - const: dout_cmu_peric0_ip 'bus' and 'ip' are sufficient because naming is local to the module. The same is true on 'dout_cmu_misc_bus'. As that has not made a release, please fix all of them. Rob
On 12/20/23 15:07, Rob Herring wrote: > On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: >> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) >> clock management unit. >> >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >> --- >> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- >> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ >> 2 files changed, 109 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >> index 3eebc03a309b..ba54c13c55bc 100644 >> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >> @@ -30,14 +30,15 @@ properties: >> - google,gs101-cmu-top >> - google,gs101-cmu-apm >> - google,gs101-cmu-misc >> + - google,gs101-cmu-peric0 >> >> clocks: >> minItems: 1 >> - maxItems: 2 >> + maxItems: 3 >> >> clock-names: >> minItems: 1 >> - maxItems: 2 >> + maxItems: 3 >> >> "#clock-cells": >> const: 1 >> @@ -88,6 +89,26 @@ allOf: >> - const: dout_cmu_misc_bus >> - const: dout_cmu_misc_sss >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: google,gs101-cmu-peric0 >> + >> + then: >> + properties: >> + clocks: >> + items: >> + - description: External reference clock (24.576 MHz) >> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) >> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) >> + >> + clock-names: >> + items: >> + - const: oscclk >> + - const: dout_cmu_peric0_bus >> + - const: dout_cmu_peric0_ip > > 'bus' and 'ip' are sufficient because naming is local to the module. The > same is true on 'dout_cmu_misc_bus'. As that has not made a release, > please fix all of them. > Ok, will fix them shortly. Thanks, Rob!
Hi Tudor, On Thu, 21 Dec 2023 at 07:20, Tudor Ambarus <tudor.ambarus@linaro.org> wrote: > > > > On 12/20/23 15:07, Rob Herring wrote: > > On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: > >> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) > >> clock management unit. > >> > >> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> > >> --- > >> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- > >> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ > >> 2 files changed, 109 insertions(+), 2 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > >> index 3eebc03a309b..ba54c13c55bc 100644 > >> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > >> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml > >> @@ -30,14 +30,15 @@ properties: > >> - google,gs101-cmu-top > >> - google,gs101-cmu-apm > >> - google,gs101-cmu-misc > >> + - google,gs101-cmu-peric0 > >> > >> clocks: > >> minItems: 1 > >> - maxItems: 2 > >> + maxItems: 3 > >> > >> clock-names: > >> minItems: 1 > >> - maxItems: 2 > >> + maxItems: 3 > >> > >> "#clock-cells": > >> const: 1 > >> @@ -88,6 +89,26 @@ allOf: > >> - const: dout_cmu_misc_bus > >> - const: dout_cmu_misc_sss > >> > >> + - if: > >> + properties: > >> + compatible: > >> + contains: > >> + const: google,gs101-cmu-peric0 > >> + > >> + then: > >> + properties: > >> + clocks: > >> + items: > >> + - description: External reference clock (24.576 MHz) > >> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) > >> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) > >> + > >> + clock-names: > >> + items: > >> + - const: oscclk > >> + - const: dout_cmu_peric0_bus > >> + - const: dout_cmu_peric0_ip > > > > 'bus' and 'ip' are sufficient because naming is local to the module. The > > same is true on 'dout_cmu_misc_bus'. As that has not made a release, > > please fix all of them. > > > > Ok, will fix them shortly. Thanks, Rob! With Robs review comments addressed feel free to add my: Reviewed-by: Peter Griffin <peter.griffin@linaro.org>
Hi, Rob, On 12/21/23 07:20, Tudor Ambarus wrote: > > > On 12/20/23 15:07, Rob Herring wrote: >> On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: >>> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) >>> clock management unit. >>> >>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >>> --- >>> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- >>> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ >>> 2 files changed, 109 insertions(+), 2 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> index 3eebc03a309b..ba54c13c55bc 100644 >>> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>> @@ -30,14 +30,15 @@ properties: >>> - google,gs101-cmu-top >>> - google,gs101-cmu-apm >>> - google,gs101-cmu-misc >>> + - google,gs101-cmu-peric0 >>> >>> clocks: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 3 >>> >>> clock-names: >>> minItems: 1 >>> - maxItems: 2 >>> + maxItems: 3 >>> >>> "#clock-cells": >>> const: 1 >>> @@ -88,6 +89,26 @@ allOf: >>> - const: dout_cmu_misc_bus >>> - const: dout_cmu_misc_sss >>> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: google,gs101-cmu-peric0 >>> + >>> + then: >>> + properties: >>> + clocks: >>> + items: >>> + - description: External reference clock (24.576 MHz) >>> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) >>> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) >>> + >>> + clock-names: >>> + items: >>> + - const: oscclk >>> + - const: dout_cmu_peric0_bus >>> + - const: dout_cmu_peric0_ip >> >> 'bus' and 'ip' are sufficient because naming is local to the module. The >> same is true on 'dout_cmu_misc_bus'. As that has not made a release, >> please fix all of them. >> > > Ok, will fix them shortly. Thanks, Rob! I tried your suggestion at https://lore.kernel.org/linux-arm-kernel/c6cc6e74-6c3d-439b-8dc1-bc50a88a3d8f@linaro.org/ and we noticed that we'd have to update the clock driver as well. These CMUs set the DT clock-name of the parent clock in the driver in struct samsung_cmu_info::clk_name[]. The driver then tries to enable the parent clock based on the clock-name in exynos_arm64_register_cmu(). In order to enable the parent clock of the CMU the following would be needed in the driver: diff --git a/drivers/clk/samsung/clk-gs101.c b/drivers/clk/samsung/clk-gs101.c index 68a27b78b00b..e91836ea3a98 100644 --- a/drivers/clk/samsung/clk-gs101.c +++ b/drivers/clk/samsung/clk-gs101.c @@ -2476,7 +2476,7 @@ static const struct samsung_cmu_info misc_cmu_info __initconst = { .nr_clk_ids = CLKS_NR_MISC, .clk_regs = misc_clk_regs, .nr_clk_regs = ARRAY_SIZE(misc_clk_regs), - .clk_name = "dout_cmu_misc_bus", + .clk_name = "bus", }; I think I lean towards keeping the name as it was because it's clearer what are the clock dependencies in the driver. "dout_cmu_misc_bus" is the clock name used when defining the clock: DIV(CLK_DOUT_CMU_MISC_BUS, "dout_cmu_misc_bus", "gout_cmu_misc_bus", CLK_CON_DIV_CLKCMU_MISC_BUS, 0, 4), The other exynos clock drivers are using too the driver's clock names for the clock-names device tree property. For consistency I'd keep it the same. If you have a stronger opinion than mine, please tell and I'll happily update the driver. Thanks, ta
On 27/12/2023 13:38, Tudor Ambarus wrote: > Hi, Rob, > > On 12/21/23 07:20, Tudor Ambarus wrote: >> >> >> On 12/20/23 15:07, Rob Herring wrote: >>> On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: >>>> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) >>>> clock management unit. >>>> >>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >>>> --- >>>> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- >>>> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ >>>> 2 files changed, 109 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>> index 3eebc03a309b..ba54c13c55bc 100644 >>>> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>> @@ -30,14 +30,15 @@ properties: >>>> - google,gs101-cmu-top >>>> - google,gs101-cmu-apm >>>> - google,gs101-cmu-misc >>>> + - google,gs101-cmu-peric0 >>>> >>>> clocks: >>>> minItems: 1 >>>> - maxItems: 2 >>>> + maxItems: 3 >>>> >>>> clock-names: >>>> minItems: 1 >>>> - maxItems: 2 >>>> + maxItems: 3 >>>> >>>> "#clock-cells": >>>> const: 1 >>>> @@ -88,6 +89,26 @@ allOf: >>>> - const: dout_cmu_misc_bus >>>> - const: dout_cmu_misc_sss >>>> >>>> + - if: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + const: google,gs101-cmu-peric0 >>>> + >>>> + then: >>>> + properties: >>>> + clocks: >>>> + items: >>>> + - description: External reference clock (24.576 MHz) >>>> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) >>>> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: oscclk >>>> + - const: dout_cmu_peric0_bus >>>> + - const: dout_cmu_peric0_ip >>> >>> 'bus' and 'ip' are sufficient because naming is local to the module. The >>> same is true on 'dout_cmu_misc_bus'. As that has not made a release, >>> please fix all of them. >>> >> >> Ok, will fix them shortly. Thanks, Rob! > > I tried your suggestion at > https://lore.kernel.org/linux-arm-kernel/c6cc6e74-6c3d-439b-8dc1-bc50a88a3d8f@linaro.org/ > > and we noticed that we'd have to update the clock driver as well. > These CMUs set the DT clock-name of the parent clock in the driver in > struct samsung_cmu_info::clk_name[]. The driver then tries to enable the > parent clock based on the clock-name in exynos_arm64_register_cmu(). > > In order to enable the parent clock of the CMU the following would be > needed in the driver: > > diff --git a/drivers/clk/samsung/clk-gs101.c > b/drivers/clk/samsung/clk-gs101.c > index 68a27b78b00b..e91836ea3a98 100644 > --- a/drivers/clk/samsung/clk-gs101.c > +++ b/drivers/clk/samsung/clk-gs101.c > @@ -2476,7 +2476,7 @@ static const struct samsung_cmu_info misc_cmu_info > __initconst = { > .nr_clk_ids = CLKS_NR_MISC, > .clk_regs = misc_clk_regs, > .nr_clk_regs = ARRAY_SIZE(misc_clk_regs), > - .clk_name = "dout_cmu_misc_bus", > + .clk_name = "bus", Yes, obviously, the names are used... The entire point was that a week ago Rob said: "As that has not made a release, please fix all of them." but if you keep waiting, like 8 days for this simple patch, his statement is not true anymore. The only point was to send a fix *the next day*, so I would apply it and send further. You kind of solved that problem by waiting entire week for a simple driver and DTS change. Best regards, Krzysztof
On 12/28/23 07:30, Krzysztof Kozlowski wrote: > On 27/12/2023 13:38, Tudor Ambarus wrote: >> Hi, Rob, >> >> On 12/21/23 07:20, Tudor Ambarus wrote: >>> >>> >>> On 12/20/23 15:07, Rob Herring wrote: >>>> On Thu, Dec 14, 2023 at 10:52:32AM +0000, Tudor Ambarus wrote: >>>>> Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) >>>>> clock management unit. >>>>> >>>>> Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> >>>>> --- >>>>> .../bindings/clock/google,gs101-clock.yaml | 25 +++++- >>>>> include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ >>>>> 2 files changed, 109 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>>> index 3eebc03a309b..ba54c13c55bc 100644 >>>>> --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>>> +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml >>>>> @@ -30,14 +30,15 @@ properties: >>>>> - google,gs101-cmu-top >>>>> - google,gs101-cmu-apm >>>>> - google,gs101-cmu-misc >>>>> + - google,gs101-cmu-peric0 >>>>> >>>>> clocks: >>>>> minItems: 1 >>>>> - maxItems: 2 >>>>> + maxItems: 3 >>>>> >>>>> clock-names: >>>>> minItems: 1 >>>>> - maxItems: 2 >>>>> + maxItems: 3 >>>>> >>>>> "#clock-cells": >>>>> const: 1 >>>>> @@ -88,6 +89,26 @@ allOf: >>>>> - const: dout_cmu_misc_bus >>>>> - const: dout_cmu_misc_sss >>>>> >>>>> + - if: >>>>> + properties: >>>>> + compatible: >>>>> + contains: >>>>> + const: google,gs101-cmu-peric0 >>>>> + >>>>> + then: >>>>> + properties: >>>>> + clocks: >>>>> + items: >>>>> + - description: External reference clock (24.576 MHz) >>>>> + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) >>>>> + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) >>>>> + >>>>> + clock-names: >>>>> + items: >>>>> + - const: oscclk >>>>> + - const: dout_cmu_peric0_bus >>>>> + - const: dout_cmu_peric0_ip >>>> >>>> 'bus' and 'ip' are sufficient because naming is local to the module. The >>>> same is true on 'dout_cmu_misc_bus'. As that has not made a release, >>>> please fix all of them. >>>> >>> >>> Ok, will fix them shortly. Thanks, Rob! >> >> I tried your suggestion at >> https://lore.kernel.org/linux-arm-kernel/c6cc6e74-6c3d-439b-8dc1-bc50a88a3d8f@linaro.org/ >> >> and we noticed that we'd have to update the clock driver as well. >> These CMUs set the DT clock-name of the parent clock in the driver in >> struct samsung_cmu_info::clk_name[]. The driver then tries to enable the >> parent clock based on the clock-name in exynos_arm64_register_cmu(). >> >> In order to enable the parent clock of the CMU the following would be >> needed in the driver: >> >> diff --git a/drivers/clk/samsung/clk-gs101.c >> b/drivers/clk/samsung/clk-gs101.c >> index 68a27b78b00b..e91836ea3a98 100644 >> --- a/drivers/clk/samsung/clk-gs101.c >> +++ b/drivers/clk/samsung/clk-gs101.c >> @@ -2476,7 +2476,7 @@ static const struct samsung_cmu_info misc_cmu_info >> __initconst = { >> .nr_clk_ids = CLKS_NR_MISC, >> .clk_regs = misc_clk_regs, >> .nr_clk_regs = ARRAY_SIZE(misc_clk_regs), >> - .clk_name = "dout_cmu_misc_bus", >> + .clk_name = "bus", > > Yes, obviously, the names are used... > > The entire point was that a week ago Rob said: > "As that has not made a release, please fix all of them." > but if you keep waiting, like 8 days for this simple patch, his > statement is not true anymore. > I saw the problem at the end of Thursday, reported it, and after that I entered vacation. > The only point was to send a fix *the next day*, so I would apply it and > send further. You kind of solved that problem by waiting entire week for > a simple driver and DTS change. > Why can't we queue the name fix as a patch for v6.8-rc1? Of course if you consider that the change is worth it. As I said, I lean towards not changing it. Thanks, ta
diff --git a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml index 3eebc03a309b..ba54c13c55bc 100644 --- a/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml +++ b/Documentation/devicetree/bindings/clock/google,gs101-clock.yaml @@ -30,14 +30,15 @@ properties: - google,gs101-cmu-top - google,gs101-cmu-apm - google,gs101-cmu-misc + - google,gs101-cmu-peric0 clocks: minItems: 1 - maxItems: 2 + maxItems: 3 clock-names: minItems: 1 - maxItems: 2 + maxItems: 3 "#clock-cells": const: 1 @@ -88,6 +89,26 @@ allOf: - const: dout_cmu_misc_bus - const: dout_cmu_misc_sss + - if: + properties: + compatible: + contains: + const: google,gs101-cmu-peric0 + + then: + properties: + clocks: + items: + - description: External reference clock (24.576 MHz) + - description: Connectivity Peripheral 0 bus clock (from CMU_TOP) + - description: Connectivity Peripheral 0 IP clock (from CMU_TOP) + + clock-names: + items: + - const: oscclk + - const: dout_cmu_peric0_bus + - const: dout_cmu_peric0_ip + additionalProperties: false examples: diff --git a/include/dt-bindings/clock/google,gs101.h b/include/dt-bindings/clock/google,gs101.h index 21adec22387c..7d7a896416a7 100644 --- a/include/dt-bindings/clock/google,gs101.h +++ b/include/dt-bindings/clock/google,gs101.h @@ -389,4 +389,90 @@ #define CLK_GOUT_MISC_WDT_CLUSTER1_PCLK 73 #define CLK_GOUT_MISC_XIU_D_MISC_ACLK 74 +/* CMU_PERIC0 */ +/* CMU_PERIC0 MUX */ +#define CLK_MOUT_PERIC0_BUS_USER 1 +#define CLK_MOUT_PERIC0_I3C_USER 2 +#define CLK_MOUT_PERIC0_USI0_UART_USER 3 +#define CLK_MOUT_PERIC0_USI14_USI_USER 4 +#define CLK_MOUT_PERIC0_USI1_USI_USER 5 +#define CLK_MOUT_PERIC0_USI2_USI_USER 6 +#define CLK_MOUT_PERIC0_USI3_USI_USER 7 +#define CLK_MOUT_PERIC0_USI4_USI_USER 8 +#define CLK_MOUT_PERIC0_USI5_USI_USER 9 +#define CLK_MOUT_PERIC0_USI6_USI_USER 10 +#define CLK_MOUT_PERIC0_USI7_USI_USER 11 +#define CLK_MOUT_PERIC0_USI8_USI_USER 12 + +/* CMU_PERIC0 Dividers */ +#define CLK_DOUT_PERIC0_I3C 13 +#define CLK_DOUT_PERIC0_USI0_UART 14 +#define CLK_DOUT_PERIC0_USI14_USI 15 +#define CLK_DOUT_PERIC0_USI1_USI 16 +#define CLK_DOUT_PERIC0_USI2_USI 17 +#define CLK_DOUT_PERIC0_USI3_USI 18 +#define CLK_DOUT_PERIC0_USI4_USI 19 +#define CLK_DOUT_PERIC0_USI5_USI 20 +#define CLK_DOUT_PERIC0_USI6_USI 21 +#define CLK_DOUT_PERIC0_USI7_USI 22 +#define CLK_DOUT_PERIC0_USI8_USI 23 + +/* CMU_PERIC0 Gates */ +#define CLK_GOUT_PERIC0_IP 24 +#define CLK_GOUT_PERIC0_PERIC0_CMU_PERIC0_PCLK 25 +#define CLK_GOUT_PERIC0_CLK_PERIC0_OSCCLK_CLK 26 +#define CLK_GOUT_PERIC0_D_TZPC_PERIC0_PCLK 27 +#define CLK_GOUT_PERIC0_GPC_PERIC0_PCLK 28 +#define CLK_GOUT_PERIC0_GPIO_PERIC0_PCLK 29 +#define CLK_GOUT_PERIC0_LHM_AXI_P_PERIC0_I_CLK 30 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_0 31 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_1 32 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_10 33 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_11 34 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_12 35 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_13 36 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_14 37 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_15 38 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_2 39 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_3 40 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_4 41 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_5 42 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_6 43 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_7 44 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_8 45 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_IPCLK_9 46 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_0 47 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_1 48 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_10 49 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_11 50 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_12 51 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_13 52 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_14 53 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_15 54 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_2 55 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_3 56 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_4 57 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_5 58 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_6 59 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_7 60 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_8 61 +#define CLK_GOUT_PERIC0_PERIC0_TOP0_PCLK_9 62 +#define CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_0 63 +#define CLK_GOUT_PERIC0_PERIC0_TOP1_IPCLK_2 64 +#define CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_0 65 +#define CLK_GOUT_PERIC0_PERIC0_TOP1_PCLK_2 66 +#define CLK_GOUT_PERIC0_CLK_PERIC0_BUSP_CLK 67 +#define CLK_GOUT_PERIC0_CLK_PERIC0_I3C_CLK 68 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI0_UART_CLK 69 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI14_USI_CLK 70 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI1_USI_CLK 71 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI2_USI_CLK 72 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI3_USI_CLK 73 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI4_USI_CLK 74 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI5_USI_CLK 75 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI6_USI_CLK 76 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI7_USI_CLK 77 +#define CLK_GOUT_PERIC0_CLK_PERIC0_USI8_USI_CLK 78 +#define CLK_GOUT_PERIC0_SYSREG_PERIC0_PCLK 79 + #endif /* _DT_BINDINGS_CLOCK_GOOGLE_GS101_H */
Add dt-schema documentation for the Connectivity Peripheral 0 (PERIC0) clock management unit. Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org> --- .../bindings/clock/google,gs101-clock.yaml | 25 +++++- include/dt-bindings/clock/google,gs101.h | 86 +++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-)