Hi Chris,
Thanks for your efforts, nice work!
A few minor comments below:
On 9/12/22 22:56, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@xxxxxxxxxxx>
>
> This adds the DSI controller nodes and DSI-DPHY controller nodes to the
> rk356x device tree.
>
> Signed-off-by: Chris Morgan <macromorgan@xxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/rockchip/rk356x.dtsi | 72 ++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk356x.dtsi b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> index 319981c3e9f7..d150568fde82 100644
> --- a/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk356x.dtsi
> @@ -699,6 +699,54 @@ vop_mmu: iommu@fe043e00 {
> status = "disabled";
> };
>
> + dsi0: dsi@fe060000 {
> + compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
> + reg = <0x00 0xfe060000 0x00 0x10000>;
> + interrupts = <GIC_SPI 68 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "pclk", "hclk";
> + clocks = <&cru PCLK_DSITX_0>, <&cru HCLK_VO>;
> + phy-names = "dphy";
> + phys = <&mipi_dphy0>;
> + power-domains = <&power RK3568_PD_VO>;
> + reset-names = "apb";
> + resets = <&cru SRST_P_DSITX_0>;
> + rockchip,grf = <&grf>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
We'll have to reference that port in the board dts, right? A label would
be helpful, e.g., dsi0_in.
> + reg = <0>;
> + };
Would it make sense to add the dsi0_out port at this point?
dsi0_out: port@1 {
reg = <1>;
};
> + };
> + };
> +
> + dsi1: dsi@fe070000 {
> + compatible = "rockchip,rk3568-mipi-dsi", "snps,dw-mipi-dsi";
> + reg = <0x0 0xfe070000 0x0 0x10000>;
> + interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
> + clock-names = "pclk", "hclk";
> + clocks = <&cru PCLK_DSITX_1>, <&cru HCLK_VO>;
> + phy-names = "dphy";
> + phys = <&mipi_dphy1>;
> + power-domains = <&power RK3568_PD_VO>;
> + reset-names = "apb";
> + resets = <&cru SRST_P_DSITX_1>;
> + rockchip,grf = <&grf>;
> + status = "disabled";
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
Ditto.
> + reg = <0>;
> + };
> + };
> + };
> +
> hdmi: hdmi@fe0a0000 {
> compatible = "rockchip,rk3568-dw-hdmi";
> reg = <0x0 0xfe0a0000 0x0 0x20000>;
> @@ -1594,6 +1642,30 @@ combphy2: phy@fe840000 {
> status = "disabled";
> };
>
> + mipi_dphy0: mipi-dphy@fe850000 {
May I suggest to call this one "dsi_dphy0" (analogous to "csi_dphy")?
> + compatible = "rockchip,rk3568-dsi-dphy";
> + reg = <0x0 0xfe850000 0x0 0x10000>;
> + clock-names = "ref", "pclk";
> + clocks = <&pmucru CLK_MIPIDSIPHY0_REF>, <&cru PCLK_MIPIDSIPHY0>;
> + #phy-cells = <0>;
> + power-domains = <&power RK3568_PD_VO>;
> + reset-names = "apb";
> + resets = <&cru SRST_P_MIPIDSIPHY0>;
> + status = "disabled";
> + };
> +
> + mipi_dphy1: mipi-dphy@fe860000 {
Ditto (well, "dsi_dphy1" obviously).
Best regards,
Michael
> + compatible = "rockchip,rk3568-dsi-dphy";
> + reg = <0x0 0xfe860000 0x0 0x10000>;
> + clock-names = "ref", "pclk";
> + clocks = <&pmucru CLK_MIPIDSIPHY1_REF>, <&cru PCLK_MIPIDSIPHY1>;
> + #phy-cells = <0>;
> + power-domains = <&power RK3568_PD_VO>;
> + reset-names = "apb";
> + resets = <&cru SRST_P_MIPIDSIPHY1>;
> + status = "disabled";
> + };
> +
> usb2phy0: usb2phy@fe8a0000 {
> compatible = "rockchip,rk3568-usb2phy";
> reg = <0x0 0xfe8a0000 0x0 0x10000>;
