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

[Hongxing Zhu]

> -----Original Message-----
> From: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Sent: 2024年5月6日 20:10
> To: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>; Frank Li
> <frank.li@xxxxxxx>; Krzysztof Wilczyński <kwilczynski@xxxxxxxxxx>; Uwe
> Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>; linux-omap@xxxxxxxxxxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; imx@xxxxxxxxxxxxxxx;
> linux-amlogic@xxxxxxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx;
> linux-tegra@xxxxxxxxxxxxxxx; Vignesh Raghavendra <vigneshr@xxxxxx>;
> Siddharth Vadapalli <s-vadapalli@xxxxxx>; Lorenzo Pieralisi
> <lpieralisi@xxxxxxxxxx>; Krzysztof Wilczyński <kw@xxxxxxxxx>; Rob Herring
> <robh@xxxxxxxxxx>; Bjorn Helgaas <bhelgaas@xxxxxxxxxx>; Hongxing Zhu
> <hongxing.zhu@xxxxxxx>; Lucas Stach <l.stach@xxxxxxxxxxxxxx>; Shawn Guo
> <shawnguo@xxxxxxxxxx>; Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>; Pengutronix
> Kernel Team <kernel@xxxxxxxxxxxxxx>; Fabio Estevam <festevam@xxxxxxxxx>;
> Yue Wang <yue.wang@xxxxxxxxxxx>; Neil Armstrong
> <neil.armstrong@xxxxxxxxxx>; Kevin Hilman <khilman@xxxxxxxxxxxx>; Jerome
> Brunet <jbrunet@xxxxxxxxxxxx>; Martin Blumenstingl
> <martin.blumenstingl@xxxxxxxxxxxxxx>; Xiaowei Song
> <songxiaowei@xxxxxxxxxxxxx>; Binghui Wang <wangbinghui@xxxxxxxxxxxxx>;
> Thierry Reding <thierry.reding@xxxxxxxxx>; Jonathan Hunter
> <jonathanh@xxxxxxxxxx>; Thomas Petazzoni <thomas.petazzoni@xxxxxxxxxxx>;
> Pali Rohár <pali@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxx>
> > Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> 
> 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