From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
Date: Wed, 8 Jul 2020 13:06:01 +0200
Subject: [PATCH v1 25/43] x86: gpio: Add support for obtaining ACPI info
 for a GPIO
In-Reply-To: <CAPnjgZ0yYYjJhzyWchChA4MJq54n4vTwaqDS_MfohC2C6KgZ3g@mail.gmail.com>
References: <CAPnjgZ0yYYjJhzyWchChA4MJq54n4vTwaqDS_MfohC2C6KgZ3g@mail.gmail.com>
Message-ID: <OFB497E8BD.5B1147AC-ONC125859F.003414B1-C125859F.003CF9FD@br-automation.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Simon, Bin,

-----"Simon Glass" <sjg@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@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Jun 15, 2020 at 11:58 AM Simon Glass <sjg@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@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?

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

> [..]

regards, Wolfgang