2024-03-06 03:20:20

by Tudor Ambarus

[permalink] [raw]
Subject: samsung: clk: re-parent MUX to OSCCLK at run-time

Hi,

Trying to get some feedback from the samsung experts. Please consider
the following:

---------------------------------------------
| CMU_PERIC0 |
| |
| MUX_USI |
| |
| |\ |
OSCCLK ---|->| \ |
| | \ |
| | M | |
| | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
| | X | (1 ~ 16) |
| | / |
DIV_CLKCMU_PERIC0_IP ---|->| / |
(1 ~ 16) | | |/ |
| | |
| | |
| | MUX_I3C |
| | |
| | |\ |
--|->| \ |
| | \ |
| | M | |
| | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
| | X | |
| | / |
OSCCLK ---|->| / |
| |/ |
| |
---------------------------------------------

Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,
during normal operation mode? Experimentally I determined that it's
fine, but the datasheet that I'm reading mentions OSCCLK just in the
low-power mode context:
i/ CMU ... "Communicates with Power Management Unit (PMU) to stop clocks
or switch OSC clock before entering a Low-Power mode to reduce power
consumption by minimizing clock toggling".
ii/ "All CMUs have MUXs to change the OSCCLK during power-down mode".

Re-parenting the MUX to OSCCLK allows lower clock rates for the USI
blocks than the DIV_CLK_PERIC0_USI can offer. For a USI clock rate below
6.25 MHz I have to either reparent MUX_USI to OSCCLK, or to propagate
the clock rate to the common divider DIV_CLKCMU_PERIC0_IP. I find the
propagation to the common DIV less desirable as a low USI clock rate
affects I3C by lowering its clock rate too. Worse, if the common bus
divider is not protected (using CLK_SET_RATE_GATE), USI can lower the
I3C clock rate without I3C noticing.

Either re-parenting the MUX_USI to OSCCLK, or propagating the clock rate
to DIV_CLKCMU_PERIC0_IP allows the same clock ranges. The first with the
benefit of not affecting the clock rate of I3C for USI clock rates below
6.25 MHz. Is it fine to re-parent MUX_USI to OSCCLK at run-time?

If no feedback is received I lean towards propagating the USI clock rate
to the common divider, but by protecting it with CLK_SET_RATE_GATE.

Feel free to add in To: or Cc: whoever might be interested. Thanks,
ta


2024-03-06 04:49:49

by Alim Akhtar

[permalink] [raw]
Subject: RE: samsung: clk: re-parent MUX to OSCCLK at run-time

Hi Tudor

> -----Original Message-----
> From: Tudor Ambarus <[email protected]>
> Sent: Wednesday, March 6, 2024 8:50 AM
> To: Sylwester Nawrocki <[email protected]>; Chanwoo Choi
> <[email protected]>; Alim Akhtar <[email protected]>
> Cc: Sam Protsenko <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; [email protected];
> [email protected]; [email protected]; linux-arm-kernel
> <[email protected]>; Peter Griffin
> <[email protected]>; André Draszik <[email protected]>;
> William McVicker <[email protected]>; [email protected]
> Subject: samsung: clk: re-parent MUX to OSCCLK at run-time
>
> Hi,
>
> Trying to get some feedback from the samsung experts. Please consider the
> following:
>
> ---------------------------------------------
> | CMU_PERIC0 |
> | |
> | MUX_USI |
> | |
> | |\ |
> OSCCLK ---|->| \ |
> | | \ |
> | | M | |
> | | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
> | | X | (1 ~ 16) |
> | | / |
> DIV_CLKCMU_PERIC0_IP ---|->| / |
> (1 ~ 16) | | |/ |
> | | |
> | | |
> | | MUX_I3C |
> | | |
> | | |\ |
> --|->| \ |
> | | \ |
> | | M | |
> | | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
> | | X | |
> | | / |
> OSCCLK ---|->| / |
> | |/ |
> | |
> ---------------------------------------------
>
> Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,

I am not aware of the exact SOC/HW you are working on.
It depends on the CMU design about how to achieve low power mode and clock gating for an IP/Block.

In theory and looking at your clock diagram above, it is ok to switch to OSCCLK for MUX_USI.

If you can just use GATE_USI clock to clock gate USI IP, you will have a low power for USI (of course there will be a leakage current still drawn).
Is that what you want to achieve (low power mode)? Or you are looking to get lowest possible operating clock for USI IP?

You need to takecare about if that clock is being shared with any other IP,
so unless all the IPs which consume this clock, goes into idle state, you can avoid MUX_USI change to OSCCLK.


