| Message ID | 20240917101016.23238-1-inbaraj.e@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | clk: samsung: fsd: Mark PLL_CAM_CSI as critical | expand |
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. > drivers/clk/samsung/clk-fsd.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/clk-fsd.c b/drivers/clk/samsung/clk-fsd.c > index 6f984cfcd33c..b1764aab9429 100644 > --- a/drivers/clk/samsung/clk-fsd.c > +++ b/drivers/clk/samsung/clk-fsd.c > @@ -1637,8 +1637,9 @@ static const struct samsung_pll_rate_table pll_cam_csi_rate_table[] __initconst > }; > > static const struct samsung_pll_clock cam_csi_pll_clks[] __initconst = { > - PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", > - PLL_LOCKTIME_PLL_CAM_CSI, PLL_CON0_PLL_CAM_CSI, pll_cam_csi_rate_table), > + __PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", > + CLK_GET_RATE_NOCACHE | CLK_IS_CRITICAL, PLL_LOCKTIME_PLL_CAM_CSI, Please add a comment indicating that this clk can never turn off because <insert reason here>.
> -----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. > > > drivers/clk/samsung/clk-fsd.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/clk/samsung/clk-fsd.c > > b/drivers/clk/samsung/clk-fsd.c index 6f984cfcd33c..b1764aab9429 > > 100644 > > --- a/drivers/clk/samsung/clk-fsd.c > > +++ b/drivers/clk/samsung/clk-fsd.c > > @@ -1637,8 +1637,9 @@ static const struct samsung_pll_rate_table > > pll_cam_csi_rate_table[] __initconst }; > > > > static const struct samsung_pll_clock cam_csi_pll_clks[] __initconst = { > > - PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", > > - PLL_LOCKTIME_PLL_CAM_CSI, PLL_CON0_PLL_CAM_CSI, > pll_cam_csi_rate_table), > > + __PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", > > + CLK_GET_RATE_NOCACHE | CLK_IS_CRITICAL, > > + PLL_LOCKTIME_PLL_CAM_CSI, > > Please add a comment indicating that this clk can never turn off because > <insert reason here>. Sure, will post v2 after adding comment explaining reason behind marking this clock as critical. Regards, Inbaraj E
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/ Best regards, Krzysztof
diff --git a/drivers/clk/samsung/clk-fsd.c b/drivers/clk/samsung/clk-fsd.c index 6f984cfcd33c..b1764aab9429 100644 --- a/drivers/clk/samsung/clk-fsd.c +++ b/drivers/clk/samsung/clk-fsd.c @@ -1637,8 +1637,9 @@ static const struct samsung_pll_rate_table pll_cam_csi_rate_table[] __initconst }; static const struct samsung_pll_clock cam_csi_pll_clks[] __initconst = { - PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", - PLL_LOCKTIME_PLL_CAM_CSI, PLL_CON0_PLL_CAM_CSI, pll_cam_csi_rate_table), + __PLL(pll_142xx, 0, "fout_pll_cam_csi", "fin_pll", + CLK_GET_RATE_NOCACHE | CLK_IS_CRITICAL, PLL_LOCKTIME_PLL_CAM_CSI, + PLL_CON0_PLL_CAM_CSI, pll_cam_csi_rate_table), }; PNAME(mout_cam_csi_pll_p) = { "fin_pll", "fout_pll_cam_csi" };
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> --- drivers/clk/samsung/clk-fsd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)