| Message ID | 20220427185243.173594-4-detlev.casanova@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | ARM: dts: Support official Raspberry Pi 7inch touchscreen | expand |
On Wed, Apr 27, 2022 at 02:52:43PM -0400, Detlev Casanova wrote: > Add a device tree overlay to support the official Raspberrypi 7" touchscreen for > the bcm2711 devices. > > The panel is connected on the DSI 1 port and uses the simple-panel > driver. > > The device tree also makes sure to activate the pixelvalve[0-4] CRTC modules > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > --- > arch/arm/boot/dts/Makefile | 4 + > arch/arm/boot/dts/overlays/Makefile | 3 + > .../dts/overlays/bcm2711-rpi-7-inches-ts.dts | 125 ++++++++++++++++++ .dtso is preferred. I think... It was discussed, but I never got an updated patch to switch. > arch/arm64/boot/dts/broadcom/Makefile | 4 + > .../arm64/boot/dts/broadcom/overlays/Makefile | 3 + > .../overlays/bcm2711-rpi-7-inches-ts.dts | 2 + > 6 files changed, 141 insertions(+) > create mode 100644 arch/arm/boot/dts/overlays/Makefile > create mode 100644 arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts A global (to arm) 'overlays' directory will create the same mess that we have in arch/arm/boot/dts/. IMO, first you should move all the Broadcom dts files to a 'broadcom' subdirectory like we have for arm64. > create mode 100644 arch/arm64/boot/dts/broadcom/overlays/Makefile > create mode 100644 arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 235ad559acb2..eb0b0b121947 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -1549,3 +1549,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \ > aspeed-bmc-vegman-n110.dtb \ > aspeed-bmc-vegman-rx20.dtb \ > aspeed-bmc-vegman-sx20.dtb > + > +ifeq ($(CONFIG_OF_OVERLAY),y) > +subdir-y += overlays I don't think this should depend on the config. If it does, you can do this in scripts/Makefile.lib by removing .dtbo targets. Or this could have been just: subdir-$(CONFIG_OF_OVERLAY) += overlays But I prefer the former so each platform is not picking their own way. > +endif > diff --git a/arch/arm/boot/dts/overlays/Makefile b/arch/arm/boot/dts/overlays/Makefile > new file mode 100644 > index 000000000000..c90883dfaf91 > --- /dev/null > +++ b/arch/arm/boot/dts/overlays/Makefile > @@ -0,0 +1,3 @@ > +# SPDX-License-Identifier: GPL-2.0 > + > +dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-7-inches-ts.dtbo The overlays should be applied to the base dtb(s) to ensure they actually apply and so we can validate them with schema. kbuild supports this already. > diff --git a/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts > b/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts > new file mode 100644 > index 000000000000..de98a6c1079a > --- /dev/null > +++ b/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts > @@ -0,0 +1,125 @@ > +// SPDX-License-Identifier: GPL-2.0 No one uses RPi with *BSD? Dual licensing is preferred. Of course, what this applies to should have similar licensing. > + > +/dts-v1/; > +/plugin/; > + > +&{/} { > + #address-cells = <2>; > + #size-cells = <1>; > + > + panel_disp1: panel@0 { What is '0' representing? > + reg = <0 0 0>; > + compatible = "raspberrypi,7inch-dsi", "simple-panel"; > + backlight = <®_display>; > + power-supply = <®_display>; > + status = "okay"; That's the default. Same thing in a few other spots. > + > + port { > + panel_in: endpoint { > + remote-endpoint = <&bridge_out>; > + }; > + }; > + }; > + > + reg_bridge: regulator@0 { Oops! 2 different things at the same address! > + reg = <0 0 0>; 'regulator-fixed' doesn't have an address. > + compatible = "regulator-fixed"; > + regulator-name = "bridge_reg"; > + gpio = <®_display 0 0>; > + vin-supply = <®_display>; > + enable-active-high; > + status = "okay"; > + }; > +}; > + > +&i2c_csi_dsi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ft5406: touchscreen@38 { > + compatible = "edt,edt-ft5506"; > + reg = <0x38>; > + status = "okay"; > + > + vcc-supply = <®_display>; > + reset-gpio = <®_display 1 1>; > + > + touchscreen-size-x = < 800 >; > + touchscreen-size-y = < 480 >; > + > + touchscreen-inverted-x; > + touchscreen-inverted-y; > + }; > + > + reg_display: regulator@45 { > + compatible = "raspberrypi,7inch-touchscreen-panel-regulator"; > + reg = <0x45>; > + status = "okay"; > + > + gpio-controller; > + #gpio-cells = <2>; The regulator is a gpio-controller? > + }; > + > +}; > + > +&dsi1 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + status = "okay"; > + > + port { > + dsi_out: endpoint { > + remote-endpoint = <&bridge_in>; > + }; > + }; > + > + bridge@0 { > + #address-cells = <1>; > + #size-cells = <0>; Not valid here. But I shouldn't have to tell you this as the schema checks will. Please run them. Applied to a base should work for sure. As just a .dtbo, there's probably some issues, but complete nodes like this should validate fine. > + > + reg = <0>; > + compatible = "toshiba,tc358762"; > + > + vddc-supply = <®_bridge>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + bridge_in: endpoint { > + remote-endpoint = <&dsi_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + bridge_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + }; > +}; > + > +&pixelvalve0 { > + status = "okay"; > +}; > + > +&pixelvalve1 { > + status = "okay"; > +}; > + > +&pixelvalve2 { > + status = "okay"; > +}; > + > +&pixelvalve3 { > + status = "okay"; > +}; > + > +&pixelvalve4 { > + status = "okay"; > +};
Hi Rob, On Wed, Apr 27, 2022 at 11:23 PM Rob Herring <robh@kernel.org> wrote: > On Wed, Apr 27, 2022 at 02:52:43PM -0400, Detlev Casanova wrote: > > Add a device tree overlay to support the official Raspberrypi 7" touchscreen for > > the bcm2711 devices. > > > > The panel is connected on the DSI 1 port and uses the simple-panel > > driver. > > > > The device tree also makes sure to activate the pixelvalve[0-4] CRTC modules > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > --- > > arch/arm/boot/dts/Makefile | 4 + > > arch/arm/boot/dts/overlays/Makefile | 3 + > > .../dts/overlays/bcm2711-rpi-7-inches-ts.dts | 125 ++++++++++++++++++ > > .dtso is preferred. I think... It was discussed, but I never got an > updated patch to switch. Unfortunately that switch indeed hasn't happened yet. My main gripe with .dts for overlays is that you cannot know whether it's an overlay or not without reading the file's contents. Hence tools like make also cannot know, and you need to e.g. list all files explicitly in a Makefile. > > arch/arm64/boot/dts/broadcom/Makefile | 4 + > > .../arm64/boot/dts/broadcom/overlays/Makefile | 3 + > > .../overlays/bcm2711-rpi-7-inches-ts.dts | 2 + > > 6 files changed, 141 insertions(+) > > create mode 100644 arch/arm/boot/dts/overlays/Makefile > > create mode 100644 arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts > > A global (to arm) 'overlays' directory will create the same mess that we > have in arch/arm/boot/dts/. IMO, first you should move all the Broadcom > dts files to a 'broadcom' subdirectory like we have for arm64. As I believe this display is not only used with real Raspberry Pi devices, it makes sense to not have it a broadcom directory. In fact it may be used on other architectures than arm, too, so I think we need an arch-agnostic directory for overlays[1]? This may need remapping of labels. I'm aware the rpi infrastructure has support for remapping labels when applying overlays during boot, but AFAIK this is not yet supported by fdtoverlay (or perhaps by a fork?)? Note that the remapping is also needed if you want to apply two instances of the same overlay. > > create mode 100644 arch/arm64/boot/dts/broadcom/overlays/Makefile > > create mode 100644 arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts And this one just includes the former, and thus sort-of serves as an example of my point above ;-) [1] Note that this does not only apply to overlays: soon we will have a full SoC peripheral description in r9a07g043.dtsi to share between RZ/G2UL (arm64) and RZ/Five (riscv)). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Apr 28, 2022 at 08:44:17AM +0200, Geert Uytterhoeven wrote: > Hi Rob, > > On Wed, Apr 27, 2022 at 11:23 PM Rob Herring <robh@kernel.org> wrote: > > On Wed, Apr 27, 2022 at 02:52:43PM -0400, Detlev Casanova wrote: > > > Add a device tree overlay to support the official Raspberrypi 7" touchscreen for > > > the bcm2711 devices. > > > > > > The panel is connected on the DSI 1 port and uses the simple-panel > > > driver. > > > > > > The device tree also makes sure to activate the pixelvalve[0-4] CRTC modules > > > > > > Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> > > > --- > > > arch/arm/boot/dts/Makefile | 4 + > > > arch/arm/boot/dts/overlays/Makefile | 3 + > > > .../dts/overlays/bcm2711-rpi-7-inches-ts.dts | 125 ++++++++++++++++++ > > > > .dtso is preferred. I think... It was discussed, but I never got an > > updated patch to switch. > > Unfortunately that switch indeed hasn't happened yet. > My main gripe with .dts for overlays is that you cannot know whether > it's an overlay or not without reading the file's contents. > Hence tools like make also cannot know, and you need to e.g. list > all files explicitly in a Makefile. See my reply in the other thread for that. > > > arch/arm64/boot/dts/broadcom/Makefile | 4 + > > > .../arm64/boot/dts/broadcom/overlays/Makefile | 3 + > > > .../overlays/bcm2711-rpi-7-inches-ts.dts | 2 + > > > 6 files changed, 141 insertions(+) > > > create mode 100644 arch/arm/boot/dts/overlays/Makefile > > > create mode 100644 arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts > > > > A global (to arm) 'overlays' directory will create the same mess that we > > have in arch/arm/boot/dts/. IMO, first you should move all the Broadcom > > dts files to a 'broadcom' subdirectory like we have for arm64. > > As I believe this display is not only used with real Raspberry Pi > devices, it makes sense to not have it a broadcom directory. Then at a minimum 'bcm2711' in the name is not appropriate. I'm doubtful that as-is the overlay would apply to boards outside of RPi's. For this to work (well), there needs to be a connector node to translate between connector resources and the base board resources. See the recent mikrobus thread[2]. > In fact it may be used on other architectures than arm, too, so I > think we need an arch-agnostic directory for overlays[1]? Probably so. Personally, I would prefer no DTs under /arch. > This may need remapping of labels. I'm aware the rpi infrastructure has > support for remapping labels when applying overlays during boot, but > AFAIK this is not yet supported by fdtoverlay (or perhaps by a fork?)? > Note that the remapping is also needed if you want to apply two > instances of the same overlay. First I've heard of label remapping... I have a lot of concerns about using labels for overlays. For starters, with a flip of a switch (-@), they all become an ABI when they were not previously. I think at a minimum, we need an annotation so that a subset can be exported. Anything that's an ABI, we should be documenting and reviewing. The requirement for overlays upstream is that they are applied at build time to a base DT. Otherwise, we can't validate them completely. So if there's a label remapping dependency on these, sounds like there is some more work to do. The first being getting agreement that label remapping is the right approach. Common label names or some remapping for targets kind of works, but easily falls apart. For example, GPIO (or any provider with identifier cells) numbering or SPI CS numbering would be different. Rob [2] https://lore.kernel.org/all/YmFo+EntwxIsco%2Ft@robh.at.kernel.org/
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile index 235ad559acb2..eb0b0b121947 100644 --- a/arch/arm/boot/dts/Makefile +++ b/arch/arm/boot/dts/Makefile @@ -1549,3 +1549,7 @@ dtb-$(CONFIG_ARCH_ASPEED) += \ aspeed-bmc-vegman-n110.dtb \ aspeed-bmc-vegman-rx20.dtb \ aspeed-bmc-vegman-sx20.dtb + +ifeq ($(CONFIG_OF_OVERLAY),y) +subdir-y += overlays +endif diff --git a/arch/arm/boot/dts/overlays/Makefile b/arch/arm/boot/dts/overlays/Makefile new file mode 100644 index 000000000000..c90883dfaf91 --- /dev/null +++ b/arch/arm/boot/dts/overlays/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-7-inches-ts.dtbo diff --git a/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts b/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts new file mode 100644 index 000000000000..de98a6c1079a --- /dev/null +++ b/arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: GPL-2.0 + +/dts-v1/; +/plugin/; + +&{/} { + #address-cells = <2>; + #size-cells = <1>; + + panel_disp1: panel@0 { + reg = <0 0 0>; + compatible = "raspberrypi,7inch-dsi", "simple-panel"; + backlight = <®_display>; + power-supply = <®_display>; + status = "okay"; + + port { + panel_in: endpoint { + remote-endpoint = <&bridge_out>; + }; + }; + }; + + reg_bridge: regulator@0 { + reg = <0 0 0>; + compatible = "regulator-fixed"; + regulator-name = "bridge_reg"; + gpio = <®_display 0 0>; + vin-supply = <®_display>; + enable-active-high; + status = "okay"; + }; +}; + +&i2c_csi_dsi { + #address-cells = <1>; + #size-cells = <0>; + + ft5406: touchscreen@38 { + compatible = "edt,edt-ft5506"; + reg = <0x38>; + status = "okay"; + + vcc-supply = <®_display>; + reset-gpio = <®_display 1 1>; + + touchscreen-size-x = < 800 >; + touchscreen-size-y = < 480 >; + + touchscreen-inverted-x; + touchscreen-inverted-y; + }; + + reg_display: regulator@45 { + compatible = "raspberrypi,7inch-touchscreen-panel-regulator"; + reg = <0x45>; + status = "okay"; + + gpio-controller; + #gpio-cells = <2>; + }; + +}; + +&dsi1 { + #address-cells = <1>; + #size-cells = <0>; + + status = "okay"; + + port { + dsi_out: endpoint { + remote-endpoint = <&bridge_in>; + }; + }; + + bridge@0 { + #address-cells = <1>; + #size-cells = <0>; + + reg = <0>; + compatible = "toshiba,tc358762"; + + vddc-supply = <®_bridge>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + bridge_in: endpoint { + remote-endpoint = <&dsi_out>; + }; + }; + + port@1 { + reg = <1>; + bridge_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + }; +}; + +&pixelvalve0 { + status = "okay"; +}; + +&pixelvalve1 { + status = "okay"; +}; + +&pixelvalve2 { + status = "okay"; +}; + +&pixelvalve3 { + status = "okay"; +}; + +&pixelvalve4 { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile index c6882032a428..a6c43c5b053a 100644 --- a/arch/arm64/boot/dts/broadcom/Makefile +++ b/arch/arm64/boot/dts/broadcom/Makefile @@ -7,6 +7,10 @@ dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \ bcm2837-rpi-3-b-plus.dtb \ bcm2837-rpi-cm3-io3.dtb +ifeq ($(CONFIG_OF_OVERLAY),y) +subdir-y += overlays +endif + subdir-y += bcm4908 subdir-y += northstar2 subdir-y += stingray diff --git a/arch/arm64/boot/dts/broadcom/overlays/Makefile b/arch/arm64/boot/dts/broadcom/overlays/Makefile new file mode 100644 index 000000000000..c90883dfaf91 --- /dev/null +++ b/arch/arm64/boot/dts/broadcom/overlays/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-7-inches-ts.dtbo diff --git a/arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts b/arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts new file mode 100644 index 000000000000..7d532b090305 --- /dev/null +++ b/arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts @@ -0,0 +1,2 @@ +// SPDX-License-Identifier: GPL-2.0 +#include "arm/overlays/bcm2711-rpi-7-inches-ts.dts"
Add a device tree overlay to support the official Raspberrypi 7" touchscreen for the bcm2711 devices. The panel is connected on the DSI 1 port and uses the simple-panel driver. The device tree also makes sure to activate the pixelvalve[0-4] CRTC modules Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> --- arch/arm/boot/dts/Makefile | 4 + arch/arm/boot/dts/overlays/Makefile | 3 + .../dts/overlays/bcm2711-rpi-7-inches-ts.dts | 125 ++++++++++++++++++ arch/arm64/boot/dts/broadcom/Makefile | 4 + .../arm64/boot/dts/broadcom/overlays/Makefile | 3 + .../overlays/bcm2711-rpi-7-inches-ts.dts | 2 + 6 files changed, 141 insertions(+) create mode 100644 arch/arm/boot/dts/overlays/Makefile create mode 100644 arch/arm/boot/dts/overlays/bcm2711-rpi-7-inches-ts.dts create mode 100644 arch/arm64/boot/dts/broadcom/overlays/Makefile create mode 100644 arch/arm64/boot/dts/broadcom/overlays/bcm2711-rpi-7-inches-ts.dts