Hi,

Thanks for working on upstreaming PPP things :-)

On Thu Jun 19, 2025 at 7:21 AM CEST, Olivier Benjamin wrote:
> Fix a few issues in the panel section of the PinePhone Pro DTS:
>   - add the second part of the Himax HX8394 LCD panel controller
>     compatible
>   - as proposed by Diederik de Haas, reuse the mipi_out and ports
>     definitions from rk3399-base.dtsi instead of redefining them
>   - add a pinctrl for the LCD_RST signal for LCD1, derived from
>     LCD1_RST, which is on GPIO4_D1, as documented on pages 11
>     and 16 of the PinePhone Pro schematic
>
> Signed-off-by: Olivier Benjamin <olivier.benjamin@bootlin.com>
> ---
> Small fixes to the PinePhone Pro DTS to fit bindings and
> suppress warnings at build.
> ---
> Changes in v2:
> - Added the pinctrl definition for GPIO4_D1/LCD1_RST
> - Incorporated Diederik de Haas' suggestion for defining mipi_out
> - Squashed multiple patches into one
> - Link to v1: https://lore.kernel.org/r/20250618-dtb_fixes-v1-0-e54797ad2eba@bootlin.com
> ---
>  .../boot/dts/rockchip/rk3399-pinephone-pro.dts     | 33 +++++++++++-----------
>  1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> index 04ba4c4565d0a205e2e46d7535c6a3190993621d..98aba146749998dd5a798aabed0fe844c474d1cf 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-pinephone-pro.dts
> @@ -463,29 +463,18 @@ &io_domains {
>  };
>  
>  &mipi_dsi {
> -	status = "okay";
>  	clock-master;
> -
> -	ports {
> -		mipi_out: port@1 {
> -			#address-cells = <0>;
> -			#size-cells = <0>;
> -			reg = <1>;
> -
> -			mipi_out_panel: endpoint {
> -				remote-endpoint = <&mipi_in_panel>;
> -			};
> -		};
> -	};
> +	status = "okay";
>  
>  	panel@0 {
> -		compatible = "hannstar,hsd060bhw4";
> +		compatible = "hannstar,hsd060bhw4", "himax,hx8394";
>  		reg = <0>;
>  		backlight = <&backlight>;
> -		reset-gpios = <&gpio4 RK_PD1 GPIO_ACTIVE_LOW>;
> -		vcc-supply = <&vcc2v8_lcd>;
>  		iovcc-supply = <&vcc1v8_lcd>;
>  		pinctrl-names = "default";
> +		pinctrl-0 = <&lcd_reset_pin>;
> +		reset-gpios = <&gpio4 RK_PD1 GPIO_ACTIVE_LOW>;
> +		vcc-supply = <&vcc2v8_lcd>;
>  
>  		port {
>  			mipi_in_panel: endpoint {
> @@ -495,6 +484,12 @@ mipi_in_panel: endpoint {
>  	};
>  };
>  
> +&mipi_out {
> +	mipi_out_panel: endpoint {
> +		remote-endpoint = <&mipi_in_panel>;
> +	};
> +};
> +
>  &pmu_io_domains {
>  	pmu1830-supply = <&vcc_1v8>;
>  	status = "okay";
> @@ -507,6 +502,12 @@ pwrbtn_pin: pwrbtn-pin {
>  		};
>  	};
>  
> +	lcd {
> +		lcd_reset_pin: reset-pin {

I don't know if there's a 'hard rule' for it, but I'd recommend to use
``lcd1_rst_pin: lcd1-rst-pin {`` as that would match the naming from
the schematics. I realize that some but not all (other) pinctrl nodes
follow that 'rule', but it helps with traceability.

> +			rockchip,pins = <4 RK_PD1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};
> +
>  	leds {
>  		red_led_pin: red-led-pin {
>  			rockchip,pins = <4 RK_PD2 RK_FUNC_GPIO &pcfg_pull_none>;

Otherwise,

Reviewed-by: Diederik de Haas <didi.debian@cknow.org>

Cheers,
  Diederik