> during normal operation mode? Experimentally I determined that it's fine,
> but the datasheet that I'm reading mentions OSCCLK just in the low-power
> mode context:
> i/ CMU ... "Communicates with Power Management Unit (PMU) to stop
> clocks or switch OSC clock before entering a Low-Power mode to reduce
> power consumption by minimizing clock toggling".
> ii/ "All CMUs have MUXs to change the OSCCLK during power-down mode".
>
> Re-parenting the MUX to OSCCLK allows lower clock rates for the USI blocks
> than the DIV_CLK_PERIC0_USI can offer. For a USI clock rate below
> 6.25 MHz I have to either reparent MUX_USI to OSCCLK, or to propagate the
> clock rate to the common divider DIV_CLKCMU_PERIC0_IP. I find the
> propagation to the common DIV less desirable as a low USI clock rate affects
> I3C by lowering its clock rate too. Worse, if the common bus divider is not
> protected (using CLK_SET_RATE_GATE), USI can lower the I3C clock rate
> without I3C noticing.
>
> Either re-parenting the MUX_USI to OSCCLK, or propagating the clock rate to
> DIV_CLKCMU_PERIC0_IP allows the same clock ranges. The first with the
> benefit of not affecting the clock rate of I3C for USI clock rates below
> 6.25 MHz. Is it fine to re-parent MUX_USI to OSCCLK at run-time?
>
> If no feedback is received I lean towards propagating the USI clock rate to the
> common divider, but by protecting it with CLK_SET_RATE_GATE.
>
> Feel free to add in To: or Cc: whoever might be interested. Thanks, ta



2024-03-06 06:09:49

by Tudor Ambarus

[permalink] [raw]
Subject: Re: samsung: clk: re-parent MUX to OSCCLK at run-time



On 3/6/24 04:49, Alim Akhtar wrote:
> Hi Tudor

Hi!

>
>> -----Original Message-----
>> From: Tudor Ambarus <[email protected]>
>> Sent: Wednesday, March 6, 2024 8:50 AM
>> To: Sylwester Nawrocki <[email protected]>; Chanwoo Choi
>> <[email protected]>; Alim Akhtar <[email protected]>
>> Cc: Sam Protsenko <[email protected]>; Krzysztof Kozlowski
>> <[email protected]>; [email protected];
>> [email protected]; [email protected]; linux-arm-kernel
>> <[email protected]>; Peter Griffin
>> <[email protected]>; André Draszik <[email protected]>;
>> William McVicker <[email protected]>; [email protected]
>> Subject: samsung: clk: re-parent MUX to OSCCLK at run-time
>>
>> Hi,
>>
>> Trying to get some feedback from the samsung experts. Please consider the
>> following:
>>
>> ---------------------------------------------
>> | CMU_PERIC0 |
>> | |
>> | MUX_USI |
>> | |
>> | |\ |
>> OSCCLK ---|->| \ |
>> | | \ |
>> | | M | |
>> | | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
>> | | X | (1 ~ 16) |
>> | | / |
>> DIV_CLKCMU_PERIC0_IP ---|->| / |
>> (1 ~ 16) | | |/ |
>> | | |
>> | | |
>> | | MUX_I3C |
>> | | |
>> | | |\ |
>> --|->| \ |
>> | | \ |
>> | | M | |
>> | | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
>> | | X | |
>> | | / |
>> OSCCLK ---|->| / |
>> | |/ |
>> | |
>> ---------------------------------------------
>>
>> Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,
>
> I am not aware of the exact SOC/HW you are working on.

I'm working with GS101. I'm interested in exynos850 as well.

> It depends on the CMU design about how to achieve low power mode and clock gating for an IP/Block.
>
> In theory and looking at your clock diagram above, it is ok to switch to OSCCLK for MUX_USI.
>
> If you can just use GATE_USI clock to clock gate USI IP, you will have a low power for USI (of course there will be a leakage current still drawn).
> Is that what you want to achieve (low power mode)? Or you are looking to get lowest possible operating clock for USI IP?

I'm trying to get the lowest possible operating clock for the USI IP.

>
> You need to takecare about if that clock is being shared with any other IP,

It's not shared, the entire MUX USI, DIV, and GATE sequence is dedicated
per IP. GS101 has 15 USI blocks, each with its dedicated MUX-DIV-GATE
sequence of clocks.

> so unless all the IPs which consume this clock, goes into idle state, you can avoid MUX_USI change to OSCCLK.

