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. 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. 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
