mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Geert Uytterhoeven <geert@linux-m68k.org>
To: laurent.pinchart@ideasonboard.com
Cc: bmasney+clk@redhat.com, conor+dt@kernel.org,
	devicetree@vger.kernel.org, geert+renesas@glider.be,
	jbrunet+clk@baylibre.com,
	kieran.bingham+renesas@ideasonboard.com, krzk+dt@kernel.org,
	linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-media@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org, magnus.damm@gmail.com,
	mchehab@kernel.org, p.zabel@pengutronix.de,
	paul.elder+renesas@ideasonboard.com, robh@kernel.org,
	sboyd@kernel.org, ulfh@kernel.org
Subject: Re: [PATCH DNI v3 04/15] arm64: renesas: r8a78000: Add FCPV[BI] nodes
Date: Thu,  5 Nov 2026 14:05:21 +0100	[thread overview]
Message-ID: <20261105130521.85109-1-geert@linux-m68k.org> (raw)
In-Reply-To: <20261030211225.GA3144851@killaraus.ideasonboard.com>

	Hi Laurent,

On Fri, 30 Oct 2026 at 23:12:25 +0200, Laurent Pinchart wrote:
> On Thu, Oct 15, 2026 at 04:51:44PM +0900, Paul Elder wrote:
> > Add DT nodes for the FCPVB and FCPVI IP cores in the R-Car X5H SoC.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > Signed-off-by: Paul Elder <paul.elder+renesas@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> I need to reconsider this...
> 
> > Tested-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> >  arch/arm64/boot/dts/renesas/r8a78000.dtsi | 72 +++++++++++++++++++++++++++++++
> >  1 file changed, 72 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/renesas/r8a78000.dtsi b/arch/arm64/boot/dts/renesas/r8a78000.dtsi
> > index 9a07753b221541ed49a975e0c95f2f5d1fdc6ac2..2f8886f8b6545026397fd83969882f8c62afa9d3 100644
> > --- a/arch/arm64/boot/dts/renesas/r8a78000.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r8a78000.dtsi
> > @@ -1000,6 +1000,78 @@ mdlc_aon: system-controller@c1338000 {
> >  			bootph-all;
> >  		};
> >  
> > +		fcpvi0: fcp@c5030000 {
> > +			compatible = "renesas,fcpv";
> > +			reg = <0 0xc5030000 0 0x1000>;
> > +			clocks = <&cpg R8A78000_CPG_SGD4_VIO_DP_TX>;
> 
> ... as this may not be the correct clock.
> 
> The user manual lists the following SGD clocks for the media-related IP
> cores:
> 
> - CSI[0123]:		SGD1_VIO_CSI[0123]φ, SGD2_VIO_CSI[0123]φ,
> 			SGD4_VIO_CSI[0123]φ, SGD16_VIO_CSI[0123]φ
> 
> - VIN[012]:		SGD1_VIO_CSI0φ, SGD4_VIO_CSI0φ
> - VIN[345]:		SGD1_VIO_CSI1φ, SGD4_VIO_CSI1φ
> - VIN[678]:		SGD1_VIO_CSI2φ, SGD4_VIO_CSI2φ
> - VIN{9,10,11}:		SGD1_VIO_CSI3φ, SGD4_VIO_CSI3φ
> 
> - ISP CS[0123]:		SGD1_VIO_CSI[0123]φ, SGD2_VIO_CSI[0123]φ
> - ISP Core[0123]:	SGD1_VIO_ISP[0123]φ, SGD2_VIO_ISP[0123]φ
> 
> - FCPV[BI][0-9]:	SGD1_VIO_DP_TXφ, SGD4_VIO_DP_TXφ
> - FCPVD[ab][0-9]:	SGD1_VIO_DP_TXφ, SGD4_VIO_DP_TXφ
> - FCPVX[0123]:		SGD1_VIO_ISP[0123]φ, SGD4_VIO_ISP[0123]φ
> 
> - VSP[BI][0-9]:		SGD1_VIO_DP_OTHERφ, SGD4_VIO_DP_OTHERφ
> - VSPD[ab][0-9]:	SGD1_VIO_DP_TXφ, SGD4_VIO_DP_TXφ
> - VSPX[0123]:		SGD1_VIO_ISP[0123]φ, SGD4_VIO_ISP[0123]φ
> 
> - VCON[0123]:		SGD1_VIO_DP_OTHER0φ, SGD4_VIO_DP_OTHER0φ
> - VCON[4567]:		SGD1_VIO_DP_OTHER1φ, SGD4_VIO_DP_OTHER2φ
> - VCON[89]:		SGD1_VIO_DP_OTHER2φ, SGD4_VIO_DP_OTHER2φ
> 
> - DPTX[012]:		SGD2_VIO_DP_TXφ, SGD16_VIO_DP_TXφ
> 
> Those IP cores are connected in four types of pipelines:
> 
> - FCPV[BI] -> VSP[BI]
> - FCPVD -> VSPD -> VCON -> DPTX
> - FCPVX -> VSPX -> ISP Core
> - CSI -> VIN -> ISP CS
> 
> It would be quite logical for all IP cores in the same pipeline to use
> the same clock. This would be
> 
> - FCPV[BI] -> VSP[BI]:			SGD*_VIO_DP_OTHERφ
> - FCPVD -> VSPD -> VCON -> DPTX:	SGD*_VIO_DP_TXφ
> - FCPVX -> VSPX -> ISP Core:		SGD*_VIO_ISP[0123]φ
> - CSI -> VIN -> ISP CS:			SGD*_VIO_CSI[0123]φ
> 
> This mostly matches the clocks listed in the user manual, except for
> 
> - FCPV[BI] that is listed as using SGD*_VIO_DP_TXφ
> - VCON that is listed as using SGD*_VIO_DP_OTHER[012]φ
> 
> Now, note how in the clock diagram document the VIO "ISP" and "CSI"
> hierarchies both show 4 groups of SGD clocks (respectively named
> SGD*_VIO_ISP0 and SGD*_VIO_CSI0 in the first group), the VIO "DP"
> hierarchies show 3 groups of SGD clocks (named SGD*_VIO_DP0 in the first
> group) and the VIO "DP_OTHER" hierarchy shows a single group of SGD
> clocks (named SGD*_VIO_DP_OTHER).
> 
> This makes me believe that, wherever the user manual lists
> SGD*_VIO_DP_TXφ, it actually means SGD*_VIO_DP[012]φ. Similarly, where
> the user manual lists SGD*_VIO_DP_OTHER[012]φ for VCON, I believe it
> should be SGD*_VIO_DP[012]φ, both because there is a single
> SGD*_VIO_DP_OTHERφ, and because the rest of the VCON pipelines (FCPVD,
> VSPD and DPTX) all use SGD*_VIO_DP[012]φ.

The clock diagram has 3 VIO_DP[012] hierarchies and a single
VIO_DP_OTHER, while the clock list has 3 VIO_DP_OTHER[012] hierarchies
and a single VIO_DP_TX.
The clock diagram has VIO_MAIN (system bus) clocks, while the clock list
has VIO_OTHER and VIO_BUS clocks.

> For full consistency, I also suppose that the FCPV[BI][0-9] instances
> match the corresponding VSP[BI][0-9] and use the SGD*_VIO_DP_OTHERφ
> clocks, not the listed SGD*_VIO_DP_TXφ clocks.
> 
> This being said, in practice this probably makes no difference, as all
> those clocks derive from the same SGD1_VIO_MAIN clock and none of them
> are programmable or gateable.

Probably, but it would be good to have that confirmed.

> If the above explanation makes sense to you (and to Geert), we can ask
> Renesas to check if the issue I suspect are errors in the documentation.

Agreed.

Current SCMI firmware does not have the CSI3 group.  It does have
VIO_IM[012], which was replaced by DP_OTHER{012] according to the
"Differences" sheet in the clock list.

I guess we really need to know if e.g. all these SGD* clocks are the
same, and if they can be gated individually.  Same for the various DIV6
clocks, as there is only a single register to control them (which
probably controls the parent/singleton in reality).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

  reply	other threads:[~2026-11-05 13:05 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  7:51 [PATCH DNI v3 00/15] media: rcar-fcp, vsp1: Enable on R-Car X5H Paul Elder
2026-10-15  7:51 ` [PATCH DNI v3 01/15] dt-bindings: clock: renesas,r8a78000-cpg: Add CPG_SGD4_VIO_DP_{TX,OTHER} Paul Elder
2026-10-30 21:57   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH DNI v3 02/15] clk: renesas: r8a78000: Add clock for FCPV and VSP for SCP FW SDKv4.36 Paul Elder
2026-10-15  7:51 ` [PATCH DNI v3 03/15] pmdomain: renesas: r8a78000: Add maps for VIO " Paul Elder
2026-10-30 22:05   ` Laurent Pinchart
2026-11-02  9:07     ` Geert Uytterhoeven
2026-10-15  7:51 ` [PATCH DNI v3 04/15] arm64: renesas: r8a78000: Add FCPV[BI] nodes Paul Elder
2026-10-30 21:12   ` Laurent Pinchart
2026-11-05 13:05     ` Geert Uytterhoeven [this message]
2026-10-15  7:51 ` [PATCH DNI v3 05/15] arm64: renesas: r8a78000: Add VSP[BI] nodes Paul Elder
2026-10-30 21:38   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH DNI v3 06/15] arm64: renesas: r8a78000: Add FCPVD and VSPD nodes Paul Elder
2026-10-30 21:42   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH v3 07/15] media: vsp1: Support full VSPB on R-Car M3-W, M3-N and E3 Paul Elder
2026-10-15  7:51 ` [PATCH v3 08/15] media: vsp1: Add device info entries for VSPD on X5H Paul Elder
2026-10-15  7:51 ` [PATCH v3 09/15] media: rcar-fcp: Handle resets Paul Elder
2026-11-05 14:06   ` Philipp Zabel
2026-10-15  7:51 ` [PATCH v3 10/15] media: vsp1: Fix access to reserved registers Paul Elder
2026-10-30 21:34   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH v3 11/15] media: vsp1: Fix du_complete race condition issue Paul Elder
2026-10-15  7:51 ` [PATCH v3 12/15] media: vsp1: Fix setting control register by display list Paul Elder
2026-10-30 21:51   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH v3 13/15] dt-bindings: Increase Renesas X5H MDLC PD specifier bit width Paul Elder
2026-10-17 16:36   ` Krzysztof Kozlowski
2026-10-30 21:28   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH v3 14/15] media: vsp1: Complete buffers as queued in start_streaming error path Paul Elder
2026-10-30 21:53   ` Laurent Pinchart
2026-10-15  7:51 ` [PATCH DNI v3 15/15] media: rcar-fcp: Check device revision at probe time Paul Elder
2026-10-20 12:20   ` Geert Uytterhoeven
2026-10-30 22:02     ` Laurent Pinchart
2026-10-20 13:22   ` Geert Uytterhoeven

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261105130521.85109-1-geert@linux-m68k.org \
    --to=geert@linux-m68k.org \
    --cc=bmasney+clk@redhat.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=geert+renesas@glider.be \
    --cc=jbrunet+clk@baylibre.com \
    --cc=kieran.bingham+renesas@ideasonboard.com \
    --cc=krzk+dt@kernel.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=magnus.damm@gmail.com \
    --cc=mchehab@kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=paul.elder+renesas@ideasonboard.com \
    --cc=robh@kernel.org \
    --cc=sboyd@kernel.org \
    --cc=ulfh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®