Hi Tomi,

Thank you for the patch.

On Tue, Jan 17, 2023 at 03:51:50PM +0200, Tomi Valkeinen wrote:
> From: Koji Matsuoka <koji.matsuoka...@renesas.com>
> 
> Reset LVDS using the reset control as CPG reset/release is required in
> H/W manual sequence.

s@H/W@the hardware@

> Signed-off-by: Koji Matsuoka <koji.matsuoka...@renesas.com>
> Signed-off-by: LUU HOAI <hoai.luu...@renesas.com>
> [tomi.valkeinen: Rewrite the patch description]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+rene...@ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/Kconfig     |  1 +
>  drivers/gpu/drm/rcar-du/rcar_lvds.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index a8f862c68b4f..151e400b996d 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -43,6 +43,7 @@ config DRM_RCAR_LVDS
>       select DRM_PANEL
>       select OF_FLATTREE
>       select OF_OVERLAY
> +     select RESET_CONTROLLER
>  
>  config DRM_RCAR_USE_MIPI_DSI
>       bool "R-Car DU MIPI DSI Encoder Support"
> diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
> b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> index 81a060c2fe3f..674b727cdaa2 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
> @@ -16,6 +16,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of_graph.h>
>  #include <linux/platform_device.h>
> +#include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/sys_soc.h>
>  
> @@ -60,6 +61,7 @@ struct rcar_lvds_device_info {
>  struct rcar_lvds {
>       struct device *dev;
>       const struct rcar_lvds_device_info *info;
> +     struct reset_control *rstc;
>  
>       struct drm_bridge bridge;
>  
> @@ -316,6 +318,8 @@ int rcar_lvds_pclk_enable(struct drm_bridge *bridge, 
> unsigned long freq)
>  
>       dev_dbg(lvds->dev, "enabling LVDS PLL, freq=%luHz\n", freq);
>  
> +     reset_control_deassert(lvds->rstc);

Can this fail ? Same in __rcar_lvds_atomic_enable().

> +
>       ret = clk_prepare_enable(lvds->clocks.mod);

It would be nice to switch this driver to runtime PM and move reset and
clock handling to the suspend/resume handlers. A candidate for a future
patch.

>       if (ret < 0)

Missing reset_control_assert(). Same in other error paths if applicable,
here and in __rcar_lvds_atomic_enable().

>               return ret;
> @@ -338,6 +342,8 @@ void rcar_lvds_pclk_disable(struct drm_bridge *bridge)
>       rcar_lvds_write(lvds, LVDPLLCR, 0);
>  
>       clk_disable_unprepare(lvds->clocks.mod);
> +
> +     reset_control_assert(lvds->rstc);
>  }
>  EXPORT_SYMBOL_GPL(rcar_lvds_pclk_disable);
>  
> @@ -396,6 +402,8 @@ static void __rcar_lvds_atomic_enable(struct drm_bridge 
> *bridge,
>       u32 lvdcr0;
>       int ret;
>  
> +     reset_control_deassert(lvds->rstc);
> +
>       ret = clk_prepare_enable(lvds->clocks.mod);
>       if (ret < 0)
>               return;
> @@ -552,6 +560,7 @@ static void rcar_lvds_atomic_disable(struct drm_bridge 
> *bridge,
>                                                      old_bridge_state);
>  
>       clk_disable_unprepare(lvds->clocks.mod);
> +     reset_control_assert(lvds->rstc);
>  }
>  
>  static bool rcar_lvds_mode_fixup(struct drm_bridge *bridge,
> @@ -844,6 +853,12 @@ static int rcar_lvds_probe(struct platform_device *pdev)
>       if (ret < 0)
>               return ret;
>  
> +     lvds->rstc = devm_reset_control_get(&pdev->dev, NULL);
> +     if (IS_ERR(lvds->rstc)) {
> +             dev_err(&pdev->dev, "failed to get cpg reset\n");
> +             return PTR_ERR(lvds->rstc);
> +     }
> +
>       drm_bridge_add(&lvds->bridge);
>  
>       return 0;

-- 
Regards,

Laurent Pinchart

Reply via email to