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@xxxxxxxxxxx/ 
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@xxxxxxxxxxx>
>
> > > +	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@xxxxxxxxx>
> >
> > Cheers,
> >    Diederik

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