[PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
voltage for touch panel, LVDS receiver and RGB LCD panel. Add regulator
for these device nodes and remove powerdown-gpios property from
lvds-receiver node as it results in touch controller driver probe failure.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
v4->v5 : Restored Laurent's Rb tag, since it is minor change, renaming vcc-supply to power-supply

v3->v4 : Incorporated Laurent's review comments(https://patchwork.kernel.org/patch/11707887/)
         Added Laurent's Reviewed-by tag
v2->v3 : Added the missing part from the patch. removal of powerdown-gpios property.
v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel device nodes
           (Ref: https://patchwork.kernel.org/patch/11707559/)
v1 : https://patchwork.kernel.org/patch/11705819/
---
 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index ebbe1518ef8a..63cafd220dba 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -57,7 +57,7 @@
 
 	lvds-receiver {
 		compatible = "ti,ds90cf384a", "lvds-decoder";
-		powerdown-gpios = <&gpio7 25 GPIO_ACTIVE_LOW>;
+		power-supply = <&vcc_3v3_tft1>;
 
 		ports {
 			#address-cells = <1>;
@@ -81,6 +81,7 @@
 	panel {
 		compatible = "edt,etm0700g0dh6";
 		backlight = <&lcd_backlight>;
+		power-supply = <&vcc_3v3_tft1>;
 
 		port {
 			panel_in: endpoint {
@@ -113,6 +114,17 @@
 		};
 	};
 
+	vcc_3v3_tft1: regulator-panel {
+		compatible = "regulator-fixed";
+
+		regulator-name = "vcc-3v3-tft1";
+		regulator-min-microvolt = <3300000>;
+		regulator-max-microvolt = <3300000>;
+		enable-active-high;
+		startup-delay-us = <500>;
+		gpio = <&gpio7 25 GPIO_ACTIVE_HIGH>;
+	};
+
 	vcc_sdhi1: regulator-vcc-sdhi1 {
 		compatible = "regulator-fixed";
 
@@ -207,6 +219,7 @@
 		reg = <0x38>;
 		interrupt-parent = <&gpio2>;
 		interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+		vcc-supply = <&vcc_3v3_tft1>;
 	};
 };
 
-- 
2.17.1

Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Geert Uytterhoeven]

Hi Biju,

On Thu, Sep 24, 2020 at 10:05 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
> voltage for touch panel, LVDS receiver and RGB LCD panel. Add regulator
> for these device nodes and remove powerdown-gpios property from
> lvds-receiver node as it results in touch controller driver probe failure.
>
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> v4->v5 : Restored Laurent's Rb tag, since it is minor change, renaming vcc-supply to power-supply
>
> v3->v4 : Incorporated Laurent's review comments(https://patchwork.kernel.org/patch/11707887/)
>          Added Laurent's Reviewed-by tag
> v2->v3 : Added the missing part from the patch. removal of powerdown-gpios property.
> v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel device nodes
>            (Ref: https://patchwork.kernel.org/patch/11707559/)
> v1 : https://patchwork.kernel.org/patch/11705819/

Thanks for the update!

As the prerequisites are now in next, I can queue this in renesas-devel
for v5.11 after v5.10-rc1 has been released.
Or do you think this should be fast-tracked as a fix for v5.10 or v5.9?
I.e. is this an actual regression, or just something that never worked
before?
Note that v1 had a Fixes tag, which was lost in subsequent versions.

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

RE: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller
> probe failure
>
> Hi Biju,
>
> On Thu, Sep 24, 2020 at 10:05 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls
> > supply voltage for touch panel, LVDS receiver and RGB LCD panel. Add
> > regulator for these device nodes and remove powerdown-gpios property
> > from lvds-receiver node as it results in touch controller driver probe failure.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > v4->v5 : Restored Laurent's Rb tag, since it is minor change, renaming
> > v4->vcc-supply to power-supply
> >
> > v3->v4 : Incorporated Laurent's review
> > v3->comments(https://patchwork.kernel.org/patch/11707887/)
> >          Added Laurent's Reviewed-by tag
> > v2->v3 : Added the missing part from the patch. removal of powerdown-
> gpios property.
> > v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel
> > v1->device nodes
> >            (Ref: https://patchwork.kernel.org/patch/11707559/)
> > v1 : https://patchwork.kernel.org/patch/11705819/
>
> Thanks for the update!
>
> As the prerequisites are now in next, I can queue this in renesas-devel for
> v5.11 after v5.10-rc1 has been released.
> Or do you think this should be fast-tracked as a fix for v5.10 or v5.9?

I think it should be fast tracked, since it is a regression. Also we need to fast track  the dependency patches as well.

Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
[1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200923&id=946a61ab2d758cc645f6e63f1a5e2731690c3943
[2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200923&id=2cd9df2be75766452fef87c37ec37d91f4cbaf6b

> I.e. is this an actual regression, or just something that never worked before?
> Note that v1 had a Fixes tag, which was lost in subsequent versions.

I missed to add fixes tag in subsequent versions.
Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")

Cheers,
Biju


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Geert Uytterhoeven]

Hi Biju,

On Thu, Sep 24, 2020 at 11:40 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller
> > probe failure
> > On Thu, Sep 24, 2020 at 10:05 AM Biju Das <biju.das.jz@bp.renesas.com>
> > wrote:
> > > As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls
> > > supply voltage for touch panel, LVDS receiver and RGB LCD panel. Add
> > > regulator for these device nodes and remove powerdown-gpios property
> > > from lvds-receiver node as it results in touch controller driver probe failure.
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > v4->v5 : Restored Laurent's Rb tag, since it is minor change, renaming
> > > v4->vcc-supply to power-supply
> > >
> > > v3->v4 : Incorporated Laurent's review
> > > v3->comments(https://patchwork.kernel.org/patch/11707887/)
> > >          Added Laurent's Reviewed-by tag
> > > v2->v3 : Added the missing part from the patch. removal of powerdown-
> > gpios property.
> > > v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD panel
> > > v1->device nodes
> > >            (Ref: https://patchwork.kernel.org/patch/11707559/)
> > > v1 : https://patchwork.kernel.org/patch/11705819/
> >
> > Thanks for the update!
> >
> > As the prerequisites are now in next, I can queue this in renesas-devel for
> > v5.11 after v5.10-rc1 has been released.
> > Or do you think this should be fast-tracked as a fix for v5.10 or v5.9?
>
> I think it should be fast tracked, since it is a regression. Also we need to fast track  the dependency patches as well.

OK.

> Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200923&id=946a61ab2d758cc645f6e63f1a5e2731690c3943
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200923&id=2cd9df2be75766452fef87c37ec37d91f4cbaf6b

Does anything bad (e.g. another regression) happen if this patch is
applied, but the 2 patches above are not?
If not, I will queue this as a fix for v5.9.

> > I.e. is this an actual regression, or just something that never worked before?
> > Note that v1 had a Fixes tag, which was lost in subsequent versions.
>
> I missed to add fixes tag in subsequent versions.
> Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")

No worries, I can add that while applying.

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

RE: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

Hi Geert,

> Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller
> probe failure
>
> Hi Biju,
>
> On Thu, Sep 24, 2020 at 11:40 AM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > > Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch
> > > controller probe failure On Thu, Sep 24, 2020 at 10:05 AM Biju Das
> > > <biju.das.jz@bp.renesas.com>
> > > wrote:
> > > > As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls
> > > > supply voltage for touch panel, LVDS receiver and RGB LCD panel.
> > > > Add regulator for these device nodes and remove powerdown-gpios
> > > > property from lvds-receiver node as it results in touch controller driver
> probe failure.
> > > >
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > ---
> > > > v4->v5 : Restored Laurent's Rb tag, since it is minor change,
> > > > v4->renaming vcc-supply to power-supply
> > > >
> > > > v3->v4 : Incorporated Laurent's review
> > > > v3->comments(https://patchwork.kernel.org/patch/11707887/)
> > > >          Added Laurent's Reviewed-by tag
> > > > v2->v3 : Added the missing part from the patch. removal of
> > > > v2->powerdown-
> > > gpios property.
> > > > v1->v2 : Add regulator in touch panel, LVDS receiver and RGB LCD
> > > > v1->panel device nodes
> > > >            (Ref: https://patchwork.kernel.org/patch/11707559/)
> > > > v1 : https://patchwork.kernel.org/patch/11705819/
> > >
> > > Thanks for the update!
> > >
> > > As the prerequisites are now in next, I can queue this in
> > > renesas-devel for
> > > v5.11 after v5.10-rc1 has been released.
> > > Or do you think this should be fast-tracked as a fix for v5.10 or v5.9?
> >
> > I think it should be fast tracked, since it is a regression. Also we need to fast
> track  the dependency patches as well.
>
> OK.
>
> > Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/co
> > mmit/?h=next-20200923&id=946a61ab2d758cc645f6e63f1a5e2731690c3943
> > [2]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/co
> > mmit/?h=next-20200923&id=2cd9df2be75766452fef87c37ec37d91f4cbaf6b
>
> Does anything bad (e.g. another regression) happen if this patch is applied,
> but the 2 patches above are not?
> If not, I will queue this as a fix for v5.9.

It works without that 2 patches,  I confirm no side effects.

root@iwg20m-g1m:~# dmesg | grep edt
root@iwg20m-g1m:~# dmesg | grep input0
[    2.386156] input: EP0700M06 as /devices/platform/soc/e6530000.i2c/i2c-0/0-0038/input/input0
root@iwg20m-g1m:~#
root@iwg20m-g1m:~# uname -a
Linux iwg20m-g1m 5.9.0-rc1-00003-g58dba537f8cb #64 SMP Thu Sep 24 13:00:42 BST 2020 armv7l GNU/Linux

>
> > > I.e. is this an actual regression, or just something that never worked
> before?
> > > Note that v1 had a Fixes tag, which was lost in subsequent versions.
> >
> > I missed to add fixes tag in subsequent versions.
> > Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
>
> No worries, I can add that while applying.

Thank you.

Cheers,
Biju


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

[PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

Remove powerdown-gpios property from lvds-receiver node as it results in
touch controller driver probe failure.
As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
voltage for touch panel, LVDS receiver and RGB LCD panel.

Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
 arch/arm/boot/dts/iwg20d-q7-common.dtsi | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
index ebbe1518ef8a..4c8b9a6b0125 100644
--- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
+++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
@@ -57,7 +57,6 @@
 
 	lvds-receiver {
 		compatible = "ti,ds90cf384a", "lvds-decoder";
-		powerdown-gpios = <&gpio7 25 GPIO_ACTIVE_LOW>;
 
 		ports {
 			#address-cells = <1>;
-- 
2.17.1

Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Laurent Pinchart]

Hi Biju,

Thank you for the patch.

On Fri, Aug 07, 2020 at 04:20:38PM +0100, Biju Das wrote:
> Remove powerdown-gpios property from lvds-receiver node as it results in
> touch controller driver probe failure.
> As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls supply
> voltage for touch panel, LVDS receiver and RGB LCD panel.

If it controls the supply voltage, shouldn't it be modelled as a
regulator ? Dropping it completely would rely on the boot loader
powering the display on and keeping it powered, which isn't very nice.

> Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
>  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> index ebbe1518ef8a..4c8b9a6b0125 100644
> --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> @@ -57,7 +57,6 @@
>  
>  	lvds-receiver {
>  		compatible = "ti,ds90cf384a", "lvds-decoder";
> -		powerdown-gpios = <&gpio7 25 GPIO_ACTIVE_LOW>;
>  
>  		ports {
>  			#address-cells = <1>;

-- 
Regards,

Laurent Pinchart

RE: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Biju Das]

Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller
> probe failure
>
> Hi Biju,
>
> Thank you for the patch.
>
> On Fri, Aug 07, 2020 at 04:20:38PM +0100, Biju Das wrote:
> > Remove powerdown-gpios property from lvds-receiver node as it results
> > in touch controller driver probe failure.
> > As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls
> > supply voltage for touch panel, LVDS receiver and RGB LCD panel.
>
> If it controls the supply voltage, shouldn't it be modelled as a regulator ?
> Dropping it completely would rely on the boot loader powering the display
> on and keeping it powered, which isn't very nice.

On the schematic LVDS_PPEN, controls the voltage VCC_3V3_TFT1, which supplies voltage to
LVDS receiver, Touch controller and RGB LCD Panel.

I agree, it should be modelled as a regulator and this regulator should present in
all these devices nodes right(LVDS receiver, Touch controller and RGB LCD Panel),
not just on LVDS receiver node? Please share your views on this.

Cheers,
Biju
>
> > Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-
> lad.rj@bp.renesas.com>
> > ---
> >  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > index ebbe1518ef8a..4c8b9a6b0125 100644
> > --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > @@ -57,7 +57,6 @@
> >
> >  lvds-receiver {
> >  compatible = "ti,ds90cf384a", "lvds-decoder";
> > -powerdown-gpios = <&gpio7 25 GPIO_ACTIVE_LOW>;
> >
> >  ports {
> >  #address-cells = <1>;
>
> --
> Regards,
>
> Laurent Pinchart


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. no.: DE 14978647

Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller probe failure

[Laurent Pinchart]

Hi Biju,

On Sun, Aug 09, 2020 at 09:17:42AM +0000, Biju Das wrote:
> > Subject: Re: [PATCH] ARM: dts: iwg20d-q7-common: Fix touch controller
> > probe failure
> >
> > Hi Biju,
> >
> > Thank you for the patch.
> >
> > On Fri, Aug 07, 2020 at 04:20:38PM +0100, Biju Das wrote:
> > > Remove powerdown-gpios property from lvds-receiver node as it results
> > > in touch controller driver probe failure.
> > > As per the iWave RZ/G1M schematic, the signal LVDS_PPEN controls
> > > supply voltage for touch panel, LVDS receiver and RGB LCD panel.
> >
> > If it controls the supply voltage, shouldn't it be modelled as a regulator ?
> > Dropping it completely would rely on the boot loader powering the display
> > on and keeping it powered, which isn't very nice.
> 
> On the schematic LVDS_PPEN, controls the voltage VCC_3V3_TFT1, which supplies voltage to
> LVDS receiver, Touch controller and RGB LCD Panel.
> 
> I agree, it should be modelled as a regulator and this regulator should present in
> all these devices nodes right(LVDS receiver, Touch controller and RGB LCD Panel),
> not just on LVDS receiver node? Please share your views on this.

Correct, that's what I would recommend.

> > > Fixes: 6f89dd9e9325 ("ARM: dts: iwg20d-q7-common: Add LCD support")
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-
> > lad.rj@bp.renesas.com>
> > > ---
> > >  arch/arm/boot/dts/iwg20d-q7-common.dtsi | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > > b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > > index ebbe1518ef8a..4c8b9a6b0125 100644
> > > --- a/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > > +++ b/arch/arm/boot/dts/iwg20d-q7-common.dtsi
> > > @@ -57,7 +57,6 @@
> > >
> > >  lvds-receiver {
> > >  compatible = "ti,ds90cf384a", "lvds-decoder";
> > > -powerdown-gpios = <&gpio7 25 GPIO_ACTIVE_LOW>;
> > >
> > >  ports {
> > >  #address-cells = <1>;

-- 
Regards,

Laurent Pinchart