[v4,1/7] dt-bindings: clock: exynos850: Add bindings for Exynos850 sysreg clocks

Commit Message

Sam Protsenko Dec. 17, 2021, 4:15 p.m. UTC

System Register is used to configure system behavior, like USI protocol,
etc. SYSREG clocks should be provided to corresponding syscon nodes, to
make it possible to modify SYSREG registers.

While at it, add also missing PMU and GPIO clocks, which looks necessary
and might be needed for corresponding Exynos850 features soon.

Reviewed-by: Krzysztof Kozlowski <[e-mail]>
Acked-by: Rob Herring <[e-mail]>
Acked-by: Chanwoo Choi <[e-mail]>
Signed-off-by: Sam Protsenko <[e-mail]>
---
Changes in v4:
  - (none)

Changes in v3:
  - (none)

Changes in v2:
  - Added R-b tag by Krzysztof Kozlowski
  - Added Ack tag by Rob Herring
  - Added Ack tag by Chanwoo Choi

 include/dt-bindings/clock/exynos850.h | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Sylwester Nawrocki Dec. 19, 2021, 10:29 p.m. UTC | #1

On 17.12.2021 17:15, Sam Protsenko wrote:
> System Register is used to configure system behavior, like USI protocol,
> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
> make it possible to modify SYSREG registers.
> 
> While at it, add also missing PMU and GPIO clocks, which looks necessary
> and might be needed for corresponding Exynos850 features soon.
> 
> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
> Acked-by: Rob Herring<[e-mail]>
> Acked-by: Chanwoo Choi<[e-mail]>
> Signed-off-by: Sam Protsenko<[e-mail]>

Apologies for late reply, this patch is applied now.

Krzysztof Kozlowski Dec. 20, 2021, 9:31 a.m. UTC | #2

On 19/12/2021 23:29, Sylwester Nawrocki wrote:
> On 17.12.2021 17:15, Sam Protsenko wrote:
>> System Register is used to configure system behavior, like USI protocol,
>> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
>> make it possible to modify SYSREG registers.
>>
>> While at it, add also missing PMU and GPIO clocks, which looks necessary
>> and might be needed for corresponding Exynos850 features soon.
>>
>> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
>> Acked-by: Rob Herring<[e-mail]>
>> Acked-by: Chanwoo Choi<[e-mail]>
>> Signed-off-by: Sam Protsenko<[e-mail]>
> 
> Apologies for late reply, this patch is applied now.
> 

Sam,

The clock is used in the DTSI, so since this was applied, there are only
two choices now:
1. wait for next cycle with DTSI and DTS,
2. Resubmit with replacing the newly added clocks in DTSI/DTS with
numbers and a TODO note.

Best regards,
Krzysztof

Sam Protsenko Dec. 20, 2021, 2:55 p.m. UTC | #3

On Mon, 20 Dec 2021 at 11:31, Krzysztof Kozlowski
<[e-mail]> wrote:
>
> On 19/12/2021 23:29, Sylwester Nawrocki wrote:
> > On 17.12.2021 17:15, Sam Protsenko wrote:
> >> System Register is used to configure system behavior, like USI protocol,
> >> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
> >> make it possible to modify SYSREG registers.
> >>
> >> While at it, add also missing PMU and GPIO clocks, which looks necessary
> >> and might be needed for corresponding Exynos850 features soon.
> >>
> >> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
> >> Acked-by: Rob Herring<[e-mail]>
> >> Acked-by: Chanwoo Choi<[e-mail]>
> >> Signed-off-by: Sam Protsenko<[e-mail]>
> >
> > Apologies for late reply, this patch is applied now.
> >
>
> Sam,
>
> The clock is used in the DTSI, so since this was applied, there are only
> two choices now:
> 1. wait for next cycle with DTSI and DTS,
> 2. Resubmit with replacing the newly added clocks in DTSI/DTS with
> numbers and a TODO note.
>

But why? I thought because Sylwester applied my clock patches, those
will get into v5.17, and so DTSI/DTS might rely on those clocks? If I
get it wrong, please let me know why, and I'll go with item (2) you
suggested.

> Best regards,
> Krzysztof

Krzysztof Kozlowski Dec. 21, 2021, 8:19 a.m. UTC | #4

On 20/12/2021 15:55, Sam Protsenko wrote:
> On Mon, 20 Dec 2021 at 11:31, Krzysztof Kozlowski
> <[e-mail]> wrote:
>>
>> On 19/12/2021 23:29, Sylwester Nawrocki wrote:
>>> On 17.12.2021 17:15, Sam Protsenko wrote:
>>>> System Register is used to configure system behavior, like USI protocol,
>>>> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
>>>> make it possible to modify SYSREG registers.
>>>>
>>>> While at it, add also missing PMU and GPIO clocks, which looks necessary
>>>> and might be needed for corresponding Exynos850 features soon.
>>>>
>>>> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
>>>> Acked-by: Rob Herring<[e-mail]>
>>>> Acked-by: Chanwoo Choi<[e-mail]>
>>>> Signed-off-by: Sam Protsenko<[e-mail]>
>>>
>>> Apologies for late reply, this patch is applied now.
>>>
>>
>> Sam,
>>
>> The clock is used in the DTSI, so since this was applied, there are only
>> two choices now:
>> 1. wait for next cycle with DTSI and DTS,
>> 2. Resubmit with replacing the newly added clocks in DTSI/DTS with
>> numbers and a TODO note.
>>
> 
> But why? I thought because Sylwester applied my clock patches, those
> will get into v5.17, and so DTSI/DTS might rely on those clocks? If I
> get it wrong, please let me know why, and I'll go with item (2) you
> suggested.

