Re: [PATCH] gpiolib-acpi: Set gpiod flags for ACPI GPIO resources based on pullup and polarity

["andriy.shevchenko@xxxxxxxxxxxxxxx" <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Mon, Dec 02, 2019 at 01:44:04PM +0000, Stanimir, Vasile-Laurentiu wrote:
> ________________________________________
> From: andriy.shevchenko@xxxxxxxxxxxxxxx [andriy.shevchenko@xxxxxxxxxxxxxxx]
> Sent: Monday, December 02, 2019 3:05 PM
> To: Stanimir, Vasile-Laurentiu
> Cc: linux-gpio@xxxxxxxxxxxxxxx; linus.walleij@xxxxxxxxxx; mika.westerberg@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] gpiolib-acpi: Set gpiod flags for ACPI GPIO resources based on pullup and polarity
> 
> On Mon, Dec 02, 2019 at 12:36:47PM +0000, Stanimir, Vasile-Laurentiu wrote:
> > From f8093f2c73c636b75fcf4dee4178af0e24c2f878 Mon Sep 17 00:00:00 2001
> > From: Vasile-Laurentiu Stanimir <vasile-laurentiu.stanimir@xxxxxxxxxxxxx>
> > Date: Mon, 2 Dec 2019 14:20:11 +0200
> > Subject: [PATCH] gpiolib-acpi: Set gpiod flags for ACPI GPIO resources based
> >  on pullup and polarity
> >
> > ACPI GPIO resources don't contain an initial value for the
> > GPIO. Therefore instead of deducting its value based on pullup field
> > we should deduce that value from the polarity and the pull field.
> > Typical scenario is when ACPI is defined in acpi-table and its polarity
> > is defined as ACTIVE-LOW in the following call:
> >
> > acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data)
> >   acpi_gpio_to_gpiod_flags(const struct acpi_resource_gpio *agpio)
> >
> > it will return GPIOD_OUT_HIGH if pull_up is set no matter if
> > polarity is GPIO_ACTIVE_LOW, so it will return the current level instead
> > of the logical level.
> 
> Thank you for the patch.
> 
> I have question in general. If we have Active Low polarity and Pull Down,
> isn't it simple a bad ACPI table and rather quirk is needed here?
> 
> --
> With Best Regards,
> Andy Shevchenko
> 
> 
> Hi, 
> 
> It may be, also it may be a bad hardware design but it is also a possible situation.
> 
> In our case here we have an FPGA whose pcie link is held in reset by BIOS during 
> boot, the reset pin is active low, and its configuration is specified in the Acpi DSDT 
> table. When Linux starts, our userspace driver shall load the FPGA, and the first 
> step is to request all GPIO's needed to configure the pcie phy on the FPGA side; 
> the pcie link reset should be held active while this configuration (loading of the 
> Altera fpp image) is ongoing.
> Now all active low pins have their initial value inverted by the kernel. This means 
> that the pcie link reset is briefly released, which generates a pcie hot unplug event, 
> which in turn delays start of a successful loading sequence, and so SW has to make 
> a second reload attempt which will delay too much the normal boot sequence.
> 
> Sorry for giving you too much details which probability are not important but I only
> wanted to emphasise that it may be a real situation whether or not it is a good
> design.

No need to sorry, the details are useful as well as knowing your use case.

Q1: who is providing DSDT?
Q2: why it can't be fixed?
Q3: how the GPIO lines are being requested?
Q4: how are they described, btw, in DSDT?

Side note, if it will be no other choice left, the above code still can be
added as a quirk (see code related to ACPI_GPIO_QUIRK_NO_IO_RESTRICTION
and ACPI_GPIO_QUIRK_ONLY_GPIOIO).

-- 
With Best Regards,
Andy Shevchenko