linux-kernel - Re: [PATCH v2 1/4] arm64: dts: rockchip: Add PD to csi dphy node on rk356x

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: <D4QNJ85V43NU.YD01E8AB4116@cknow.org>
Date: Tue, 08 Oct 2024 20:55:34 +0200
From: "Diederik de Haas" <didi.debian@...ow.org>
To: "Michael Riesch" <michael.riesch@...fvision.net>, "Rob Herring"
 <robh@...nel.org>, "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor
 Dooley" <conor+dt@...nel.org>, "Heiko Stuebner" <heiko@...ech.de>
Cc: "Dragan Simic" <dsimic@...jaro.org>, "Samuel Holland"
 <samuel@...lland.org>, <devicetree@...r.kernel.org>,
 <linux-arm-kernel@...ts.infradead.org>,
 <linux-rockchip@...ts.infradead.org>, <linux-kernel@...r.kernel.org>,
 "Diederik de Haas" <didi.debian@...ow.org>
Subject: Re: [PATCH v2 1/4] arm64: dts: rockchip: Add PD to csi dphy node on
 rk356x

Hi Michael,

On Tue Oct 8, 2024 at 2:32 PM CEST, Michael Riesch wrote:
> On 10/8/24 13:15, Diederik de Haas wrote:
> > The "rockchip-inno-csi-dphy.yaml" binding requires the power-domains
> > property. According to RK3568 TRM Part 1 section 7.3 (page 475) the
> > CSIHOST is placed in the PD_VI power domain.
> > So set the csi_dphy node power-domains property accordingly.
>
> Thanks for the patch. However, I am not sure about this one.

Thanks for your reply.

> The CSI host sure is in this power domain, but we are talking about the
> CSI PHY here, right? According to the table CSIPHY is part of the power
> domain "ALIVE", which leads me to believe that the power domain is not
> necessary here. However, I guess you could put "RK3568_PD_LOGIC_ALIVE" here.
>
> It should be noted, though, that I still haven't figured out what the
> role of this CSI host actually is. I know that the RK3568 ISP has its
> own MIPI CSI host controller within its register space. But I can only
> guess right now that this CSI host is somehow linked to the RK3568
> VICAP, which is also capable of receiving MIPI CSI. Maybe we can leave
> this up to however brings up the RK3568 VICAP MIPI CSI feature :-)

It indeed isn't as clear cut as I want(ed) it to be.

Given that you're also the author of commit 29c99fb085ad ("phy:
rockchip: add support for the rk356x variant to rockchip-inno-csidphy"),
which added support to the driver part, your opinion should have more
weight then mine (IMO).

Nonetheless, I collected some extra data points:
- The TRM part 1 makes several mentions of 'dphy' in a block describing
  'GRF_VI_CON1' (page 381 & 382). The above mentioned commit only added
  'PHY_REG' for 'CON0', which I assume was deliberate given your
  response above. But 'GRF_VI_CON1' does have 'VI' in its name
- In rk356x.dtsi there are 'dsi_dphy0' and 'dsi_dphy1' which have
  'RK3568_PD_VO' in their 'power-domains' property. Page 475 has
  'DSIHOST' in 'PD_VO', while 'DSIPHY' is (also) in the 'ALIVE' power
  domain. So to be consistent it seems to me that 'csi_dphy' should be
  in 'PD_VI' or the 'dsi_dphy' nodes should be placed/moved to
  'RK3568_PD_LOGIC_ALIVE'.
- Of all the compatibles from the binding, I only found
  'rockchip,px30-csi-dphy' referenced in DT files (next to
  'rockchip,rk3568-csi-dphy' in rk356x.dtsi) and in px30.dtsi the
  'csi_dphy' node has 'PX30_PD_VI' as value for its 'power-domains'
  property.

But this is all 'circumstantial'; it would be nice to have a clear
answer (from Rockchip?).

Cheers,
  Diederik

> > Fixes: b6c228401b25 ("arm64: dts: rockchip: add csi dphy node to rk356x")
> > Signed-off-by: Diederik de Haas <didi.debian@...ow.org>
> > ---
> > changes in v2:
> > - No change
> > 
> >  arch/arm64/boot/dts/rockchip/rk356x.dtsi | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > index 0ee0ada6f0ab..d581170914f9 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> > @@ -1790,6 +1790,7 @@ csi_dphy: phy@...70000 {
> >  		clocks = <&cru PCLK_MIPICSIPHY>;
> >  		clock-names = "pclk";
> >  		#phy-cells = <0>;
> > +		power-domains = <&power RK3568_PD_VI>;
> >  		resets = <&cru SRST_P_MIPICSIPHY>;
> >  		reset-names = "apb";
> >  		rockchip,grf = <&grf>;


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)