| Message ID | 20171103130340.42459-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. > --- > 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
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 -- 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
On Sun, 19 Nov 2017 15:29:34 +0000 Jonathan Cameron <jic23@kernel.org> wrote: > 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. > 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@vger.kernel.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@vger.kernel.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. Yours, Linus Walleij -- 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
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?
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;
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> --- drivers/iio/proximity/sx9500.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)