[v1] gpio: merrifield: Add support of ACPI enabled platforms

Message ID 20180103173240.45755-1-andriy.shevchenko@linux.intel.com
State New
Headers show
Series
  • [v1] gpio: merrifield: Add support of ACPI enabled platforms
Related show

Commit Message

Andy Shevchenko Jan. 3, 2018, 5:32 p.m.
The driver needs the pin control device name for ACPI.

We are looking through ACPI namespace and return first found device
based on ACPI HID for Intel Merrifield FLIS.

Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpio-merrifield.c | 43 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

Comments

Mika Westerberg Jan. 4, 2018, 9:36 a.m. | #1
On Wed, Jan 03, 2018 at 07:32:40PM +0200, Andy Shevchenko wrote:
> The driver needs the pin control device name for ACPI.
> 
> We are looking through ACPI namespace and return first found device
> based on ACPI HID for Intel Merrifield FLIS.
> 
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpio-merrifield.c | 43 +++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
> index dd67a31ac337..61e75d10669d 100644
> --- a/drivers/gpio/gpio-merrifield.c
> +++ b/drivers/gpio/gpio-merrifield.c
> @@ -9,6 +9,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/acpi.h>
>  #include <linux/bitops.h>
>  #include <linux/gpio/driver.h>
>  #include <linux/init.h>
> @@ -380,9 +381,48 @@ static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
>  	}
>  }
>  
> +#ifdef CONFIG_ACPI
> +static const struct acpi_device_id mrfld_pinctrl_acpi_ids[] = {
> +	{"INTC1002"},
> +	{}
> +};
> +
> +static acpi_status
> +mrfld_acpi_find_pinctrl(acpi_handle handle, u32 lvl, void *context, void **rv)
> +{
> +	const char **name = context;
> +	struct acpi_device *adev;
> +
> +	if (acpi_bus_get_device(handle, &adev))
> +		return AE_OK;
> +
> +	if (acpi_match_device_ids(adev, mrfld_pinctrl_acpi_ids))
> +		return AE_OK;
> +
> +	*name = dev_name(&adev->dev);
> +	return AE_CTRL_TERMINATE;
> +}
> +
> +static const char *mrfld_acpi_get_pinctrl_dev_name(const char *fallback)
> +{
> +	const char *pinctrl_dev_name = fallback;
> +
> +	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> +			    ACPI_UINT32_MAX, mrfld_acpi_find_pinctrl, NULL,
> +			    &pinctrl_dev_name, NULL);
> +	return pinctrl_dev_name;
> +}

I wonder if there is a better way to "connect" these things without need
to walk ACPI namespace in GPIO drivers? How does DT handle handle
separated GPIO and pinctrl drivers? Maybe there is a binding that we
could reuse in ACPI side.
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Shevchenko Jan. 4, 2018, 12:38 p.m. | #2
On Thu, 2018-01-04 at 11:36 +0200, Mika Westerberg wrote:
> On Wed, Jan 03, 2018 at 07:32:40PM +0200, Andy Shevchenko wrote:
> > The driver needs the pin control device name for ACPI.
> > 
> > We are looking through ACPI namespace and return first found device
> > based on ACPI HID for Intel Merrifield FLIS.

> > +static const struct acpi_device_id mrfld_pinctrl_acpi_ids[] = {
> > +	{"INTC1002"},
> > +	{}
> > +};

...

> > +static const char *mrfld_acpi_get_pinctrl_dev_name(const char
> > *fallback)
> > +{
> > +	const char *pinctrl_dev_name = fallback;
> > +
> > +	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> > +			    ACPI_UINT32_MAX,
> > mrfld_acpi_find_pinctrl, NULL,
> > +			    &pinctrl_dev_name, NULL);
> > +	return pinctrl_dev_name;
> > +}
> 
> I wonder if there is a better way to "connect" these things without
> need
> to walk ACPI namespace in GPIO drivers?

I would lovely use that if there is any.

>  How does DT handle handle
> separated GPIO and pinctrl drivers? Maybe there is a binding that we
> could reuse in ACPI side.

Do they need it?

The problem here is in GPIO <-> pin mapping which requires instance name
of the device to be supplied.

