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

[Michael Riesch]

Hi Diederik,

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.

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 :-)

Best regards,
Michael

> 
> 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>;