Hi Dmitry,
Am Donnerstag, 6. März 2025, 08:28:40 MEZ schrieb Dmitry Osipenko:
> From: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
>
> Add device tree support for Synopsys DesignWare HDMI RX
> Controller.
>
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> Tested-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> Co-developed-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
> Signed-off-by: Dingxian Wen <shawn.wen@xxxxxxxxxxxxxx>
> Signed-off-by: Shreeya Patel <shreeya.patel@xxxxxxxxxxxxx>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@xxxxxxxxxxxxx>
> ---
> .../dts/rockchip/rk3588-base-pinctrl.dtsi | 14 +++++
> .../arm64/boot/dts/rockchip/rk3588-extra.dtsi | 57 +++++++++++++++++++
> 2 files changed, 71 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> index 7f874c77410c..2d4b9986a177 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-base-pinctrl.dtsi
> @@ -594,6 +594,20 @@ hdmim0_tx1_hpd: hdmim0-tx1-hpd {
> /* hdmim0_tx1_hpd */
> <1 RK_PA6 5 &pcfg_pull_none>;
> };
> +
> + /omit-if-no-ref/
> + hdmim1_rx: hdmim1-rx {
> + rockchip,pins =
> + /* hdmim1_rx_cec */
> + <3 RK_PD1 5 &pcfg_pull_none>,
> + /* hdmim1_rx_scl */
> + <3 RK_PD2 5 &pcfg_pull_none_smt>,
> + /* hdmim1_rx_sda */
> + <3 RK_PD3 5 &pcfg_pull_none_smt>,
> + /* hdmim1_rx_hpdin */
> + <3 RK_PD4 5 &pcfg_pull_none>;
> + };
> +
what's the reason for duplicating these pinctrl entries?
The base-pinctrl already contains a separate set of pins (and also a
variant for the m0 set of pins), so why not check and use the already
existing ones:
hdmim1_rx_cec: hdmim1-rx-cec {
rockchip,pins =
/* hdmim1_rx_cec */
<3 RK_PD1 5 &pcfg_pull_none>;
};
hdmim1_rx_hpdin: hdmim1-rx-hpdin {
rockchip,pins =
/* hdmim1_rx_hpdin */
<3 RK_PD4 5 &pcfg_pull_none>;
};
hdmim1_rx_scl: hdmim1-rx-scl {
rockchip,pins =
/* hdmim1_rx_scl */
<3 RK_PD2 5 &pcfg_pull_none>;
};
hdmim1_rx_sda: hdmim1-rx-sda {
rockchip,pins =
/* hdmim1_rx_sda */
<3 RK_PD3 5 &pcfg_pull_none>;
};
Having multiple sets (even with differeing settings) will cause confusion
later on.
> /omit-if-no-ref/
> hdmim1_rx_cec: hdmim1-rx-cec {
> rockchip,pins =
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> index 4a950907ea6f..b7d06f93c8ce 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-extra.dtsi
> @@ -135,6 +159,39 @@ i2s10_8ch: i2s@fde00000 {
> status = "disabled";
> };
>
> + hdmi_receiver: hdmi_receiver@fdee0000 {
> + compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> + reg = <0x0 0xfdee0000 0x0 0x6000>;
> + power-domains = <&power RK3588_PD_VO1>;
> + rockchip,grf = <&sys_grf>;
> + rockchip,vo1-grf = <&vo1_grf>;
> + interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
> + interrupt-names = "cec", "hdmi", "dma";
> + clocks = <&cru ACLK_HDMIRX>,
> + <&cru CLK_HDMIRX_AUD>,
> + <&cru CLK_CR_PARA>,
> + <&cru PCLK_HDMIRX>,
> + <&cru CLK_HDMIRX_REF>,
> + <&cru PCLK_S_HDMIRX>,
> + <&cru HCLK_VO1>;
> + clock-names = "aclk",
> + "audio",
> + "cr_para",
> + "pclk",
> + "ref",
> + "hclk_s_hdmirx",
> + "hclk_vo1";
> + resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
> + <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
> + reset-names = "axi", "apb", "ref", "biu";
> + memory-region = <&hdmi_receiver_cma>;
> + pinctrl-0 = <&hdmim1_rx>;
> + pinctrl-names = "default";
hmm, this might be better living in the board dts?
Heiko
