Re: [PATCH] clk: samsung: fsd: Mark PLL_CAM_CSI as critical

Krzysztof Kozlowski
2024-10-21T09:51:03Z

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