| Message ID | 1573157463-14070-7-git-send-email-fabrizio.castro@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add LCD panel support to iwg20d | expand |
Hi Fabrizio, Thank you for the patch. On Thu, Nov 07, 2019 at 08:11:02PM +0000, Fabrizio Castro wrote: > The iwg20d comes with a 7" capacitive touch screen, therefore > add support for it. > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > --- > v2->v3: > * No change > v1->v2: > * No change > --- > arch/arm/boot/dts/iwg20d-q7-common.dtsi | 85 ++++++++++++++++++++++++++++++++ > arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi | 1 - > 2 files changed, 85 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi > index ae75a1db..3428b8d 100644 > --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi > +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi > @@ -46,6 +46,49 @@ > clock-frequency = <26000000>; > }; > > + lcd_backlight: backlight { > + compatible = "pwm-backlight"; > + > + pwms = <&pwm3 0 5000000 0>; > + brightness-levels = <0 4 8 16 32 64 128 255>; > + default-brightness-level = <7>; > + enable-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>; > + }; > + > + lvds-receiver { > + compatible = "lvds-decoder"; A specific compatible string is required. I think the lvds-decoder driver should error out at probe time if only one compatible string is listed. > + powerdown = <&gpio7 25 GPIO_ACTIVE_LOW>; powerdown-gpios ? > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds_receiver_in: endpoint { > + remote-endpoint = <&lvds0_out>; > + }; > + }; > + port@1 { > + reg = <1>; > + lvds_receiver_out: endpoint { > + remote-endpoint = <&panel_in>; > + }; > + }; > + }; > + }; > + > + panel { > + compatible = "edt,etm0700g0dh6", "simple-panel"; There's no "simple-panel" compatible string defined anywhere as far as I can tell. > + backlight = <&lcd_backlight>; > + > + port { > + panel_in: endpoint { > + remote-endpoint = <&lvds_receiver_out>; > + }; > + }; > + }; > + > reg_1p5v: 1p5v { > compatible = "regulator-fixed"; > regulator-name = "1P5V"; > @@ -120,6 +163,18 @@ > status = "okay"; > }; > > +&du { > + status = "okay"; > +}; > + > +&gpio2 { > + touch-interrupt { > + gpio-hog; > + gpios = <12 GPIO_ACTIVE_LOW>; > + input; > + }; Do you need this, with the touchpanel@38 node already listing the interrupt ? > +}; > + > &hsusb { > status = "okay"; > pinctrl-0 = <&usb0_pins>; > @@ -147,6 +202,25 @@ > VDDIO-supply = <®_3p3v>; > VDDD-supply = <®_1p5v>; > }; > + > + touch: touchpanel@38 { > + compatible = "edt,edt-ft5406"; > + reg = <0x38>; > + interrupt-parent = <&gpio2>; > + interrupts = <12 IRQ_TYPE_EDGE_FALLING>; > + }; > +}; > + > +&lvds0 { > + status = "okay"; > + > + ports { > + port@1 { > + lvds0_out: endpoint { > + remote-endpoint = <&lvds_receiver_in>; > + }; > + }; > + }; > }; > > &pci0 { > @@ -180,6 +254,11 @@ > function = "i2c2"; > }; > > + pwm3_pins: pwm3 { > + groups = "pwm3"; > + function = "pwm3"; > + }; > + > scif0_pins: scif0 { > groups = "scif0_data_d"; > function = "scif0"; > @@ -218,6 +297,12 @@ > }; > }; > > +&pwm3 { > + pinctrl-0 = <&pwm3_pins>; > + pinctrl-names = "default"; > + status = "okay"; > +}; > + > &rcar_sound { > pinctrl-0 = <&sound_pins>; > pinctrl-names = "default"; > diff --git a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > index 0e99df2..ede2e0c 100644 > --- a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > +++ b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > @@ -39,7 +39,6 @@ > &du { > pinctrl-0 = <&du_pins>; > pinctrl-names = "default"; > - status = "okay"; > > ports { > port@0 {
Hi Laurent, Thank you for your feedback! > From: linux-renesas-soc-owner@vger.kernel.org <linux-renesas-soc-owner@vger.kernel.org> On Behalf Of Laurent Pinchart > Sent: 07 November 2019 20:55 > Subject: Re: [PATCH v3 6/7] ARM: dts: iwg20d-q7-common: Add LCD support > > Hi Fabrizio, > > Thank you for the patch. > > On Thu, Nov 07, 2019 at 08:11:02PM +0000, Fabrizio Castro wrote: > > The iwg20d comes with a 7" capacitive touch screen, therefore > > add support for it. > > > > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> > > > > --- > > v2->v3: > > * No change > > v1->v2: > > * No change > > --- > > arch/arm/boot/dts/iwg20d-q7-common.dtsi | 85 ++++++++++++++++++++++++++++++++ > > arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi | 1 - > > 2 files changed, 85 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi > > index ae75a1db..3428b8d 100644 > > --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi > > +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi > > @@ -46,6 +46,49 @@ > > clock-frequency = <26000000>; > > }; > > > > + lcd_backlight: backlight { > > + compatible = "pwm-backlight"; > > + > > + pwms = <&pwm3 0 5000000 0>; > > + brightness-levels = <0 4 8 16 32 64 128 255>; > > + default-brightness-level = <7>; > > + enable-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>; > > + }; > > + > > + lvds-receiver { > > + compatible = "lvds-decoder"; > > A specific compatible string is required. Will document the specific compatible in the binding doc > > I think the lvds-decoder driver should error out at probe time if only > one compatible string is listed. Ok, will modify the driver accordingly > > > + powerdown = <&gpio7 25 GPIO_ACTIVE_LOW>; > > powerdown-gpios ? That's a bug, well spotted. > > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + lvds_receiver_in: endpoint { > > + remote-endpoint = <&lvds0_out>; > > + }; > > + }; > > + port@1 { > > + reg = <1>; > > + lvds_receiver_out: endpoint { > > + remote-endpoint = <&panel_in>; > > + }; > > + }; > > + }; > > + }; > > + > > + panel { > > + compatible = "edt,etm0700g0dh6", "simple-panel"; > > There's no "simple-panel" compatible string defined anywhere as far as I > can tell. The usage of "simple-panel" as a compatible is widespread and really confusing. The fact that you made this comment is good enough for me to say we don't need it, I'll take it out. > > > + backlight = <&lcd_backlight>; > > + > > + port { > > + panel_in: endpoint { > > + remote-endpoint = <&lvds_receiver_out>; > > + }; > > + }; > > + }; > > + > > reg_1p5v: 1p5v { > > compatible = "regulator-fixed"; > > regulator-name = "1P5V"; > > @@ -120,6 +163,18 @@ > > status = "okay"; > > }; > > > > +&du { > > + status = "okay"; > > +}; > > + > > +&gpio2 { > > + touch-interrupt { > > + gpio-hog; > > + gpios = <12 GPIO_ACTIVE_LOW>; > > + input; > > + }; > > Do you need this, with the touchpanel@38 node already listing the > interrupt ? Yes, unfortunately we do need this as the bootloader is poking with the gpio. I can't fix this in the bootloader as we have no control over it. Thanks, Fab > > > +}; > > + > > &hsusb { > > status = "okay"; > > pinctrl-0 = <&usb0_pins>; > > @@ -147,6 +202,25 @@ > > VDDIO-supply = <®_3p3v>; > > VDDD-supply = <®_1p5v>; > > }; > > + > > + touch: touchpanel@38 { > > + compatible = "edt,edt-ft5406"; > > + reg = <0x38>; > > + interrupt-parent = <&gpio2>; > > + interrupts = <12 IRQ_TYPE_EDGE_FALLING>; > > + }; > > +}; > > + > > +&lvds0 { > > + status = "okay"; > > + > > + ports { > > + port@1 { > > + lvds0_out: endpoint { > > + remote-endpoint = <&lvds_receiver_in>; > > + }; > > + }; > > + }; > > }; > > > > &pci0 { > > @@ -180,6 +254,11 @@ > > function = "i2c2"; > > }; > > > > + pwm3_pins: pwm3 { > > + groups = "pwm3"; > > + function = "pwm3"; > > + }; > > + > > scif0_pins: scif0 { > > groups = "scif0_data_d"; > > function = "scif0"; > > @@ -218,6 +297,12 @@ > > }; > > }; > > > > +&pwm3 { > > + pinctrl-0 = <&pwm3_pins>; > > + pinctrl-names = "default"; > > + status = "okay"; > > +}; > > + > > &rcar_sound { > > pinctrl-0 = <&sound_pins>; > > pinctrl-names = "default"; > > diff --git a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > > index 0e99df2..ede2e0c 100644 > > --- a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > > +++ b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi > > @@ -39,7 +39,6 @@ > > &du { > > pinctrl-0 = <&du_pins>; > > pinctrl-names = "default"; > > - status = "okay"; > > > > ports { > > port@0 { > > -- > Regards, > > Laurent Pinchart
diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi index ae75a1db..3428b8d 100644 --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi @@ -46,6 +46,49 @@ clock-frequency = <26000000>; }; + lcd_backlight: backlight { + compatible = "pwm-backlight"; + + pwms = <&pwm3 0 5000000 0>; + brightness-levels = <0 4 8 16 32 64 128 255>; + default-brightness-level = <7>; + enable-gpios = <&gpio5 14 GPIO_ACTIVE_HIGH>; + }; + + lvds-receiver { + compatible = "lvds-decoder"; + powerdown = <&gpio7 25 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds_receiver_in: endpoint { + remote-endpoint = <&lvds0_out>; + }; + }; + port@1 { + reg = <1>; + lvds_receiver_out: endpoint { + remote-endpoint = <&panel_in>; + }; + }; + }; + }; + + panel { + compatible = "edt,etm0700g0dh6", "simple-panel"; + backlight = <&lcd_backlight>; + + port { + panel_in: endpoint { + remote-endpoint = <&lvds_receiver_out>; + }; + }; + }; + reg_1p5v: 1p5v { compatible = "regulator-fixed"; regulator-name = "1P5V"; @@ -120,6 +163,18 @@ status = "okay"; }; +&du { + status = "okay"; +}; + +&gpio2 { + touch-interrupt { + gpio-hog; + gpios = <12 GPIO_ACTIVE_LOW>; + input; + }; +}; + &hsusb { status = "okay"; pinctrl-0 = <&usb0_pins>; @@ -147,6 +202,25 @@ VDDIO-supply = <®_3p3v>; VDDD-supply = <®_1p5v>; }; + + touch: touchpanel@38 { + compatible = "edt,edt-ft5406"; + reg = <0x38>; + interrupt-parent = <&gpio2>; + interrupts = <12 IRQ_TYPE_EDGE_FALLING>; + }; +}; + +&lvds0 { + status = "okay"; + + ports { + port@1 { + lvds0_out: endpoint { + remote-endpoint = <&lvds_receiver_in>; + }; + }; + }; }; &pci0 { @@ -180,6 +254,11 @@ function = "i2c2"; }; + pwm3_pins: pwm3 { + groups = "pwm3"; + function = "pwm3"; + }; + scif0_pins: scif0 { groups = "scif0_data_d"; function = "scif0"; @@ -218,6 +297,12 @@ }; }; +&pwm3 { + pinctrl-0 = <&pwm3_pins>; + pinctrl-names = "default"; + status = "okay"; +}; + &rcar_sound { pinctrl-0 = <&sound_pins>; pinctrl-names = "default"; diff --git a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi index 0e99df2..ede2e0c 100644 --- a/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi +++ b/arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi @@ -39,7 +39,6 @@ &du { pinctrl-0 = <&du_pins>; pinctrl-names = "default"; - status = "okay"; ports { port@0 {
The iwg20d comes with a 7" capacitive touch screen, therefore add support for it. Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com> --- v2->v3: * No change v1->v2: * No change --- arch/arm/boot/dts/iwg20d-q7-common.dtsi | 85 ++++++++++++++++++++++++++++++++ arch/arm/boot/dts/iwg20d-q7-dbcm-ca.dtsi | 1 - 2 files changed, 85 insertions(+), 1 deletion(-)