linux-kernel - Re: samsung: clk: re-parent MUX to OSCCLK at run-time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 22 Mar 2024 09:17:08 +0000
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: 김재원/JAEWON KIM <jaewon02.kim@...sung.com>,
 'Sylwester Nawrocki' <s.nawrocki@...sung.com>,
 'Chanwoo Choi' <cw00.choi@...sung.com>,
 'Alim Akhtar' <alim.akhtar@...sung.com>
Cc: 'Sam Protsenko' <semen.protsenko@...aro.org>,
 'Krzysztof Kozlowski' <krzysztof.kozlowski@...aro.org>,
 linux-samsung-soc@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 'linux-arm-kernel' <linux-arm-kernel@...ts.infradead.org>,
 'Peter Griffin' <peter.griffin@...aro.org>,
 'André Draszik' <andre.draszik@...aro.org>,
 'William McVicker' <willmcvicker@...gle.com>, kernel-team@...roid.com
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