Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022-11-22 09:58, Quentin Schulz wrote:
Hi David,

Thanks Fabio for the Cc.

On 11/22/22 08:18, David Jander wrote:
On Mon, 21 Nov 2022 15:18:32 -0300
Fabio Estevam <festevam@xxxxxxxxx> wrote:

[Adding Angus and David]

Thanks. This was apparently necessary ;-)

On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz <foss+kernel@xxxxxxxxx> wrote:

From: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>

The reset line is active low for the Goodix touchscreen controller so
let's fix the polarity in the Device Tree node.

Signed-off-by: Quentin Schulz <quentin.schulz@xxxxxxxxxxxxxxxxxxxxx>
---
  arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts         | 2 +-
  arch/arm64/boot/dts/freescale/imx8mq-librem5-devkit.dts | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
index 9fbbbb556c0b3..df7e5ae9698e1 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mm-prt8mm.dts
@@ -107,7 +107,7 @@ touchscreeen@5d {
                 interrupt-parent = <&gpio1>;
                 interrupts = <8 IRQ_TYPE_NONE>;
                 irq-gpios = <&gpio1 8 GPIO_ACTIVE_HIGH>;
-               reset-gpios = <&gpio1 9 GPIO_ACTIVE_HIGH>;
+               reset-gpios = <&gpio1 9 GPIO_ACTIVE_LOW>;

NACK!

The PRT8MM has an inverter in the reset line. The reason for that is that the
reset line needs to be inactive when the driving side is unpowered.
The DT was correct, this change will break it.


The DT was correct. The implementation in the driver is changed (the polarity is swapped) in this patch series, therefore the DT isn't correct anymore, hence this patch.

I'm not sure it's quite that simple... FWIW I'm using an add-on LCD module with a GT9271[1] (and I won't be the only one - Raspberry Pi and other SBC users using DT overlays or custom-built DTBs are a whole other can of worms here), where GPIO_ACTIVE_LOW is correctly specified per the schematics, thus "wrong" for the current driver behaviour, yet it *is* working OK as-is. I guess that's because /RSTB ends up driven low for long enough between the current "deassertion" by gpiod_direction_output(1) and gpiod_direction_input() allowing the external pull-up to take it high again.

Robin.

[1] https://www.friendlyelec.com/index.php?route=product/product&path=81&product_id=230


See https://lore.kernel.org/linux-input/20221103-upstream-goodix-reset-v2-0-2c38fb03a300@xxxxxxxxxxxxxxxxxxxxx/ for the whole patch series.

This DT patch alone is obviously incorrect, but the context around it matters. I could/should have made it all into one big patch, the question is then how this big tree-crossing patch would be merged into Linux (if there's consensus). We're not there yet.

For some additional background on the discussion that was had in the v1:
https://lore.kernel.org/all/267de96a-0129-a97d-9bf6-e1001b422a1a@xxxxxxxxxxxxxxxxxxxxx/
I messed up the Cc list in the v1, apologies for the missing context in the archived mails, I think one should be able to understand the important bits by reading the answers in-mail. There, Dmitry, Hans and I discussed the meaning of the active level of GPIOs/reset lines and I expressed the reasons for such a change (which are also listed in the cover letter of this patch series).

As stated in v1 cover letter, no implementation will satisfy every one. We either make the DT binding implementation specific (which is what it shouldn't be), or we swap the polarity in the Linux implementation and thus the DT but then break DT backward compatibility.

Cheers,
Quentin

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-rockchip



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux