| Message ID | 20171110134033.85461-6-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/6] gpiolib: acpi: Assign polarity when call acpi_populate_gpio_lookup() | expand |
On Fri, Nov 10, 2017 at 03:40:33PM +0200, Andy Shevchenko wrote: > Allow to relax IoRestriction for certain cases. > > One of the use case is incorrectly cooked ACPI table where interrupt pin is > defined with GpioIo() macro with IoRestrictionOutputOnly. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> I think you should include user of this quirk in the patch series as well. Otherwise it is pretty pointless to add random quirks without real issues they are supposed to solve ;-) Anyway looks good to me, Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> -- 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
On Mon, 2017-11-13 at 13:55 +0200, Mika Westerberg wrote: > On Fri, Nov 10, 2017 at 03:40:33PM +0200, Andy Shevchenko wrote: > > Allow to relax IoRestriction for certain cases. > > > > One of the use case is incorrectly cooked ACPI table where interrupt > > pin is > > defined with GpioIo() macro with IoRestrictionOutputOnly. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > I think you should include user of this quirk in the patch series as > well. Otherwise it is pretty pointless to add random quirks without > real issues they are supposed to solve ;-) Something like below? --- a/drivers/extcon/extcon-intel-int3496.c +++ b/drivers/extcon/extcon-intel-int3496.c @@ -50,7 +50,7 @@ static const struct acpi_gpio_params vbus_gpios = { INT3496_GPIO_VBUS_EN, 0, fal static const struct acpi_gpio_params mux_gpios = { INT3496_GPIO_USB_MUX, 0, false }; static const struct acpi_gpio_mapping acpi_int3496_default_gpios[] = { - { "id-gpios", &id_gpios, 1 }, + { "id-gpios", &id_gpios, 1, ACPI_GPIO_QUIRK_NO_IO_RESTRICTION }, { "vbus-gpios", &vbus_gpios, 1 }, { "mux-gpios", &mux_gpios, 1 }, { }, @@ -112,9 +112,6 @@ static int int3496_probe(struct platform_device *pdev) ret = PTR_ERR(data->gpio_usb_id); dev_err(dev, "can't request USB ID GPIO: %d\n", ret); return ret; - } else if (gpiod_get_direction(data->gpio_usb_id) != GPIOF_DIR_IN) { - dev_warn(dev, FW_BUG "USB ID GPIO not in input mode, fixing\n"); - gpiod_direction_input(data->gpio_usb_id); } > > Anyway looks good to me, > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> Thanks!
On Mon, Nov 13, 2017 at 03:10:48PM +0200, Andy Shevchenko wrote: > On Mon, 2017-11-13 at 13:55 +0200, Mika Westerberg wrote: > > On Fri, Nov 10, 2017 at 03:40:33PM +0200, Andy Shevchenko wrote: > > > Allow to relax IoRestriction for certain cases. > > > > > > One of the use case is incorrectly cooked ACPI table where interrupt > > > pin is > > > defined with GpioIo() macro with IoRestrictionOutputOnly. > > > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > > I think you should include user of this quirk in the patch series as > > well. Otherwise it is pretty pointless to add random quirks without > > real issues they are supposed to solve ;-) > > Something like below? Yes, but I would also add comment on top of the added line explaining why this particular quirk is needed in this particular system. > --- a/drivers/extcon/extcon-intel-int3496.c > +++ b/drivers/extcon/extcon-intel-int3496.c > @@ -50,7 +50,7 @@ static const struct acpi_gpio_params vbus_gpios = { > INT3496_GPIO_VBUS_EN, 0, fal > static const struct acpi_gpio_params mux_gpios = { > INT3496_GPIO_USB_MUX, 0, false }; > > static const struct acpi_gpio_mapping acpi_int3496_default_gpios[] = { > - { "id-gpios", &id_gpios, 1 }, > + { "id-gpios", &id_gpios, 1, ACPI_GPIO_QUIRK_NO_IO_RESTRICTION }, > { "vbus-gpios", &vbus_gpios, 1 }, > { "mux-gpios", &mux_gpios, 1 }, > { }, > @@ -112,9 +112,6 @@ static int int3496_probe(struct platform_device > *pdev) > ret = PTR_ERR(data->gpio_usb_id); > dev_err(dev, "can't request USB ID GPIO: %d\n", ret); > return ret; > - } else if (gpiod_get_direction(data->gpio_usb_id) != > GPIOF_DIR_IN) { > - dev_warn(dev, FW_BUG "USB ID GPIO not in input mode, > fixing\n"); > - gpiod_direction_input(data->gpio_usb_id); > } > > > > > Anyway looks good to me, > > > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> > > Thanks! > > -- > Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Intel Finland Oy -- 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
On Fri, Nov 10, 2017 at 2:40 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Allow to relax IoRestriction for certain cases. > > One of the use case is incorrectly cooked ACPI table where interrupt pin is > defined with GpioIo() macro with IoRestrictionOutputOnly. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Patch applied with Mika's ACK. Yours, Linus Walleij -- 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
On Mon, Nov 13, 2017 at 2:10 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Mon, 2017-11-13 at 13:55 +0200, Mika Westerberg wrote: >> On Fri, Nov 10, 2017 at 03:40:33PM +0200, Andy Shevchenko wrote: >> > Allow to relax IoRestriction for certain cases. >> > >> > One of the use case is incorrectly cooked ACPI table where interrupt >> > pin is >> > defined with GpioIo() macro with IoRestrictionOutputOnly. >> > >> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> >> I think you should include user of this quirk in the patch series as >> well. Otherwise it is pretty pointless to add random quirks without >> real issues they are supposed to solve ;-) > > Something like below? I bet this patch is already somewhere in my mailbox else just send it separately. All these patches are applied. Yours, Linus Walleij -- 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
On Wed, 2017-11-29 at 13:35 +0100, Linus Walleij wrote: > On Mon, Nov 13, 2017 at 2:10 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Mon, 2017-11-13 at 13:55 +0200, Mika Westerberg wrote: > > > I think you should include user of this quirk in the patch series > > > as > > > well. Otherwise it is pretty pointless to add random quirks > > > without > > > real issues they are supposed to solve ;-) > > > > Something like below? > > I bet this patch is already somewhere in my mailbox else just send > it separately. All these patches are applied. I'm going to send them (there are two affected users) next week. It would be nice if you can prepare immutable branch in case the subsystems would like to merge patches via their respective trees. Thanks for applying!
On Wed, Nov 29, 2017 at 2:41 PM, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > It > would be nice if you can prepare immutable branch in case the subsystems > would like to merge patches via their respective trees. I have created a branch named "ib-gpio-acpi-quirks" in my tree with the six patches from you, then merged that to my devel branch. You can point subsystem maintainers to that one. Yours, Linus Walleij -- 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
On Thu, 2017-11-30 at 10:57 +0100, Linus Walleij wrote: > On Wed, Nov 29, 2017 at 2:41 PM, Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > It > > would be nice if you can prepare immutable branch in case the > > subsystems > > would like to merge patches via their respective trees. > > I have created a branch named "ib-gpio-acpi-quirks" in my tree > with the six patches from you, then merged that to my devel > branch. > > You can point subsystem maintainers to that one. > Thanks!
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 711f64b9dd30..430a1475212d 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -496,11 +496,18 @@ int acpi_gpio_update_gpiod_flags(enum gpiod_flags *flags, struct acpi_gpio_info *info) { struct device *dev = &info->adev->dev; + enum gpiod_flags old = *flags; int ret; - ret = __acpi_gpio_update_gpiod_flags(flags, info->flags); - if (ret) - dev_dbg(dev, "Override GPIO initialization flags\n"); + ret = __acpi_gpio_update_gpiod_flags(&old, info->flags); + if (info->quirks & ACPI_GPIO_QUIRK_NO_IO_RESTRICTION) { + if (ret) + dev_warn(dev, FW_BUG "GPIO not in correct mode, fixing\n"); + } else { + if (ret) + dev_dbg(dev, "Override GPIO initialization flags\n"); + *flags = old; + } return ret; } diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 25fe77fccea0..06b6eb775115 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -978,6 +978,10 @@ struct acpi_gpio_mapping { const char *name; const struct acpi_gpio_params *data; unsigned int size; + +/* Ignore IoRestriction field */ +#define ACPI_GPIO_QUIRK_NO_IO_RESTRICTION BIT(0) + unsigned int quirks; };
Allow to relax IoRestriction for certain cases. One of the use case is incorrectly cooked ACPI table where interrupt pin is defined with GpioIo() macro with IoRestrictionOutputOnly. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpiolib-acpi.c | 13 ++++++++++--- include/linux/acpi.h | 4 ++++ 2 files changed, 14 insertions(+), 3 deletions(-)