linux-kernel - Re: [PATCH 3/9] clk: renesas: r8a779h0: Add display clocks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241203083738.GF10736@pendragon.ideasonboard.com>
Date: Tue, 3 Dec 2024 10:37:38 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Cc: Kieran Bingham <kieran.bingham+renesas@...asonboard.com>,
	Andrzej Hajda <andrzej.hajda@...el.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	Robert Foss <rfoss@...nel.org>, Jonas Karlman <jonas@...boo.se>,
	Jernej Skrabec <jernej.skrabec@...il.com>,
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
	Maxime Ripard <mripard@...nel.org>,
	Thomas Zimmermann <tzimmermann@...e.de>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Magnus Damm <magnus.damm@...il.com>,
	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...nel.org>, LUU HOAI <hoai.luu.ub@...esas.com>,
	Jagan Teki <jagan@...rulasolutions.com>,
	Sam Ravnborg <sam@...nborg.org>,
	Biju Das <biju.das.jz@...renesas.com>,
	dri-devel@...ts.freedesktop.org, linux-renesas-soc@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-clk@...r.kernel.org,
	Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
Subject: Re: [PATCH 3/9] clk: renesas: r8a779h0: Add display clocks

Hi Tomi,

Thank you for the patch.

On Tue, Dec 03, 2024 at 10:01:37AM +0200, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
> 
> Add display related clocks for DU, DSI, FCPVD, and VSPD.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas@...asonboard.com>
> ---
>  drivers/clk/renesas/r8a779h0-cpg-mssr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/clk/renesas/r8a779h0-cpg-mssr.c b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> index e20c048bfa9b..dc37e987c0e6 100644
> --- a/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> +++ b/drivers/clk/renesas/r8a779h0-cpg-mssr.c
> @@ -179,6 +179,9 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
>  	DEF_MOD("canfd0",	328,	R8A779H0_CLK_SASYNCPERD2),
>  	DEF_MOD("csi40",	331,	R8A779H0_CLK_CSI),
>  	DEF_MOD("csi41",	400,	R8A779H0_CLK_CSI),
> +	DEF_MOD("dis0",		411,	R8A779H0_CLK_S0D3),
> +	DEF_MOD("dsitxlink0",	415,	R8A779H0_CLK_DSIREF),
> +	DEF_MOD("fcpvd0",	508,	R8A779H0_CLK_S0D3),
>  	DEF_MOD("hscif0",	514,	R8A779H0_CLK_SASYNCPERD1),
>  	DEF_MOD("hscif1",	515,	R8A779H0_CLK_SASYNCPERD1),
>  	DEF_MOD("hscif2",	516,	R8A779H0_CLK_SASYNCPERD1),
> @@ -227,6 +230,7 @@ static const struct mssr_mod_clk r8a779h0_mod_clks[] __initconst = {
>  	DEF_MOD("vin15",	811,	R8A779H0_CLK_S0D4_VIO),
>  	DEF_MOD("vin16",	812,	R8A779H0_CLK_S0D4_VIO),
>  	DEF_MOD("vin17",	813,	R8A779H0_CLK_S0D4_VIO),
> +	DEF_MOD("vspd0",	830,	R8A779H0_CLK_S0D1_VIO),

The clock names and numbers are fine. The parents are not explicitly
listed in documentation, but the VIODBUSD1 clock description (table
8.1.4a) mentions FCPVD and VSPD as target modules. This is something
that should probably be double-checked. Quite interestingly, VIOBUSD2
also mentions the same target modules, hinting as a more complex clock
tree. A similar issue is perhaps present for "dis0" too, that's a DU
clock and the DU isn't listed as a target module of S0D3.

The way we model the "module stop" bits as clocks is clearly a limiting
factor as it can't represent real clock topologies. I don't however
don't expect it to cause any functional issue here, as the devices
related to the above clocks do not depend on the clock frequency.
There's no strict need to model the real hardware clock tree if it has
no impact on software, so

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>

I think it could be worth it check with Renesas what parent to use here,
and we can update the clock definitions later if needed.

>  	DEF_MOD("wdt1:wdt0",	907,	R8A779H0_CLK_R),
>  	DEF_MOD("cmt0",		910,	R8A779H0_CLK_R),
>  	DEF_MOD("cmt1",		911,	R8A779H0_CLK_R),
> 

-- 
Regards,

Laurent Pinchart