From: Olivier Benjamin <hidden> Date: 2025-06-19 05:21:14
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 <redacted>
---
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(-)
From: Diederik de Haas <hidden> Date: 2025-06-19 09:46:40
Hi,
Thanks for working on upstreaming PPP things :-)
On Thu Jun 19, 2025 at 7:21 AM CEST, Olivier Benjamin wrote:
quoted hunk
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 <redacted>
---
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(-)
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.
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:
quoted
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 <redacted>
quoted
+ 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
On Thu Jun 19, 2025 at 7:21 AM CEST, Olivier Benjamin wrote:
quoted
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 <redacted>
quoted
quoted
+ 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)
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 :-)
quoted
quoted
quoted
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 <redacted>
quoted
quoted
+ 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
quoted
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)