On 20/09/2024 06:15, Inbaraj E wrote: > > >> -----Original Message----- >> From: Inbaraj E <inbaraj.e@xxxxxxxxxxx> >> Sent: 20 September 2024 09:35 >> To: 'Krzysztof Kozlowski' <krzk@xxxxxxxxxx>; 'Stephen Boyd' >> <sboyd@xxxxxxxxxx>; 'alim.akhtar@xxxxxxxxxxx' >> <alim.akhtar@xxxxxxxxxxx>; 'cw00.choi@xxxxxxxxxxx' >> <cw00.choi@xxxxxxxxxxx>; 'linux-clk@xxxxxxxxxxxxxxx' <linux- >> clk@xxxxxxxxxxxxxxx>; 'linux-kernel@xxxxxxxxxxxxxxx' <linux- >> kernel@xxxxxxxxxxxxxxx>; 'linux-samsung-soc@xxxxxxxxxxxxxxx' <linux- >> samsung-soc@xxxxxxxxxxxxxxx>; 'mturquette@xxxxxxxxxxxx' >> <mturquette@xxxxxxxxxxxx>; 's.nawrocki@xxxxxxxxxxx' >> <s.nawrocki@xxxxxxxxxxx> >> Cc: 'pankaj.dubey@xxxxxxxxxxx' <pankaj.dubey@xxxxxxxxxxx>; >> 'gost.dev@xxxxxxxxxxx' <gost.dev@xxxxxxxxxxx> >> Subject: RE: [PATCH] clk: samsung: fsd: Mark PLL_CAM_CSI as critical >> >> >> >>> -----Original Message----- >>> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx> >>> Sent: 19 September 2024 17:33 >>> To: Inbaraj E <inbaraj.e@xxxxxxxxxxx>; 'Stephen Boyd' >>> <sboyd@xxxxxxxxxx>; alim.akhtar@xxxxxxxxxxx; >> cw00.choi@xxxxxxxxxxx; >>> linux-clk@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >>> linux-samsung- soc@xxxxxxxxxxxxxxx; mturquette@xxxxxxxxxxxx; >>> s.nawrocki@xxxxxxxxxxx >>> Cc: pankaj.dubey@xxxxxxxxxxx; gost.dev@xxxxxxxxxxx >>> 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@xxxxxxxxxx> >>>>> Sent: 19 September 2024 15:51 >>>>> To: Inbaraj E <inbaraj.e@xxxxxxxxxxx>; alim.akhtar@xxxxxxxxxxx; >>>>> cw00.choi@xxxxxxxxxxx; krzk@xxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx; >>>>> linux- kernel@xxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; >>>>> mturquette@xxxxxxxxxxxx; s.nawrocki@xxxxxxxxxxx >>>>> Cc: pankaj.dubey@xxxxxxxxxxx; gost.dev@xxxxxxxxxxx; Inbaraj E >>>>> <inbaraj.e@xxxxxxxxxxx> >>>>> 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@xxxxxxxxxxx> >>>>>> --- >>>>> >>>>> 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. >> > > This is clock hierarchy for CMU_CAM_CSI block. > > PLL_CAM_CSI -----> DIVIDER --------> CSI_SFR clock > | > |----> DIVIDER --------> CMU_SFR clock > | > |----> DIVIDER --------> CSI NOC clock. > And what is the problem in adding proper handling in the driver? You just described case valid for 99% of SoC components. Best regards, Krzysztof
