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@xxxxxxxxxxxxx>
> ---
> 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";
> +};
