[PATCH v3 4/5] PCI: imx6: Convert to agnostic GPIO API

[Hongxing Zhu]

> -----Original Message-----
> From: Linus Walleij <linus.walleij at linaro.org>
> Sent: 2024年5月6日 20:10
> To: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>; Frank Li
> <frank.li at nxp.com>; Krzysztof Wilczyński <kwilczynski at kernel.org>; Uwe
> Kleine-König <u.kleine-koenig at pengutronix.de>; linux-omap at vger.kernel.org;
> linux-pci at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> linux-kernel at vger.kernel.org; imx at lists.linux.dev;
> linux-amlogic at lists.infradead.org; linux-arm-msm at vger.kernel.org;
> linux-tegra at vger.kernel.org; Vignesh Raghavendra <vigneshr at ti.com>;
> Siddharth Vadapalli <s-vadapalli at ti.com>; Lorenzo Pieralisi
> <lpieralisi at kernel.org>; Krzysztof Wilczyński <kw at linux.com>; Rob Herring
> <robh at kernel.org>; Bjorn Helgaas <bhelgaas at google.com>; Hongxing Zhu
> <hongxing.zhu at nxp.com>; Lucas Stach <l.stach at pengutronix.de>; Shawn Guo
> <shawnguo at kernel.org>; Sascha Hauer <s.hauer at pengutronix.de>; Pengutronix
> Kernel Team <kernel at pengutronix.de>; Fabio Estevam <festevam at gmail.com>;
> Yue Wang <yue.wang at amlogic.com>; Neil Armstrong
> <neil.armstrong at linaro.org>; Kevin Hilman <khilman at baylibre.com>; Jerome
> Brunet <jbrunet at baylibre.com>; Martin Blumenstingl
> <martin.blumenstingl at googlemail.com>; Xiaowei Song
> <songxiaowei at hisilicon.com>; Binghui Wang <wangbinghui at hisilicon.com>;
> Thierry Reding <thierry.reding at gmail.com>; Jonathan Hunter
> <jonathanh at nvidia.com>; Thomas Petazzoni <thomas.petazzoni at bootlin.com>;
> Pali Rohár <pali at kernel.org>
> Subject: Re: [PATCH v3 4/5] PCI: imx6: Convert to agnostic GPIO API
> 
> On Mon, Apr 29, 2024 at 12:25 PM Andy Shevchenko
> <andriy.shevchenko at linux.intel.com> wrote:
> 
> > The of_gpio.h is going to be removed. In preparation of that convert
> > the driver to the agnostic API.
> >
> > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam at linaro.org>
> > Reviewed-by: Frank Li <Frank.Li at nxp.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> 
> I think there is a bug here, the code is respecting the OF property
> "reset-gpio-active-high"
Yes, you're right.
As I remember that this property is used for the buggy hardware design.
In general implementation, the PERST# is active low.
In pci_imx6 driver, it used to call the following callbacks to toggle perst#
 gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);
 msleep(100);
 gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);

But, some buggy hardware designs use this GPIO signal active high.
And the correct toggle sequence for thos board is reversed.
 gpio_set_value_cansleep(imx6_pcie->reset_gpio, 1);
 msleep(100);
 gpio_set_value_cansleep(imx6_pcie->reset_gpio, 0);

So, this property is added for those buggy hardware designs.

Best Regards
Richard Zhu
> but the code in drivers/gpio/gpiolib-of.h actually has a quirk for this so you can
> just delete all the active high handling and rely on 1 = asserted and 0 = deasserted
> when using GPIO descriptors.
> 
> Just delete this thing:
> imx6_pcie->gpio_active_high = of_property_read_bool(node,
>                                            "reset-gpio-active-high");
> 
> Yours,
> Linus Walleij