Since the MUX USI is per IP, I guess I shouldn't be concerned about
this, right?

I'm trying to find out if it's OK to reparent the MUX to OSCCLK in
normal operation mode (not low power mode), in order to get the lowest
possible operating clock for the USI IP. Would be great if the decision
is backed up by some info from datasheet. Unfortunately the datasheet
that I have access to it's not explicit.

Thanks for the help!
ta

>
>
>> during normal operation mode? Experimentally I determined that it's fine,
>> but the datasheet that I'm reading mentions OSCCLK just in the low-power
>> mode context:
>> i/ CMU ... "Communicates with Power Management Unit (PMU) to stop
>> clocks or switch OSC clock before entering a Low-Power mode to reduce
>> power consumption by minimizing clock toggling".
>> ii/ "All CMUs have MUXs to change the OSCCLK during power-down mode".
>>
>> Re-parenting the MUX to OSCCLK allows lower clock rates for the USI blocks
>> than the DIV_CLK_PERIC0_USI can offer. For a USI clock rate below
>> 6.25 MHz I have to either reparent MUX_USI to OSCCLK, or to propagate the
>> clock rate to the common divider DIV_CLKCMU_PERIC0_IP. I find the
>> propagation to the common DIV less desirable as a low USI clock rate affects
>> I3C by lowering its clock rate too. Worse, if the common bus divider is not
>> protected (using CLK_SET_RATE_GATE), USI can lower the I3C clock rate
>> without I3C noticing.
>>
>> Either re-parenting the MUX_USI to OSCCLK, or propagating the clock rate to
>> DIV_CLKCMU_PERIC0_IP allows the same clock ranges. The first with the
>> benefit of not affecting the clock rate of I3C for USI clock rates below
>> 6.25 MHz. Is it fine to re-parent MUX_USI to OSCCLK at run-time?
>>
>> If no feedback is received I lean towards propagating the USI clock rate to the
>> common divider, but by protecting it with CLK_SET_RATE_GATE.
>>
>> Feel free to add in To: or Cc: whoever might be interested. Thanks, ta
>
>

2024-03-07 02:05:32

by Alim Akhtar

[permalink] [raw]
Subject: RE: samsung: clk: re-parent MUX to OSCCLK at run-time

Hi Tudor

> -----Original Message-----
> From: Tudor Ambarus <[email protected]>
> Sent: Wednesday, March 6, 2024 11:40 AM
> To: Alim Akhtar <[email protected]>; 'Sylwester Nawrocki'
> <[email protected]>; 'Chanwoo Choi' <[email protected]>
> Cc: 'Sam Protsenko' <[email protected]>; 'Krzysztof Kozlowski'
> <[email protected]>; [email protected];
> [email protected]; [email protected]; 'linux-arm-kernel'
> <[email protected]>; 'Peter Griffin'
> <[email protected]>; 'André Draszik' <[email protected]>;
> 'William McVicker' <[email protected]>; [email protected];
> [email protected]
> Subject: Re: samsung: clk: re-parent MUX to OSCCLK at run-time
>
>
>
> On 3/6/24 04:49, Alim Akhtar wrote:
> > Hi Tudor
>
> Hi!
>
> >
> >> -----Original Message-----
> >> From: Tudor Ambarus <[email protected]>
> >> Sent: Wednesday, March 6, 2024 8:50 AM
> >> To: Sylwester Nawrocki <[email protected]>; Chanwoo Choi
> >> <[email protected]>; Alim Akhtar <[email protected]>
> >> Cc: Sam Protsenko <[email protected]>; Krzysztof Kozlowski
> >> <[email protected]>; [email protected];
> >> [email protected]; [email protected];
> >> linux-arm-kernel <[email protected]>; Peter
> >> Griffin <[email protected]>; André Draszik
> >> <[email protected]>; William McVicker
> >> <[email protected]>; [email protected]
> >> Subject: samsung: clk: re-parent MUX to OSCCLK at run-time
> >>
> >> Hi,
> >>
> >> Trying to get some feedback from the samsung experts. Please consider
> >> the
> >> following:
> >>
> >> ---------------------------------------------
> >> | CMU_PERIC0 |
> >> | |
> >> | MUX_USI |
> >> | |
> >> | |\ |
> >> OSCCLK ---|->| \ |
> >> | | \ |
> >> | | M | |
> >> | | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
> >> | | X | (1 ~ 16) |
> >> | | / |
> >> DIV_CLKCMU_PERIC0_IP ---|->| / |
> >> (1 ~ 16) | | |/ |
> >> | | |
> >> | | |
> >> | | MUX_I3C |
> >> | | |
> >> | | |\ |
> >> --|->| \ |
> >> | | \ |
> >> | | M | |
> >> | | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
> >> | | X | |
> >> | | / |
> >> OSCCLK ---|->| / |
> >> | |/ |
> >> | |
> >>
> >> ---------------------------------------------
> >>
> >> Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,
> >
> > I am not aware of the exact SOC/HW you are working on.
>
> I'm working with GS101. I'm interested in exynos850 as well.
>
> > It depends on the CMU design about how to achieve low power mode and
> clock gating for an IP/Block.
> >
> > In theory and looking at your clock diagram above, it is ok to switch to
> OSCCLK for MUX_USI.
> >
> > If you can just use GATE_USI clock to clock gate USI IP, you will have a low
> power for USI (of course there will be a leakage current still drawn).
> > Is that what you want to achieve (low power mode)? Or you are looking to
> get lowest possible operating clock for USI IP?
>
> I'm trying to get the lowest possible operating clock for the USI IP.
>
> >
> > You need to takecare about if that clock is being shared with any
> > other IP,
>
> It's not shared, the entire MUX USI, DIV, and GATE sequence is dedicated
> per IP. GS101 has 15 USI blocks, each with its dedicated MUX-DIV-GATE
> sequence of clocks.
>
> > so unless all the IPs which consume this clock, goes into idle state, you can
> avoid MUX_USI change to OSCCLK.
>
> Since the MUX USI is per IP, I guess I shouldn't be concerned about this,
> right?
Yes, that should be fine

