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 */
+/* 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-googOn 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, 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
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>
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 >
© 2016 - 2025 Red Hat, Inc.