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.xm@xxxxxxxxxxx>
>
> 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.xm@xxxxxxxxxxx>
> Signed-off-by: LUU HOAI <hoai.luu.ub@xxxxxxxxxxx>
> [tomi.valkeinen: Rewrite the patch description]
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@xxxxxxxxxxxxxxxx>
> ---
> 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
