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

Inbaraj E
2024-09-20T04:12:45Z

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: 19 September 2024 17:33
> To: Inbaraj E <inbaraj.e@samsung.com>; 'Stephen Boyd'
> <sboyd@kernel.org>; alim.akhtar@samsung.com; cw00.choi@samsung.com;
> linux-clk@vger.kernel.org; linux-kernel@vger.kernel.org; linux-samsung-
> soc@vger.kernel.org; mturquette@baylibre.com; s.nawrocki@samsung.com
> Cc: pankaj.dubey@samsung.com; gost.dev@samsung.com
> Subject: Re: [PATCH] clk: samsung: fsd: Mark PLL_CAM_CSI as critical
>
> On 19/09/2024 13:33, Inbaraj E wrote:
> >
> >
> >> -----Original Message-----
> >> From: Stephen Boyd <sboyd@kernel.org>
> >> Sent: 19 September 2024 15:51
> >> To: Inbaraj E <inbaraj.e@samsung.com>; alim.akhtar@samsung.com;
> >> cw00.choi@samsung.com; krzk@kernel.org; linux-clk@vger.kernel.org;
> >> linux- kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org;
> >> mturquette@baylibre.com; s.nawrocki@samsung.com
> >> Cc: pankaj.dubey@samsung.com; gost.dev@samsung.com; Inbaraj E
> >> <inbaraj.e@samsung.com>
> >> Subject: Re: [PATCH] clk: samsung: fsd: Mark PLL_CAM_CSI as critical
> >>
> >> Quoting Inbaraj E (2024-09-17 03:10:16)
> >>> PLL_CAM_CSI is the parent clock for the ACLK and PCLK in the
> >>> CMU_CAM_CSI block. When we gate ACLK or PCLK, the clock framework
> >> will
> >>> subsequently disables the parent clocks(PLL_CAM_CSI). Disabling
> >>> PLL_CAM_CSI is causing sytem level halt.
> >>>
> >>> It was observed on FSD SoC, when we gate the ACLK and PCLK during
> >>> CSI stop streaming through pm_runtime_put system is getting halted.
> >>> So marking PLL_CAM_CSI as critical to prevent disabling.
> >>>
> >>> Signed-off-by: Inbaraj E <inbaraj.e@samsung.com>
> >>> ---
> >>
> >> Please add a fixes tag. Although this is likely a band-aid fix
> >> because marking something critical leaves it enabled forever.
> >
> > Sure, will add fixes tag. As per HW manual, this PLL_CAM_CSI is
> > supplying clock even for CMU SFR access of CSI block, so we can't gate
> > this.
>
> Hm, I am not so sure. The CMU driver should just take appropriate clock.
> Sprinkling CLK_CRITICAL looks as substitute of missing clock handling/

As per HW design, PLL_CAM_CSI is responsible for suppling clock to
CSI SFR, CMU SFR and some internal block of CAM_CSI. In this some of
the clock is not handled by any driver but it is required for CSI to
work properly. For example CSI NOC clock. So this is the reason we
are marking PLL_CAM_CSI as critical.

>
>
> Best regards,
> Krzysztof