Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Jonathan Cameron]

On Fri, 3 Nov 2017 15:03:36 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> The commit 0f0796509c07
> 
> ("iio: remove gpio interrupt probing from drivers that use a single
> interrupt")
> 
> removed custom IRQ assignment for the drivers which are enumerated via
> ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
> GpioIo() resource and thus automatic IRQ allocation will fail.

I'll ask the obvious question - is this allowed under the ACPI spec?

> 
> Partially revert the commit 0f0796509c07 to restore original
> behaviour.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

I really don't like scattering fixes for broken ACPI tables through
drivers...  Is there really no better solution to this?

On patches like this best to pull in ACPI and GPIO on the cc list.

Also cc'd Mika who made the original change to support gpioint.



Jonathan

> ---
>  drivers/iio/proximity/sx9500.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/iio/proximity/sx9500.c
> b/drivers/iio/proximity/sx9500.c index 53c5d653e780..df23dbcc030a
> 100644 --- a/drivers/iio/proximity/sx9500.c
> +++ b/drivers/iio/proximity/sx9500.c
> @@ -869,6 +869,7 @@ static int sx9500_init_device(struct iio_dev
> *indio_dev) static void sx9500_gpio_probe(struct i2c_client *client,
>  			      struct sx9500_data *data)
>  {
> +	struct gpio_desc *gpiod_int;
>  	struct device *dev;
>  
>  	if (!client)
> @@ -876,6 +877,14 @@ static void sx9500_gpio_probe(struct i2c_client
> *client, 
>  	dev = &client->dev;
>  
> +	if (client->irq <= 0) {
> +		gpiod_int = devm_gpiod_get(dev, SX9500_GPIO_INT,
> GPIOD_IN);
> +		if (IS_ERR(gpiod_int))
> +			dev_err(dev, "gpio get irq failed\n");
> +		else
> +			client->irq = gpiod_to_irq(gpiod_int);
> +	}
> +
>  	data->gpiod_rst = devm_gpiod_get(dev, SX9500_GPIO_RESET,
> GPIOD_OUT_HIGH); if (IS_ERR(data->gpiod_rst)) {
>  		dev_warn(dev, "gpio get reset pin failed\n");

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Linus Walleij]

On Sat, Nov 4, 2017 at 4:11 AM, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
> On Fri, 3 Nov 2017 15:03:36 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
>> The commit 0f0796509c07
>>
>> ("iio: remove gpio interrupt probing from drivers that use a single
>> interrupt")
>>
>> removed custom IRQ assignment for the drivers which are enumerated via
>> ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
>> GpioIo() resource and thus automatic IRQ allocation will fail.
>
> I'll ask the obvious question - is this allowed under the ACPI spec?
>
>>
>> Partially revert the commit 0f0796509c07 to restore original
>> behaviour.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> I really don't like scattering fixes for broken ACPI tables through
> drivers...  Is there really no better solution to this?
>
> On patches like this best to pull in ACPI and GPIO on the cc list.
>
> Also cc'd Mika who made the original change to support gpioint.

Andy and Mika are the maintainers of ACPI GPIO, I have only
superficial knowledge of how that actually works. But it's good if
Mika can look at it too.

The only thing that I know for sure about ACPI GPIO is the same
as always : the people who devised it think that they are releaving
the OS authors of a burden by taking stuff over, and end up
creating more work for us since they make mistakes and
deploy them in firmware that "cannot be fixed".

Linus Walleij

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Mika Westerberg]

On Sat, Nov 04, 2017 at 03:11:19AM +0000, Jonathan Cameron wrote:
> On Fri, 3 Nov 2017 15:03:36 +0200
> Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> 
> > The commit 0f0796509c07
> > 
> > ("iio: remove gpio interrupt probing from drivers that use a single
> > interrupt")
> > 
> > removed custom IRQ assignment for the drivers which are enumerated via
> > ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
> > GpioIo() resource and thus automatic IRQ allocation will fail.
> 
> I'll ask the obvious question - is this allowed under the ACPI spec?

Yes, it is perfectly fine.

> > Partially revert the commit 0f0796509c07 to restore original
> > behaviour.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> I really don't like scattering fixes for broken ACPI tables through
> drivers...  Is there really no better solution to this?

This is not about broken ACPI tables. We just currently have
"convenience" stuff in the kernel that translates trivial things like a
single ACPI GpioInt() resource directly to a device interrupt. If the
table has multiple GpioInt()s or uses GpioIo() then it is up to the
driver to handle device specific details.

> On patches like this best to pull in ACPI and GPIO on the cc list.
> 
> Also cc'd Mika who made the original change to support gpioint.

The patch looks fine to me,

Acked-by: Mika Westerberg <mika.westerberg-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Jonathan Cameron]

On Mon, 6 Nov 2017 11:35:56 +0200
Mika Westerberg <mika.westerberg@linux.intel.com> wrote:

> On Sat, Nov 04, 2017 at 03:11:19AM +0000, Jonathan Cameron wrote:
> > On Fri, 3 Nov 2017 15:03:36 +0200
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > The commit 0f0796509c07
> > > 
> > > ("iio: remove gpio interrupt probing from drivers that use a single
> > > interrupt")
> > > 
> > > removed custom IRQ assignment for the drivers which are enumerated via
> > > ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
> > > GpioIo() resource and thus automatic IRQ allocation will fail.  
> > 
> > I'll ask the obvious question - is this allowed under the ACPI spec?  
> 
> Yes, it is perfectly fine.

I'm unconvinced...

> 
> > > Partially revert the commit 0f0796509c07 to restore original
> > > behaviour.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> > 
> > I really don't like scattering fixes for broken ACPI tables through
> > drivers...  Is there really no better solution to this?  
> 
> This is not about broken ACPI tables. We just currently have
> "convenience" stuff in the kernel that translates trivial things like a
> single ACPI GpioInt() resource directly to a device interrupt. If the
> table has multiple GpioInt()s or uses GpioIo() then it is up to the
> driver to handle device specific details.

I agree on the multiple cases needing hanlding. 
What bothers me is that there is no guarantee at all that a GpioIo
can even do an interrupt.

(table 6.2.17 in the 6.1 spec for example makes it clear that we are
in a mess)  If it is a gpioio lots of the interrupt specific stuff
cannot be supplied (all the stuff in byte 7)

So as I read the ACPI specification any interrupt specified as GpioIO
is simply broken.

> 
> > On patches like this best to pull in ACPI and GPIO on the cc list.
> > 
> > Also cc'd Mika who made the original change to support gpioint.  
> 
> The patch looks fine to me,
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

I'll probably take it anyway to support the platforms doing this particular
piece of fun as doubtlessly the chance of fixing the firmware is next
to nothing...

Jonathan
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Mika Westerberg]

On Sun, Nov 19, 2017 at 03:24:11PM +0000, Jonathan Cameron wrote:
> On Mon, 6 Nov 2017 11:35:56 +0200
> Mika Westerberg <mika.westerberg@linux.intel.com> wrote:
> 
> > On Sat, Nov 04, 2017 at 03:11:19AM +0000, Jonathan Cameron wrote:
> > > On Fri, 3 Nov 2017 15:03:36 +0200
> > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > >   
> > > > The commit 0f0796509c07
> > > > 
> > > > ("iio: remove gpio interrupt probing from drivers that use a single
> > > > interrupt")
> > > > 
> > > > removed custom IRQ assignment for the drivers which are enumerated via
> > > > ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
> > > > GpioIo() resource and thus automatic IRQ allocation will fail.  
> > > 
> > > I'll ask the obvious question - is this allowed under the ACPI spec?  
> > 
> > Yes, it is perfectly fine.
> 
> I'm unconvinced...
> 
> > 
> > > > Partially revert the commit 0f0796509c07 to restore original
> > > > behaviour.
> > > > 
> > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> > > 
> > > I really don't like scattering fixes for broken ACPI tables through
> > > drivers...  Is there really no better solution to this?  
> > 
> > This is not about broken ACPI tables. We just currently have
> > "convenience" stuff in the kernel that translates trivial things like a
> > single ACPI GpioInt() resource directly to a device interrupt. If the
> > table has multiple GpioInt()s or uses GpioIo() then it is up to the
> > driver to handle device specific details.
> 
> I agree on the multiple cases needing hanlding. 
> What bothers me is that there is no guarantee at all that a GpioIo
> can even do an interrupt.
> 
> (table 6.2.17 in the 6.1 spec for example makes it clear that we are
> in a mess)  If it is a gpioio lots of the interrupt specific stuff
> cannot be supplied (all the stuff in byte 7)
> 
> So as I read the ACPI specification any interrupt specified as GpioIO
> is simply broken.

Well, it is the same with DT if you just declare GPIOs as in "xxx-gpios"
but still use them to trigger interrupts.

Normally you would use GpioInt in ACPI case but sometimes there might
actually be need to use the GPIO as input/output without interrupts. I
remember there was some I2C connected touchpanel that required some
magic to be done when it was put to low power states. In those cases
GpioIo is more correct IMHO.

It is also possible that the BIOS people just messed this up.

> > > On patches like this best to pull in ACPI and GPIO on the cc list.
> > > 
> > > Also cc'd Mika who made the original change to support gpioint.  
> > 
> > The patch looks fine to me,
> > 
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> I'll probably take it anyway to support the platforms doing this particular
> piece of fun as doubtlessly the chance of fixing the firmware is next
> to nothing...

Thanks!

Re: [PATCH v3 1/5] iio: proximity: sx9500: Assign interrupt from GpioIo()

[Jonathan Cameron]

On Mon, 20 Nov 2017 12:30:27 +0200
Mika Westerberg <mika.westerberg@linux.intel.com> wrote:

> On Sun, Nov 19, 2017 at 03:24:11PM +0000, Jonathan Cameron wrote:
> > On Mon, 6 Nov 2017 11:35:56 +0200
> > Mika Westerberg <mika.westerberg@linux.intel.com> wrote:
> >   
> > > On Sat, Nov 04, 2017 at 03:11:19AM +0000, Jonathan Cameron wrote:  
> > > > On Fri, 3 Nov 2017 15:03:36 +0200
> > > > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > > >     
> > > > > The commit 0f0796509c07
> > > > > 
> > > > > ("iio: remove gpio interrupt probing from drivers that use a single
> > > > > interrupt")
> > > > > 
> > > > > removed custom IRQ assignment for the drivers which are enumerated via
> > > > > ACPI or OF. Unfortunately, some ACPI tables have IRQ line defined as
> > > > > GpioIo() resource and thus automatic IRQ allocation will fail.    
> > > > 
> > > > I'll ask the obvious question - is this allowed under the ACPI spec?    
> > > 
> > > Yes, it is perfectly fine.  
> > 
> > I'm unconvinced...
> >   
> > >   
> > > > > Partially revert the commit 0f0796509c07 to restore original
> > > > > behaviour.
> > > > > 
> > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>    
> > > > 
> > > > I really don't like scattering fixes for broken ACPI tables through
> > > > drivers...  Is there really no better solution to this?    
> > > 
> > > This is not about broken ACPI tables. We just currently have
> > > "convenience" stuff in the kernel that translates trivial things like a
> > > single ACPI GpioInt() resource directly to a device interrupt. If the
> > > table has multiple GpioInt()s or uses GpioIo() then it is up to the
> > > driver to handle device specific details.  
> > 
> > I agree on the multiple cases needing hanlding. 
> > What bothers me is that there is no guarantee at all that a GpioIo
> > can even do an interrupt.
> > 
> > (table 6.2.17 in the 6.1 spec for example makes it clear that we are
> > in a mess)  If it is a gpioio lots of the interrupt specific stuff
> > cannot be supplied (all the stuff in byte 7)
> > 
> > So as I read the ACPI specification any interrupt specified as GpioIO
> > is simply broken.  
> 
> Well, it is the same with DT if you just declare GPIOs as in "xxx-gpios"
> but still use them to trigger interrupts.
> 
> Normally you would use GpioInt in ACPI case but sometimes there might
> actually be need to use the GPIO as input/output without interrupts. I
> remember there was some I2C connected touchpanel that required some
> magic to be done when it was put to low power states. In those cases
> GpioIo is more correct IMHO.
> 
> It is also possible that the BIOS people just messed this up.
> 
> > > > On patches like this best to pull in ACPI and GPIO on the cc list.
> > > > 
> > > > Also cc'd Mika who made the original change to support gpioint.    
> > > 
> > > The patch looks fine to me,
> > > 
> > > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>  
> > 
> > I'll probably take it anyway to support the platforms doing this particular
> > piece of fun as doubtlessly the chance of fixing the firmware is next
> > to nothing...  
> 
> Thanks!
Not sure I actually replied to say I had applied this one to the fixes-togreg
branch of iio.git.  I certainly hadn't marked it as such locally and
just tried to apply it again ;)

Thanks,

Jonathan

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Jonathan Cameron]

On Fri, 3 Nov 2017 15:03:37 +0200
Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:

> In order to satisfy GPIO ACPI library requirements convert users of
> gpiod_get_index() to correctly behave when there no mapping is
> provided by firmware.
> 
> Here we add explicit mapping between _CRS GpioIo() resources and
> their names used in the driver.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

Added cc's as for previous patch.  I guess this makes sense if we
accept that fixes like the previous one should be in drivers at all.

If not the reset part still makes sense I suppose.

> ---
>  drivers/iio/proximity/sx9500.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/proximity/sx9500.c
> b/drivers/iio/proximity/sx9500.c index df23dbcc030a..eb687b3dd442
> 100644 --- a/drivers/iio/proximity/sx9500.c
> +++ b/drivers/iio/proximity/sx9500.c
> @@ -32,9 +32,6 @@
>  #define SX9500_DRIVER_NAME		"sx9500"
>  #define SX9500_IRQ_NAME			"sx9500_event"
>  
> -#define SX9500_GPIO_INT			"interrupt"
> -#define SX9500_GPIO_RESET		"reset"
> -
>  /* Register definitions. */
>  #define SX9500_REG_IRQ_SRC		0x00
>  #define SX9500_REG_STAT			0x01
> @@ -866,26 +863,40 @@ static int sx9500_init_device(struct iio_dev
> *indio_dev) return sx9500_init_compensation(indio_dev);
>  }
>  
> +static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> +static const struct acpi_gpio_params interrupt_gpios = { 2, 0,
> false }; +
> +static const struct acpi_gpio_mapping acpi_sx9500_gpios[] = {
> +	{ "reset-gpios", &reset_gpios, 1 },
> +	{ "interrupt-gpios", &interrupt_gpios, 1 },
> +	{ },
> +};
> +
>  static void sx9500_gpio_probe(struct i2c_client *client,
>  			      struct sx9500_data *data)
>  {
>  	struct gpio_desc *gpiod_int;
>  	struct device *dev;
> +	int ret;
>  
>  	if (!client)
>  		return;
>  
>  	dev = &client->dev;
>  
> +	ret = devm_acpi_dev_add_driver_gpios(dev, acpi_sx9500_gpios);
> +	if (ret)
> +		dev_dbg(dev, "Unable to add GPIO mapping table\n");
> +
>  	if (client->irq <= 0) {
> -		gpiod_int = devm_gpiod_get(dev, SX9500_GPIO_INT,
> GPIOD_IN);
> +		gpiod_int = devm_gpiod_get(dev, "interrupt",
> GPIOD_IN); if (IS_ERR(gpiod_int))
>  			dev_err(dev, "gpio get irq failed\n");
>  		else
>  			client->irq = gpiod_to_irq(gpiod_int);
>  	}
>  
> -	data->gpiod_rst = devm_gpiod_get(dev, SX9500_GPIO_RESET,
> GPIOD_OUT_HIGH);
> +	data->gpiod_rst = devm_gpiod_get(dev, "reset",
> GPIOD_OUT_HIGH); if (IS_ERR(data->gpiod_rst)) {
>  		dev_warn(dev, "gpio get reset pin failed\n");
>  		data->gpiod_rst = NULL;

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Jonathan Cameron]

On Sat, 4 Nov 2017 03:14:27 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Fri, 3 Nov 2017 15:03:37 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > In order to satisfy GPIO ACPI library requirements convert users of
> > gpiod_get_index() to correctly behave when there no mapping is
> > provided by firmware.
> > 
> > Here we add explicit mapping between _CRS GpioIo() resources and
> > their names used in the driver.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>  
> 
> Added cc's as for previous patch.  I guess this makes sense if we
> accept that fixes like the previous one should be in drivers at all.
> 
> If not the reset part still makes sense I suppose.
So, what this description is missing:
* Is this a fix for known problem?
* If so please add a fixes tag.
* If it is because we now have platforms that need this then say that.

I have no idea on the urgency of this patch from the description.

Jonathan
> 
> > ---
> >  drivers/iio/proximity/sx9500.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/iio/proximity/sx9500.c
> > b/drivers/iio/proximity/sx9500.c index df23dbcc030a..eb687b3dd442
> > 100644 --- a/drivers/iio/proximity/sx9500.c
> > +++ b/drivers/iio/proximity/sx9500.c
> > @@ -32,9 +32,6 @@
> >  #define SX9500_DRIVER_NAME		"sx9500"
> >  #define SX9500_IRQ_NAME			"sx9500_event"
> >  
> > -#define SX9500_GPIO_INT			"interrupt"
> > -#define SX9500_GPIO_RESET		"reset"
> > -
> >  /* Register definitions. */
> >  #define SX9500_REG_IRQ_SRC		0x00
> >  #define SX9500_REG_STAT			0x01
> > @@ -866,26 +863,40 @@ static int sx9500_init_device(struct iio_dev
> > *indio_dev) return sx9500_init_compensation(indio_dev);
> >  }
> >  
> > +static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> > +static const struct acpi_gpio_params interrupt_gpios = { 2, 0,
> > false }; +
> > +static const struct acpi_gpio_mapping acpi_sx9500_gpios[] = {
> > +	{ "reset-gpios", &reset_gpios, 1 },
> > +	{ "interrupt-gpios", &interrupt_gpios, 1 },
> > +	{ },
> > +};
> > +
> >  static void sx9500_gpio_probe(struct i2c_client *client,
> >  			      struct sx9500_data *data)
> >  {
> >  	struct gpio_desc *gpiod_int;
> >  	struct device *dev;
> > +	int ret;
> >  
> >  	if (!client)
> >  		return;
> >  
> >  	dev = &client->dev;
> >  
> > +	ret = devm_acpi_dev_add_driver_gpios(dev, acpi_sx9500_gpios);
> > +	if (ret)
> > +		dev_dbg(dev, "Unable to add GPIO mapping table\n");
> > +
> >  	if (client->irq <= 0) {
> > -		gpiod_int = devm_gpiod_get(dev, SX9500_GPIO_INT,
> > GPIOD_IN);
> > +		gpiod_int = devm_gpiod_get(dev, "interrupt",
> > GPIOD_IN); if (IS_ERR(gpiod_int))
> >  			dev_err(dev, "gpio get irq failed\n");
> >  		else
> >  			client->irq = gpiod_to_irq(gpiod_int);
> >  	}
> >  
> > -	data->gpiod_rst = devm_gpiod_get(dev, SX9500_GPIO_RESET,
> > GPIOD_OUT_HIGH);
> > +	data->gpiod_rst = devm_gpiod_get(dev, "reset",
> > GPIOD_OUT_HIGH); if (IS_ERR(data->gpiod_rst)) {
> >  		dev_warn(dev, "gpio get reset pin failed\n");
> >  		data->gpiod_rst = NULL;  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Jonathan Cameron]

On Sun, 19 Nov 2017 15:29:34 +0000
Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> On Sat, 4 Nov 2017 03:14:27 +0000
> Jonathan Cameron <Jonathan.Cameron-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> 
> > On Fri, 3 Nov 2017 15:03:37 +0200
> > Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> >   
> > > In order to satisfy GPIO ACPI library requirements convert users of
> > > gpiod_get_index() to correctly behave when there no mapping is
> > > provided by firmware.
> > > 
> > > Here we add explicit mapping between _CRS GpioIo() resources and
> > > their names used in the driver.
> > > 
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>    
> > 
> > Added cc's as for previous patch.  I guess this makes sense if we
> > accept that fixes like the previous one should be in drivers at all.
> > 
> > If not the reset part still makes sense I suppose.  
> So, what this description is missing:
> * Is this a fix for known problem?
> * If so please add a fixes tag.
> * If it is because we now have platforms that need this then say that.
> 
> I have no idea on the urgency of this patch from the description.
> 
Hi Andy, any updates on the above?   I probably won't be sending
a fixes pull until next weekend, but would be good to know if this
should be in there or not by then!

> Jonathan
> >   
> > > ---
> > >  drivers/iio/proximity/sx9500.c | 21 ++++++++++++++++-----
> > >  1 file changed, 16 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/iio/proximity/sx9500.c
> > > b/drivers/iio/proximity/sx9500.c index df23dbcc030a..eb687b3dd442
> > > 100644 --- a/drivers/iio/proximity/sx9500.c
> > > +++ b/drivers/iio/proximity/sx9500.c
> > > @@ -32,9 +32,6 @@
> > >  #define SX9500_DRIVER_NAME		"sx9500"
> > >  #define SX9500_IRQ_NAME			"sx9500_event"
> > >  
> > > -#define SX9500_GPIO_INT			"interrupt"
> > > -#define SX9500_GPIO_RESET		"reset"
> > > -
> > >  /* Register definitions. */
> > >  #define SX9500_REG_IRQ_SRC		0x00
> > >  #define SX9500_REG_STAT			0x01
> > > @@ -866,26 +863,40 @@ static int sx9500_init_device(struct iio_dev
> > > *indio_dev) return sx9500_init_compensation(indio_dev);
> > >  }
> > >  
> > > +static const struct acpi_gpio_params reset_gpios = { 0, 0, false };
> > > +static const struct acpi_gpio_params interrupt_gpios = { 2, 0,
> > > false }; +
> > > +static const struct acpi_gpio_mapping acpi_sx9500_gpios[] = {
> > > +	{ "reset-gpios", &reset_gpios, 1 },
> > > +	{ "interrupt-gpios", &interrupt_gpios, 1 },
> > > +	{ },
> > > +};
> > > +
> > >  static void sx9500_gpio_probe(struct i2c_client *client,
> > >  			      struct sx9500_data *data)
> > >  {
> > >  	struct gpio_desc *gpiod_int;
> > >  	struct device *dev;
> > > +	int ret;
> > >  
> > >  	if (!client)
> > >  		return;
> > >  
> > >  	dev = &client->dev;
> > >  
> > > +	ret = devm_acpi_dev_add_driver_gpios(dev, acpi_sx9500_gpios);
> > > +	if (ret)
> > > +		dev_dbg(dev, "Unable to add GPIO mapping table\n");
> > > +
> > >  	if (client->irq <= 0) {
> > > -		gpiod_int = devm_gpiod_get(dev, SX9500_GPIO_INT,
> > > GPIOD_IN);
> > > +		gpiod_int = devm_gpiod_get(dev, "interrupt",
> > > GPIOD_IN); if (IS_ERR(gpiod_int))
> > >  			dev_err(dev, "gpio get irq failed\n");
> > >  		else
> > >  			client->irq = gpiod_to_irq(gpiod_int);
> > >  	}
> > >  
> > > -	data->gpiod_rst = devm_gpiod_get(dev, SX9500_GPIO_RESET,
> > > GPIOD_OUT_HIGH);
> > > +	data->gpiod_rst = devm_gpiod_get(dev, "reset",
> > > GPIOD_OUT_HIGH); if (IS_ERR(data->gpiod_rst)) {
> > >  		dev_warn(dev, "gpio get reset pin failed\n");
> > >  		data->gpiod_rst = NULL;    
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Andy Shevchenko]

On Sat, 2017-11-25 at 14:24 +0000, Jonathan Cameron wrote:
> On Sun, 19 Nov 2017 15:29:34 +0000
> Jonathan Cameron <jic23@kernel.org> wrote:

> Hi Andy, any updates on the above?   I probably won't be sending
> a fixes pull until next weekend, but would be good to know if this
> should be in there or not by then!

Yes, as you suggested I tried to add quirks to the core, so, this patch
will be definitely changed.

Thus, let's postpone.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Linus Walleij]

On Mon, Nov 27, 2017 at 4:08 PM, Andy Shevchenko
<andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
> On Sat, 2017-11-25 at 14:24 +0000, Jonathan Cameron wrote:
>> On Sun, 19 Nov 2017 15:29:34 +0000
>> Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>> Hi Andy, any updates on the above?   I probably won't be sending
>> a fixes pull until next weekend, but would be good to know if this
>> should be in there or not by then!
>
> Yes, as you suggested I tried to add quirks to the core, so, this patch
> will be definitely changed.

The quirks mechanics are merged to the GPIO core and an
immutable branch is available, so Jonathan could "just" pull that in
and apply (elegent) fixes on top. I think.

Yours,
Linus Walleij

Re: [PATCH v3 2/5] iio: proximity: sx9500: Add GPIO ACPI mapping table

[Andy Shevchenko]

On Fri, 2017-12-01 at 11:04 +0100, Linus Walleij wrote:
> On Mon, Nov 27, 2017 at 4:08 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, 2017-11-25 at 14:24 +0000, Jonathan Cameron wrote:
> > > On Sun, 19 Nov 2017 15:29:34 +0000
> > > Jonathan Cameron <jic23@kernel.org> wrote:
> > > Hi Andy, any updates on the above?   I probably won't be sending
> > > a fixes pull until next weekend, but would be good to know if this
> > > should be in there or not by then!
> > 
> > Yes, as you suggested I tried to add quirks to the core, so, this
> > patch
> > will be definitely changed.
> 
> The quirks mechanics are merged to the GPIO core and an
> immutable branch is available, so Jonathan could "just" pull that in
> and apply (elegent) fixes on top. I think.

Unfortunately I didn't see any updates WRT GPIO/pin control in linux-next.
Did you synchronize repositories?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy