[PATCH 2/6] drm: rcar-du: lvds: Add reset control

[Tomi Valkeinen]

On 18/01/2023 23:06, Laurent Pinchart wrote:
> 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 at renesas.com>
>>
>> Reset LVDS using the reset control as CPG reset/release is required in
>> H/W manual sequence.
> 
> s at H/W at the hardware@
> 
>> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm at renesas.com>
>> Signed-off-by: LUU HOAI <hoai.luu.ub at renesas.com>
>> [tomi.valkeinen: Rewrite the patch description]
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas at 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().

Yes. Too hasty in picking a patch from the BSP =).

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

I have the runtime PM patch in my work branch, but on top. I'll pick 
that into this series, before adding the reset control. Makes error 
handling a bit simpler.

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

Yep.

  Tomi