[PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[Olivier Benjamin]

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 {
+			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>;
-- 
Olivier Benjamin <olivier.benjamin@bootlin.com>

Re: [PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 3322 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[Heiko Stuebner]

Am Donnerstag, 19. Juni 2025, 11:46:15 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
> 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>

> > +	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.

not a "hard" rule, but a strong preference.
I.e. we want people to ideally be able to just hit search in the
schematics PDFs for the name they saw in the devicetree.

Sometimes that is not possible, because of naming conventions or
overly strange schematic-names ... and sometimes things slip through
as well.

But following the schematic names, is the general goal.


If this stays the only suggestion though, I can fix that when
applying. Or you can send a v3 - up to you :-)


Heiko


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

Re: [PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[Olivier Benjamin]

On 6/19/25 12:31, Heiko Stuebner wrote:
> Am Donnerstag, 19. Juni 2025, 11:46:15 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
>> Hi,
>>
>> Thanks for working on upstreaming PPP things :-)
>>
My pleasure. I also have 
https://lore.kernel.org/linux-rockchip/20250509-camera-v3-0-dab2772d229a@bootlin.com/ 
pending = )
>> 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>
> 
>>> +	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.
> 
> not a "hard" rule, but a strong preference.
> I.e. we want people to ideally be able to just hit search in the
> schematics PDFs for the name they saw in the devicetree.
> 
> Sometimes that is not possible, because of naming conventions or
> overly strange schematic-names ... and sometimes things slip through
> as well.
> 
> But following the schematic names, is the general goal.
> 
Very fair. I used "lcd_reset" because even the schematic is not super 
clear: it uses "LCD_RST" on page 16 and LCD1_RST on pages 11 and 16.

> 
> If this stays the only suggestion though, I can fix that when
> applying. Or you can send a v3 - up to you :-)
> 
I'll correct to lcd1_rst_pin and send a v3 (most likely later today)
> 
> Heiko
> 
> 
>>
>>> +			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
>>
> 
> 
> 
> 

-- 
Olivier Benjamin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v2] arm64: dts: rockchip: Fix the PinePhone Pro DTS' panel description

[Diederik de Haas]

[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]

On Thu Jun 19, 2025 at 12:47 PM CEST, Olivier Benjamin wrote:
> On 6/19/25 12:31, Heiko Stuebner wrote:
>> Am Donnerstag, 19. Juni 2025, 11:46:15 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
>>> On Thu Jun 19, 2025 at 7:21 AM CEST, Olivier Benjamin wrote:
>>> Thanks for working on upstreaming PPP things :-)
>>>
> My pleasure. I also have
> https://lore.kernel.org/linux-rockchip/20250509-camera-v3-0-dab2772d229a@bootlin.com/
> pending = )

Happy about that too, but I 'happen' to research DSI connected displays
(for other devices), so I felt comfortable to chime in with that.
I haven't looked at camera stuff yet (and it's not high on my prio
list), so hopefully others will chime in for that :-)

>>>> 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>
>> 
>>>> +	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.
>> 
>> not a "hard" rule, but a strong preference.
>> I.e. we want people to ideally be able to just hit search in the
>> schematics PDFs for the name they saw in the devicetree.
>> 
>> But following the schematic names, is the general goal.
>> 
> Very fair. I used "lcd_reset" because even the schematic is not super 
> clear: it uses "LCD_RST" on page 16 and LCD1_RST on pages 11 and 16.

AIUI, the GPIO pin (GPIO4_D1) is labeled LCD1_RST and on page 16 you can
see that is connected to LCD_RST from BL102-G39-1FR.
The definition here is about the GPIO pin, hence LCD1_RST.
The other part of my suggestion was related to another 'convention':
the name and label/phandle are the same with ``s/-/_/`` (and 'reset-pin'
is not specific enough; there are a number of reset pins).

Afaic you don't need to mention that I suggested the mipi_out related
changes; the reason to do that (unneeded redefinition of already defined
things), is.

Cheers,
  Diederik 
 
>> If this stays the only suggestion though, I can fix that when
>> applying. Or you can send a v3 - up to you :-)
>> 
> I'll correct to lcd1_rst_pin and send a v3 (most likely later today)
>
>>>> +			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>
>>>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]