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
next prev parent 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®