Messages in this thread | | | Date | Tue, 22 Nov 2022 12:46:56 +0000 | Subject | Re: [PATCH RFC v2 5/7] arm64: dts: imx: fix touchscreen reset GPIO polarity | From | Robin Murphy <> |
| |
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@gmail.com> wrote: >> >>> [Adding Angus and David] >> >> Thanks. This was apparently necessary ;-) >> >>> On Mon, Nov 21, 2022 at 3:12 PM Quentin Schulz >>> <foss+kernel@0leil.net> wrote: >>>> >>>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> >>>> >>>> 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@theobroma-systems.com> >>>> --- >>>> 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@theobroma-systems.com/ 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@theobroma-systems.com/ > 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-rockchip
| |