If I apply the DTSI+DTS, all my builds will start failing. The
linux-next (since Sylwester's tree is included) should build fine, but
my tree won't be buildable anymore. Then arm-soc pulls my tree and gets
said because it does not build. Later, Linus will be unhappy if he pulls
arm-soc (thus mine) before clock tree.

Other solution, instead of using raw numbers, is to copy-paste the clock
macros you use directly in DTSI and do not include the clock header.
This actually might be cleaner choice - changes will be limited to one
place in DTSI.

Best regards,
Krzysztof

Krzysztof Kozlowski Dec. 21, 2021, 8:20 a.m. UTC | #5

On 21/12/2021 09:19, Krzysztof Kozlowski wrote:
> On 20/12/2021 15:55, Sam Protsenko wrote:
>> On Mon, 20 Dec 2021 at 11:31, Krzysztof Kozlowski
>> <[e-mail]> wrote:
>>>
>>> On 19/12/2021 23:29, Sylwester Nawrocki wrote:
>>>> On 17.12.2021 17:15, Sam Protsenko wrote:
>>>>> System Register is used to configure system behavior, like USI protocol,
>>>>> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
>>>>> make it possible to modify SYSREG registers.
>>>>>
>>>>> While at it, add also missing PMU and GPIO clocks, which looks necessary
>>>>> and might be needed for corresponding Exynos850 features soon.
>>>>>
>>>>> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
>>>>> Acked-by: Rob Herring<[e-mail]>
>>>>> Acked-by: Chanwoo Choi<[e-mail]>
>>>>> Signed-off-by: Sam Protsenko<[e-mail]>
>>>>
>>>> Apologies for late reply, this patch is applied now.
>>>>
>>>
>>> Sam,
>>>
>>> The clock is used in the DTSI, so since this was applied, there are only
>>> two choices now:
>>> 1. wait for next cycle with DTSI and DTS,
>>> 2. Resubmit with replacing the newly added clocks in DTSI/DTS with
>>> numbers and a TODO note.
>>>
>>
>> But why? I thought because Sylwester applied my clock patches, those
>> will get into v5.17, and so DTSI/DTS might rely on those clocks? If I
>> get it wrong, please let me know why, and I'll go with item (2) you
>> suggested.
> 
> If I apply the DTSI+DTS, all my builds will start failing. The
> linux-next (since Sylwester's tree is included) should build fine, but
> my tree won't be buildable anymore. Then arm-soc pulls my tree and gets
> said because it does not build. Later, Linus will be unhappy if he pulls

s/said/sad/ obviously :)

> arm-soc (thus mine) before clock tree.
> 
> Other solution, instead of using raw numbers, is to copy-paste the clock
> macros you use directly in DTSI and do not include the clock header.
> This actually might be cleaner choice - changes will be limited to one
> place in DTSI.
> 
> Best regards,
> Krzysztof
> 


Best regards,
Krzysztof

Sam Protsenko Dec. 21, 2021, 12:09 p.m. UTC | #6

On Tue, 21 Dec 2021 at 10:19, Krzysztof Kozlowski
<[e-mail]> wrote:
>
> On 20/12/2021 15:55, Sam Protsenko wrote:
> > On Mon, 20 Dec 2021 at 11:31, Krzysztof Kozlowski
> > <[e-mail]> wrote:
> >>
> >> On 19/12/2021 23:29, Sylwester Nawrocki wrote:
> >>> On 17.12.2021 17:15, Sam Protsenko wrote:
> >>>> System Register is used to configure system behavior, like USI protocol,
> >>>> etc. SYSREG clocks should be provided to corresponding syscon nodes, to
> >>>> make it possible to modify SYSREG registers.
> >>>>
> >>>> While at it, add also missing PMU and GPIO clocks, which looks necessary
> >>>> and might be needed for corresponding Exynos850 features soon.
> >>>>
> >>>> Reviewed-by: Krzysztof Kozlowski<[e-mail]>
> >>>> Acked-by: Rob Herring<[e-mail]>
> >>>> Acked-by: Chanwoo Choi<[e-mail]>
> >>>> Signed-off-by: Sam Protsenko<[e-mail]>
> >>>
> >>> Apologies for late reply, this patch is applied now.
> >>>
> >>
> >> Sam,
> >>
> >> The clock is used in the DTSI, so since this was applied, there are only
> >> two choices now:
> >> 1. wait for next cycle with DTSI and DTS,
> >> 2. Resubmit with replacing the newly added clocks in DTSI/DTS with
> >> numbers and a TODO note.
> >>
> >
> > But why? I thought because Sylwester applied my clock patches, those
> > will get into v5.17, and so DTSI/DTS might rely on those clocks? If I
> > get it wrong, please let me know why, and I'll go with item (2) you
> > suggested.
>
> If I apply the DTSI+DTS, all my builds will start failing. The
> linux-next (since Sylwester's tree is included) should build fine, but
> my tree won't be buildable anymore. Then arm-soc pulls my tree and gets
> said because it does not build. Later, Linus will be unhappy if he pulls
> arm-soc (thus mine) before clock tree.
>

I see. Thanks for the explanation!

> Other solution, instead of using raw numbers, is to copy-paste the clock
> macros you use directly in DTSI and do not include the clock header.
> This actually might be cleaner choice - changes will be limited to one
> place in DTSI.
>

Will do so in v5.

> Best regards,
> Krzysztof

Patch

diff --git a/include/dt-bindings/clock/exynos850.h b/include/dt-bindings/clock/exynos850.h
index 8aa5e82af0d3..0b6a3c6a7c90 100644
--- a/include/dt-bindings/clock/exynos850.h
+++ b/include/dt-bindings/clock/exynos850.h
@@ -82,7 +82,10 @@ 
 #define CLK_GOUT_I3C_PCLK		19
 #define CLK_GOUT_I3C_SCLK		20
 #define CLK_GOUT_SPEEDY_PCLK		21
-#define APM_NR_CLK			22
+#define CLK_GOUT_GPIO_ALIVE_PCLK	22
+#define CLK_GOUT_PMU_ALIVE_PCLK		23
+#define CLK_GOUT_SYSREG_APM_PCLK	24
+#define APM_NR_CLK			25
 
 /* CMU_CMGP */
 #define CLK_RCO_CMGP			1
@@ -99,7 +102,8 @@ 
 #define CLK_GOUT_CMGP_USI0_PCLK		12
 #define CLK_GOUT_CMGP_USI1_IPCLK	13
 #define CLK_GOUT_CMGP_USI1_PCLK		14
-#define CMGP_NR_CLK			15
+#define CLK_GOUT_SYSREG_CMGP_PCLK	15
+#define CMGP_NR_CLK			16
 
 /* CMU_HSI */
 #define CLK_MOUT_HSI_BUS_USER		1
@@ -167,7 +171,9 @@ 
 #define CLK_GOUT_MMC_EMBD_SDCLKIN	10
 #define CLK_GOUT_SSS_ACLK		11
 #define CLK_GOUT_SSS_PCLK		12
-#define CORE_NR_CLK			13
+#define CLK_GOUT_GPIO_CORE_PCLK		13
+#define CLK_GOUT_SYSREG_CORE_PCLK	14
+#define CORE_NR_CLK			15
 
 /* CMU_DPU */
 #define CLK_MOUT_DPU_USER		1