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@xxxxxxxxx>
> > ---
> > 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@fe870000 {
> > 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>;
Attachment:
signature.asc
Description: PGP signature
