| Message ID | 20250228165755.405138-2-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/rockchip: lvds: probe logging improvements | expand |
Hi Heiko, On 2/28/25 5:57 PM, Heiko Stuebner wrote: > From: Heiko Stuebner <heiko.stuebner@cherry.de> > > The LVDS block needs a separate pclk only on some socs, so currently > requests and prepares it in the soc-specific probe function, but common > code is required to unprepare it in the error path or on driver remove. > > While this works because clk_unprepare just does nothing if clk is NULL, > this mismatch of who is responsible still is not very nice. > The clock-framework already has a helper for clk-get-and-prepare even > with devres support in devm_clk_get_prepared(). > > This will get and prepare the clock and also unprepare it on driver > removal, saving the driver from having to handle it "manually". > > Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de> Reviewed-by: Quentin Schulz <quentin.schulz@cherry.de> Small nitpick below. > --- > drivers/gpu/drm/rockchip/rockchip_lvds.c | 17 ++--------------- > 1 file changed, 2 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c > index 385cf6881504..c19b7b1f6cb5 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c > +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c > @@ -448,13 +448,11 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = { > static int rk3288_lvds_probe(struct platform_device *pdev, > struct rockchip_lvds *lvds) > { > - int ret; > - > lvds->regs = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(lvds->regs)) > return PTR_ERR(lvds->regs); > > - lvds->pclk = devm_clk_get(lvds->dev, "pclk_lvds"); > + lvds->pclk = devm_clk_get_prepared(lvds->dev, "pclk_lvds"); > if (IS_ERR(lvds->pclk)) { > DRM_DEV_ERROR(lvds->dev, "could not get pclk_lvds\n"); Should we say Could not get or prepare pclk_lvds\n instead? Thanks! Quentin
diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c index 385cf6881504..c19b7b1f6cb5 100644 --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c @@ -448,13 +448,11 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = { static int rk3288_lvds_probe(struct platform_device *pdev, struct rockchip_lvds *lvds) { - int ret; - lvds->regs = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(lvds->regs)) return PTR_ERR(lvds->regs); - lvds->pclk = devm_clk_get(lvds->dev, "pclk_lvds"); + lvds->pclk = devm_clk_get_prepared(lvds->dev, "pclk_lvds"); if (IS_ERR(lvds->pclk)) { DRM_DEV_ERROR(lvds->dev, "could not get pclk_lvds\n"); return PTR_ERR(lvds->pclk); @@ -480,12 +478,6 @@ static int rk3288_lvds_probe(struct platform_device *pdev, } } - ret = clk_prepare(lvds->pclk); - if (ret < 0) { - DRM_DEV_ERROR(lvds->dev, "failed to prepare pclk_lvds\n"); - return ret; - } - return 0; } @@ -728,20 +720,15 @@ static int rockchip_lvds_probe(struct platform_device *pdev) dev_set_drvdata(dev, lvds); ret = component_add(&pdev->dev, &rockchip_lvds_component_ops); - if (ret < 0) { + if (ret < 0) DRM_DEV_ERROR(dev, "failed to add component\n"); - clk_unprepare(lvds->pclk); - } return ret; } static void rockchip_lvds_remove(struct platform_device *pdev) { - struct rockchip_lvds *lvds = platform_get_drvdata(pdev); - component_del(&pdev->dev, &rockchip_lvds_component_ops); - clk_unprepare(lvds->pclk); } struct platform_driver rockchip_lvds_driver = {