[PATCH v1 25/43] x86: gpio: Add support for obtaining ACPI info for a GPIO

[Simon Glass]

Hi Wolfgang,

On Wed, 8 Jul 2020 at 05:06, Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Simon, Bin,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> > Betreff: Re: [PATCH v1 25/43] x86: gpio: Add support for obtaining ACPI info for a GPIO
> >
> > Hi Bin,
> >
> > On Tue, 30 Jun 2020 at 01:47, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Jun 15, 2020 at 11:58 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Implement the method that converts a GPIO into the form used by ACPI, so
> > > > that GPIOs can be added to ACPI tables.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > ---
> > > >
> > > > Changes in v1:
> > > > - Use acpi_get_path() to get device path
> > > >
> > > >  drivers/gpio/intel_gpio.c | 34 ++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 34 insertions(+)
> > > >
> > > > diff --git a/drivers/gpio/intel_gpio.c b/drivers/gpio/intel_gpio.c
> > > > index b4d5be97da..6a3a8c4cfa 100644
> > > > --- a/drivers/gpio/intel_gpio.c
> > > > +++ b/drivers/gpio/intel_gpio.c
> > > > @@ -12,6 +12,7 @@
> > > >  #include <pch.h>
> > > >  #include <pci.h>
> > > >  #include <syscon.h>
> > > > +#include <acpi/acpi_device.h>
> > > >  #include <asm/cpu.h>
> > > >  #include <asm/gpio.h>
> > > >  #include <asm/intel_pinctrl.h>
> > > > @@ -19,6 +20,7 @@
> > > >  #include <asm/io.h>
> > > >  #include <asm/pci.h>
> > > >  #include <asm/arch/gpio.h>
> > > > +#include <dm/acpi.h>
> > > >  #include <dt-bindings/gpio/x86-gpio.h>
> > > >
> > > >  static int intel_gpio_direction_input(struct udevice *dev, uint offset)
> > > > @@ -128,6 +130,35 @@ static int intel_gpio_xlate(struct udevice *orig_dev, struct gpio_desc *desc,
> > > >         return 0;
> > > >  }
> > > >
> > > > +#if CONFIG_IS_ENABLED(ACPIGEN)
> > > > +static int intel_gpio_get_acpi(const struct gpio_desc *desc,
> > > > +                              struct acpi_gpio *gpio)
> > > > +{
> > > > +       struct udevice *pinctrl;
> > > > +       int ret;
> > > > +
> > > > +       if (!dm_gpio_is_valid(desc))
> > > > +               return -ENOENT;
> > > > +       pinctrl = dev_get_parent(desc->dev);
> > > > +
> > > > +       memset(gpio, '\0', sizeof(*gpio));
> > > > +
> > > > +       gpio->type = ACPI_GPIO_TYPE_IO;
> > > > +       gpio->pull = ACPI_GPIO_PULL_DEFAULT;
> > > > +       gpio->io_restrict = ACPI_GPIO_IO_RESTRICT_OUTPUT;
> > > > +       gpio->polarity = ACPI_GPIO_ACTIVE_HIGH;
> > >
> > > Is there a way to figure out these properties from DT, instead of hardcoding?
> >
> > The answer is similar to your previous comment. But also, each pinctrl
> > driver has its own settings and limitations. If we want to support
> > different config for the pinctrl we would likely add it to the DT
> > binding for the pinctrl driver. So far I haven't seen a need but it
> > might happen with a future arch.
>
> Could gpio->pull and gpio->polarity be set depending on desc->flags?

To some extent, except that we don't have an equivalent to
ACPI_GPIO_PULL_DEFAULT.

>
> What is the reason for setting gpio->io_restrict to
> ACPI_GPIO_IO_RESTRICT_OUTPUT?

At present we only support outputs.

>
> > [..]
>
> regards, Wolfgang

Regards,
SImon