>
> I'm trying to find out if it's OK to reparent the MUX to OSCCLK in normal
> operation mode (not low power mode), in order to get the lowest possible
> operating clock for the USI IP. Would be great if the decision is backed up by
> some info from datasheet. Unfortunately the datasheet that I have access to
> it's not explicit.
>
Unfortunately, I don't have access to either of the datasheets. So won't be able to provide input from datasheet.
Looking at your explanation above, like MUX USI is per IP, so it okay to switch to OSCCLK to get lowest possible clock for USI.
You need to do some more regression test with this change (I would still suggest to reach out to GS101 team to get the confirmation of any other side effect)
For me, it looks ok to use OSCCLK for USI

> Thanks for the help!
> ta
>
> >



2024-03-07 08:22:47

by Jaewon Kim

[permalink] [raw]
Subject: RE: samsung: clk: re-parent MUX to OSCCLK at run-time

Hi Tudor


On 3/6/24 12:20, Tudor Ambarus wrote:
>
> Hi,
>
> Trying to get some feedback from the samsung experts. Please consider the
> following:
>
> ---------------------------------------------
> | CMU_PERIC0 |
> | |
> | MUX_USI |
> | |
> | |\ |
> OSCCLK ---|->| \ |
> | | \ |
> | | M | |
> | | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
> | | X | (1 ~ 16) |
> | | / |
> DIV_CLKCMU_PERIC0_IP ---|->| / |
> (1 ~ 16) | | |/ |
> | | |
> | | |
> | | MUX_I3C |
> | | |
> | | |\ |
> --|->| \ |
> | | \ |
> | | M | |
> | | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
> | | X | |
> | | / |
> OSCCLK ---|->| / |
> | |/ |
> | |
> ---------------------------------------------
>
> Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,
> during normal operation mode? Experimentally I determined that it's fine,
> but the datasheet that I'm reading mentions OSCCLK just in the low-power
> mode context:
> i/ CMU ... "Communicates with Power Management Unit (PMU) to stop clocks
> or switch OSC clock before entering a Low-Power mode to reduce power
> consumption by minimizing clock toggling".
> ii/ "All CMUs have MUXs to change the OSCCLK during power-down mode".
>
> Re-parenting the MUX to OSCCLK allows lower clock rates for the USI blocks
> than the DIV_CLK_PERIC0_USI can offer. For a USI clock rate below
> 6.25 MHz I have to either reparent MUX_USI to OSCCLK, or to propagate the
> clock rate to the common divider DIV_CLKCMU_PERIC0_IP. I find the
> propagation to the common DIV less desirable as a low USI clock rate
> affects I3C by lowering its clock rate too. Worse, if the common bus
> divider is not protected (using CLK_SET_RATE_GATE), USI can lower the I3C
> clock rate without I3C noticing.
>
> Either re-parenting the MUX_USI to OSCCLK, or propagating the clock rate
> to DIV_CLKCMU_PERIC0_IP allows the same clock ranges. The first with the
> benefit of not affecting the clock rate of I3C for USI clock rates below
> 6.25 MHz. Is it fine to re-parent MUX_USI to OSCCLK at run-time?
>
> If no feedback is received I lean towards propagating the USI clock rate
> to the common divider, but by protecting it with CLK_SET_RATE_GATE.
>
> Feel free to add in To: or Cc: whoever might be interested. Thanks, ta


