linux-kernel - Re: [PATCH] clk: samsung: fsd: Mark PLL_CAM

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]

Message-ID: <6f090ce4-fa29-40c5-8959-b57d135b3a31@kernel.org>
Date: Mon, 21 Oct 2024 11:50:57 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Inbaraj E <inbaraj.e@...sung.com>, 'Stephen Boyd' <sboyd@...nel.org>,
 alim.akhtar@...sung.com, cw00.choi@...sung.com, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-samsung-soc@...r.kernel.org,
 mturquette@...libre.com, s.nawrocki@...sung.com
Cc: pankaj.dubey@...sung.com, gost.dev@...sung.com
Subject: Re: [PATCH] clk: samsung: fsd: Mark PLL_CAM_CSI as critical

On 10/10/2024 12:45, Inbaraj E wrote:
>>> Now if we look into CSI driver perspective it needs only ACLK and PCLK
>>> clocks for it's operations. But to access CMU SFRs (including
>>> ACLK/PCLK or any other CMU SFR of BLK_CSI) we need parent clock keep
>>> supplying clocks. While we try to gate ACLK clock, due to propagation
>>> logic of clock gating the CCF scans all the clocks from leaf level to
>>> the parent clock and tries to gate clocks if enable/disable ops is
>>> valid for any such clock.
>>>
>>> Issue here is that we are trying to gate PLL_CAM_CSI which itself is
>>> accessible only when this clock is enabled. In fact none of CMU_SFR
>>> will be accessible as soon as PLL_CAM_CSI is gated. CSI driver is not
>>> intended
>>
>> Obviously, but your CMU is taking the necessary clock and enabled it so what
>> is the problem?
>>
>>> to gate this PLL clock but only the leaf level clock which is
>>> supplying to CSI IP. So in absence of any alternate source of clock
>>> hierarchy which can supply clock for CMU_CSI we can't gate PLL_CAM_CSI.
>>>
>>> Please let us know if you have any other queries why we are insisting
>>> on marking PLL_CAM_CSI as CRITICAL clock.
>>
>> This is so far quite obvious - just like in all other cases, you need the top clock
>> taken by proper driver. I don't think you are looking at right drivers and right
>> problem here.
>>
> 
> Hi Krzysztof,
> 
> In this case, platform driver need to get this PLL clock and keep it
> enabled always. As PLL_CAM_CSI is source clock for accessing CMU
> registers of CSI block, and PLL_CAM_CSI itself lies in CSI_CMU,
> driver need to prepare and keep it enabled always. This way other PCLK
> and ACLK clocks can be gated. But the PLL_CAM_CSI which is parent of the
> PCLK and ACLK gate clock won't be disabled. Hope this should not be a
> concern. 
> 

Yes, that's expected. It properly models the hierarchy and consumers of
clocks, while keeping something as critical does not model the consumers.

Best regards,
Krzysztof