On 18.02.2025 4:46 PM, 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() ? > > There doesn't seem to be any difference except that we will have an additional delay introduced. > > Are you describing a race condition ? > > I don't see _logic_ here to moving the call into the controller's higher level probe. > > Can you describe some more ? I think this is a sane course of action: 1. associate pll config with the pll (treewide change) 2. add a generic pll_configure call that parses the type 3. store PLLs to be configured in an array of dt-bindings indices 4. move PLL configuration to really_probe 5. move RPM enablement to really_probe, make it conditional to limit resource waste on e.g. gcc 6. move attaching GDSCs to really_probe Konrad