"DIV_CLK_PERIC0_USI" re-parent to OSCCLK is already used samsung downstream driver.
Looking at the samsung downstream SPI driver, if the SPI request clock is lower than the clock that can be supported by the CMU, it re-parents to OSCCLK.

There is no problem with clock switching before USI data transfer.

Thanks
Jaewon Kim


2024-03-22 09:17:42

by Tudor Ambarus

[permalink] [raw]
Subject: Re: samsung: clk: re-parent MUX to OSCCLK at run-time



On 3/7/24 08:21, 김재원/JAEWON KIM wrote:
> Hi Tudor

Hi, Jaewon!

>
>
> On 3/6/24 12:20, Tudor Ambarus wrote:
>>
>> Hi,
>>
>> Trying to get some feedback from the samsung experts. Please consider the
>> following:
>>
>> ---------------------------------------------
>> | CMU_PERIC0 |
>> | |
>> | MUX_USI |
>> | |
>> | |\ |
>> OSCCLK ---|->| \ |
>> | | \ |
>> | | M | |
>> | | U |--> DIV_CLK_PERIC0_USI*_ --> GATE_USI |
>> | | X | (1 ~ 16) |
>> | | / |
>> DIV_CLKCMU_PERIC0_IP ---|->| / |
>> (1 ~ 16) | | |/ |
>> | | |
>> | | |
>> | | MUX_I3C |
>> | | |
>> | | |\ |
>> --|->| \ |
>> | | \ |
>> | | M | |
>> | | U |--> DIV_CLK_PERIC0_I3C --> GATE_I3C |
>> | | X | |
>> | | / |
>> OSCCLK ---|->| / |
>> | |/ |
>> | |
>> ---------------------------------------------
>>
>> Is it fine to re-parent the MUX_USI from above to OSCCLK at run-time,
>> during normal operation mode? Experimentally I determined that it's fine,
>> but the datasheet that I'm reading mentions OSCCLK just in the low-power
>> mode context:
>> i/ CMU ... "Communicates with Power Management Unit (PMU) to stop clocks
>> or switch OSC clock before entering a Low-Power mode to reduce power
>> consumption by minimizing clock toggling".
>> ii/ "All CMUs have MUXs to change the OSCCLK during power-down mode".
>>
>> Re-parenting the MUX to OSCCLK allows lower clock rates for the USI blocks
>> than the DIV_CLK_PERIC0_USI can offer. For a USI clock rate below
>> 6.25 MHz I have to either reparent MUX_USI to OSCCLK, or to propagate the
>> clock rate to the common divider DIV_CLKCMU_PERIC0_IP. I find the
>> propagation to the common DIV less desirable as a low USI clock rate
>> affects I3C by lowering its clock rate too. Worse, if the common bus
>> divider is not protected (using CLK_SET_RATE_GATE), USI can lower the I3C
>> clock rate without I3C noticing.
>>
>> Either re-parenting the MUX_USI to OSCCLK, or propagating the clock rate
>> to DIV_CLKCMU_PERIC0_IP allows the same clock ranges. The first with the
>> benefit of not affecting the clock rate of I3C for USI clock rates below
>> 6.25 MHz. Is it fine to re-parent MUX_USI to OSCCLK at run-time?
>>
>> If no feedback is received I lean towards propagating the USI clock rate
>> to the common divider, but by protecting it with CLK_SET_RATE_GATE.
>>
>> Feel free to add in To: or Cc: whoever might be interested. Thanks, ta
>
>
> "DIV_CLK_PERIC0_USI" re-parent to OSCCLK is already used samsung downstream driver.
> Looking at the samsung downstream SPI driver, if the SPI request clock is lower than the clock that can be supported by the CMU, it re-parents to OSCCLK.

I've just verified the downstream driver, added some prints, and indeed
the MUX re-parents to OSCCLK on low clock rates.
>
> There is no problem with clock switching before USI data transfer.

I think that too. Thanks a lot, Jaewon!

Cheers,
ta