On Wed, Feb 19, 2025 at 05:08:52PM +0530, Jagadeesh Kona wrote: > > > On 2/18/2025 10:49 PM, Dmitry Baryshkov wrote: > > On Tue, Feb 18, 2025 at 03:46:15PM +0000, Bryan O'Donoghue wrote: > >> On 18/02/2025 14:26, Jagadeesh Kona wrote: > >>> During boot-up, the PLL configuration might be missed even after > >>> calling pll_configure() from the clock controller probe. This can > >>> happen because the PLL is connected to one or more rails that are > >>> turned off, and the current clock controller code cannot enable > >>> multiple rails during probe. Consequently, the PLL may be activated > >>> with suboptimal settings, causing functional issues. > >>> > >>> To properly configure the video PLLs in the probe on SM8450, SM8475, > >>> SM8550, and SM8650 platforms, the MXC rail must be ON along with MMCX. > >>> Therefore, add support to attach multiple power domains to videocc on > >>> these platforms. > >>> > >>> Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> > >>> --- > >>> drivers/clk/qcom/videocc-sm8450.c | 4 ++++ > >>> drivers/clk/qcom/videocc-sm8550.c | 4 ++++ > >>> 2 files changed, 8 insertions(+) > >>> > >>> diff --git a/drivers/clk/qcom/videocc-sm8450.c b/drivers/clk/qcom/videocc-sm8450.c > >>> index f26c7eccb62e7eb8dbd022e2f01fa496eb570b3f..b50a14547336580de88a741f1d33b126e9daa848 100644 > >>> --- a/drivers/clk/qcom/videocc-sm8450.c > >>> +++ b/drivers/clk/qcom/videocc-sm8450.c > >>> @@ -437,6 +437,10 @@ static int video_cc_sm8450_probe(struct platform_device *pdev) > >>> struct regmap *regmap; > >>> int ret; > >>> + ret = qcom_cc_attach_pds(&pdev->dev, &video_cc_sm8450_desc); > >>> + if (ret) > >>> + return ret; > >>> + > >>> ret = devm_pm_runtime_enable(&pdev->dev); > >>> if (ret) > >>> return ret; > >>> diff --git a/drivers/clk/qcom/videocc-sm8550.c b/drivers/clk/qcom/videocc-sm8550.c > >>> index 7c25a50cfa970dff55d701cb24bc3aa5924ca12d..d4b223d1392f0721afd1b582ed35d5061294079e 100644 > >>> --- a/drivers/clk/qcom/videocc-sm8550.c > >>> +++ b/drivers/clk/qcom/videocc-sm8550.c > >>> @@ -542,6 +542,10 @@ static int video_cc_sm8550_probe(struct platform_device *pdev) > >>> int ret; > >>> u32 sleep_clk_offset = 0x8140; > >>> + ret = qcom_cc_attach_pds(&pdev->dev, &video_cc_sm8550_desc); > >>> + if (ret) > >>> + return ret; > >>> + > >>> ret = devm_pm_runtime_enable(&pdev->dev); > >>> if (ret) > >>> return ret; > >>> > >> > >> What's the difference between doing the attach here or doing it in > >> really_probe() ? > > > > I'd second this. If the domains are to be attached before calling any > > other functions, move the call to the qcom_cc_map(), so that all drivers > > get all domains attached before configuring PLLs instead of manually > > calling the function. > > > > I earlier tried moving the attach PDs call to qcom_cc_map(), but I faced the below issues > 1. desc passed to qcom_cc_map() has const qualifier, so updating desc->pd_list > inside qcom_cc_map() is leading to a warning. And? Can you fix the warning? > 2. If we attach the PDs after calling get_sync() on device, I observed > that PDS are not getting enabled during probe. Currently qcom_cc_map() > is called after get_sync() is already called on device. Move PM handling into qcom_cc_map(). Then together with the Bryan's proposal most of the probe() functions can just call qcom_cc_probe() > > Probably, we can add a new function qcom_cc_attach_pds_map() where we can > attach PDs and call qcom_cc_map() inside it. We can then invoke this new > function at the start of probe before get_sync(). I will post this change > in next version if this aligns with your thoughts. > > Thanks, > Jagadeesh -- With best wishes Dmitry