AFAIU (perhaps missed something) in DT pinctrl device directly has this
mapping in the DTS, so, no need to call gpiochip_add_pin_range().
Andy Shevchenko Jan. 4, 2018, 4:32 p.m. | #3
On Thu, 2018-01-04 at 14:38 +0200, Andy Shevchenko wrote:
> On Thu, 2018-01-04 at 11:36 +0200, Mika Westerberg wrote:
> > On Wed, Jan 03, 2018 at 07:32:40PM +0200, Andy Shevchenko wrote:
> > > The driver needs the pin control device name for ACPI.
> > > 
> > > We are looking through ACPI namespace and return first found
> > > device
> > > based on ACPI HID for Intel Merrifield FLIS.
> > > +static const struct acpi_device_id mrfld_pinctrl_acpi_ids[] = {
> > > +	{"INTC1002"},
> > > +	{}
> > > +};
> 
> ...
> 
> > > +static const char *mrfld_acpi_get_pinctrl_dev_name(const char
> > > *fallback)
> > > +{
> > > +	const char *pinctrl_dev_name = fallback;
> > > +
> > > +	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
> > > +			    ACPI_UINT32_MAX,
> > > mrfld_acpi_find_pinctrl, NULL,
> > > +			    &pinctrl_dev_name, NULL);
> > > +	return pinctrl_dev_name;
> > > +}
> > 
> > I wonder if there is a better way to "connect" these things without
> > need
> > to walk ACPI namespace in GPIO drivers?
> 
> I would lovely use that if there is any.

It appears there is another user of this, i.e. ASoC Intel, which needs
same.

Moreover, we can avoid scan entire ACPI namespace and narrow to scan
only registered ACPI devices on ACPI bus in Linux (see
acpi_dev_present() for example).

I'm about to send patch (series) v2 utilizing that.

Patch

diff --git a/drivers/gpio/gpio-merrifield.c b/drivers/gpio/gpio-merrifield.c
index dd67a31ac337..61e75d10669d 100644
--- a/drivers/gpio/gpio-merrifield.c
+++ b/drivers/gpio/gpio-merrifield.c
@@ -9,6 +9,7 @@ 
  * published by the Free Software Foundation.
  */
 
+#include <linux/acpi.h>
 #include <linux/bitops.h>
 #include <linux/gpio/driver.h>
 #include <linux/init.h>
@@ -380,9 +381,48 @@  static void mrfld_irq_init_hw(struct mrfld_gpio *priv)
 	}
 }
 
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id mrfld_pinctrl_acpi_ids[] = {
+	{"INTC1002"},
+	{}
+};
+
+static acpi_status
+mrfld_acpi_find_pinctrl(acpi_handle handle, u32 lvl, void *context, void **rv)
+{
+	const char **name = context;
+	struct acpi_device *adev;
+
+	if (acpi_bus_get_device(handle, &adev))
+		return AE_OK;
+
+	if (acpi_match_device_ids(adev, mrfld_pinctrl_acpi_ids))
+		return AE_OK;
+
+	*name = dev_name(&adev->dev);
+	return AE_CTRL_TERMINATE;
+}
+
+static const char *mrfld_acpi_get_pinctrl_dev_name(const char *fallback)
+{
+	const char *pinctrl_dev_name = fallback;
+
+	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+			    ACPI_UINT32_MAX, mrfld_acpi_find_pinctrl, NULL,
+			    &pinctrl_dev_name, NULL);
+	return pinctrl_dev_name;
+}
+#else
+static inline const char *mrfld_acpi_get_pinctrl_dev_name(const char *fallback)
+{
+	return fallback;
+}
+#endif
+
 static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	const struct mrfld_gpio_pinrange *range;
+	const char *pinctrl_dev_name;
 	struct mrfld_gpio *priv;
 	u32 gpio_base, irq_base;
 	void __iomem *base;
@@ -439,10 +479,11 @@  static int mrfld_gpio_probe(struct pci_dev *pdev, const struct pci_device_id *id
 		return retval;
 	}
 
+	pinctrl_dev_name = mrfld_acpi_get_pinctrl_dev_name("pinctrl-merrifield");
 	for (i = 0; i < ARRAY_SIZE(mrfld_gpio_ranges); i++) {
 		range = &mrfld_gpio_ranges[i];
 		retval = gpiochip_add_pin_range(&priv->chip,
-						"pinctrl-merrifield",
+						pinctrl_dev_name,
 						range->gpio_base,
 						range->pin_base,
 						range->npins);