Hi Jinjie, (CC'ing Tomi) Thank you for the patch. On Fri, Aug 25, 2023 at 03:23:24PM +0800, Jinjie Ruan wrote: > The devm_clk_get_enabled() helper: > - calls devm_clk_get() > - calls clk_prepare_enable() and registers what is needed in order to > call clk_disable_unprepare() when needed, as a managed resource. > > This simplifies the code. While this indeed simplifies the code, I think we should instead control the clock dynamically at runtime. I don't have access to the hardware at the moment. Tomi, would you be able to give this a go ? I can also write a patch and let you test it if desired. > Signed-off-by: Jinjie Ruan <ruanjinjie@xxxxxxxxxx> > --- > drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c > index 88eb33acd5f0..92e61434473f 100644 > --- a/drivers/gpu/drm/xlnx/zynqmp_dpsub.c > +++ b/drivers/gpu/drm/xlnx/zynqmp_dpsub.c > @@ -92,16 +92,10 @@ unsigned int zynqmp_dpsub_get_audio_clk_rate(struct zynqmp_dpsub *dpsub) > > static int zynqmp_dpsub_init_clocks(struct zynqmp_dpsub *dpsub) > { > - int ret; > - > - dpsub->apb_clk = devm_clk_get(dpsub->dev, "dp_apb_clk"); > - if (IS_ERR(dpsub->apb_clk)) > - return PTR_ERR(dpsub->apb_clk); > - > - ret = clk_prepare_enable(dpsub->apb_clk); > - if (ret) { > + dpsub->apb_clk = devm_clk_get_enabled(dpsub->dev, "dp_apb_clk"); > + if (IS_ERR(dpsub->apb_clk)) { > dev_err(dpsub->dev, "failed to enable the APB clock\n"); > - return ret; > + return PTR_ERR(dpsub->apb_clk); > } > > /* > @@ -274,7 +268,6 @@ static int zynqmp_dpsub_probe(struct platform_device *pdev) > zynqmp_dp_remove(dpsub); > err_pm: > pm_runtime_disable(&pdev->dev); > - clk_disable_unprepare(dpsub->apb_clk); > err_mem: > of_reserved_mem_device_release(&pdev->dev); > if (!dpsub->drm) > @@ -295,7 +288,6 @@ static void zynqmp_dpsub_remove(struct platform_device *pdev) > zynqmp_dp_remove(dpsub); > > pm_runtime_disable(&pdev->dev); > - clk_disable_unprepare(dpsub->apb_clk); > of_reserved_mem_device_release(&pdev->dev); > > if (!dpsub->drm) -- Regards, Laurent Pinchart