[v2] gpiolib: tighten up ACPI legacy gpio lookups
diff mbox

Message ID 20151111194530.GA26559@dtor-ws
State New
Headers show

Commit Message

Dmitry Torokhov Nov. 11, 2015, 7:45 p.m. UTC
We should not fall back to the legacy unnamed gpio lookup style if the
driver requests gpios with different names, because we'll give out the same
gpio twice. Let's keep track of the names that were used for the device and
only do the fallback for the first name used.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

v1: incorporated changes suggested by Mika Westerberg in response to the
draft patch I posted in Goodix thread.

v2: moved acpi_can_fallback_to_crs body to drivers/gpio/gpiolib-acpi.c
and provided stub for !CONFIG_ACPI case to fix build error if ACPI is
disabled.

 drivers/gpio/gpiolib-acpi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpio/gpiolib.c      |  3 +++
 drivers/gpio/gpiolib.h      |  8 ++++++++
 3 files changed, 54 insertions(+)

Comments

Mika Westerberg Nov. 12, 2015, 9:02 a.m. UTC | #1
On Wed, Nov 11, 2015 at 11:45:30AM -0800, Dmitry Torokhov wrote:
> We should not fall back to the legacy unnamed gpio lookup style if the
> driver requests gpios with different names, because we'll give out the same
> gpio twice. Let's keep track of the names that were used for the device and
> only do the fallback for the first name used.
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Acked-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
Linus Walleij Nov. 17, 2015, 1:43 p.m. UTC | #2
On Wed, Nov 11, 2015 at 8:45 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

> We should not fall back to the legacy unnamed gpio lookup style if the
> driver requests gpios with different names, because we'll give out the same
> gpio twice. Let's keep track of the names that were used for the device and
> only do the fallback for the first name used.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>
> v1: incorporated changes suggested by Mika Westerberg in response to the
> draft patch I posted in Goodix thread.
>
> v2: moved acpi_can_fallback_to_crs body to drivers/gpio/gpiolib-acpi.c
> and provided stub for !CONFIG_ACPI case to fix build error if ACPI is
> disabled.

Patch applied with Mika's ACK.

Had to fuzz a bit to make it apply, hope nothing breaks.

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
Dmitry Torokhov Nov. 27, 2015, 5:37 p.m. UTC | #3
On Tue, Nov 17, 2015 at 02:43:14PM +0100, Linus Walleij wrote:
> On Wed, Nov 11, 2015 at 8:45 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> 
> > We should not fall back to the legacy unnamed gpio lookup style if the
> > driver requests gpios with different names, because we'll give out the same
> > gpio twice. Let's keep track of the names that were used for the device and
> > only do the fallback for the first name used.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >
> > v1: incorporated changes suggested by Mika Westerberg in response to the
> > draft patch I posted in Goodix thread.
> >
> > v2: moved acpi_can_fallback_to_crs body to drivers/gpio/gpiolib-acpi.c
> > and provided stub for !CONFIG_ACPI case to fix build error if ACPI is
> > disabled.
> 
> Patch applied with Mika's ACK.
> 
> Had to fuzz a bit to make it apply, hope nothing breaks.

Hmm, I'd like to get this patch into my tree as well to apply Irina's
goodix pieces on top to make sure they do not break. Would it be OK for
me to just apply the patch and hope git will sort it out when we merge
into next/mainline, you maybe you could prepare a 4.3-based branch with
the patch for me to pull?

Thanks!
Linus Walleij Dec. 9, 2015, 3:11 p.m. UTC | #4
Hi Dmitry,

Sorry for me being so damned slow.

On Fri, Nov 27, 2015 at 6:37 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Tue, Nov 17, 2015 at 02:43:14PM +0100, Linus Walleij wrote:
>> On Wed, Nov 11, 2015 at 8:45 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>
>> > We should not fall back to the legacy unnamed gpio lookup style if the
>> > driver requests gpios with different names, because we'll give out the same
>> > gpio twice. Let's keep track of the names that were used for the device and
>> > only do the fallback for the first name used.
>> >
>> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > ---
>> >
>> > v1: incorporated changes suggested by Mika Westerberg in response to the
>> > draft patch I posted in Goodix thread.
>> >
>> > v2: moved acpi_can_fallback_to_crs body to drivers/gpio/gpiolib-acpi.c
>> > and provided stub for !CONFIG_ACPI case to fix build error if ACPI is
>> > disabled.
>>
>> Patch applied with Mika's ACK.
>>
>> Had to fuzz a bit to make it apply, hope nothing breaks.
>
> Hmm, I'd like to get this patch into my tree as well to apply Irina's
> goodix pieces on top to make sure they do not break. Would it be OK for
> me to just apply the patch and hope git will sort it out when we merge
> into next/mainline, you maybe you could prepare a 4.3-based branch with
> the patch for me to pull?

I had to fuzz the patch so then you need to get the version I applied out
of my tree.

But I'm not going to rebase the tree so you can pull commit
9c3c9bc9cc980d8981f75109f3921576daf75723
from the GPIO tree if you like. Or I can make a tag there if
you prefer, just tell me.

I'm stacking a loooot of GPIO patches on top, so I prefer not to have
any conflicts.

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
Dmitry Torokhov Dec. 9, 2015, 5:21 p.m. UTC | #5
On Wed, Dec 09, 2015 at 04:11:30PM +0100, Linus Walleij wrote:
> Hi Dmitry,
> 
> Sorry for me being so damned slow.

Heh, I am not the one to complain about people being slow ;)

> 
> On Fri, Nov 27, 2015 at 6:37 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Tue, Nov 17, 2015 at 02:43:14PM +0100, Linus Walleij wrote:
> >> On Wed, Nov 11, 2015 at 8:45 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >>
> >> > We should not fall back to the legacy unnamed gpio lookup style if the
> >> > driver requests gpios with different names, because we'll give out the same
> >> > gpio twice. Let's keep track of the names that were used for the device and
> >> > only do the fallback for the first name used.
> >> >
> >> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> > ---
> >> >
> >> > v1: incorporated changes suggested by Mika Westerberg in response to the
> >> > draft patch I posted in Goodix thread.
> >> >
> >> > v2: moved acpi_can_fallback_to_crs body to drivers/gpio/gpiolib-acpi.c
> >> > and provided stub for !CONFIG_ACPI case to fix build error if ACPI is
> >> > disabled.
> >>
> >> Patch applied with Mika's ACK.
> >>
> >> Had to fuzz a bit to make it apply, hope nothing breaks.
> >
> > Hmm, I'd like to get this patch into my tree as well to apply Irina's
> > goodix pieces on top to make sure they do not break. Would it be OK for
> > me to just apply the patch and hope git will sort it out when we merge
> > into next/mainline, you maybe you could prepare a 4.3-based branch with
> > the patch for me to pull?
> 
> I had to fuzz the patch so then you need to get the version I applied out
> of my tree.
> 
> But I'm not going to rebase the tree so you can pull commit
> 9c3c9bc9cc980d8981f75109f3921576daf75723
> from the GPIO tree if you like. Or I can make a tag there if
> you prefer, just tell me.

Both would mean I will end up with parts of your work-in-progress queue,
which I would rather avoid. If you create a branch (lets call it A) off
v4.3 and apply that one patch there and merge branch A it into your main
branch (even though you already have a version of the same patch in your
main branch) and I will merge that branch A into my tree then we are
guaranteed not to have conflicts as git will resolve everything nicely.

Thanks.
Linus Walleij Dec. 13, 2015, 4:37 p.m. UTC | #6
On Wed, Dec 9, 2015 at 6:21 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:

>> > Hmm, I'd like to get this patch into my tree as well to apply Irina's
>> > goodix pieces on top to make sure they do not break. Would it be OK for
>> > me to just apply the patch and hope git will sort it out when we merge
>> > into next/mainline, you maybe you could prepare a 4.3-based branch with
>> > the patch for me to pull?
>>
>> I had to fuzz the patch so then you need to get the version I applied out
>> of my tree.
>>
>> But I'm not going to rebase the tree so you can pull commit
>> 9c3c9bc9cc980d8981f75109f3921576daf75723
>> from the GPIO tree if you like. Or I can make a tag there if
>> you prefer, just tell me.
>
> Both would mean I will end up with parts of your work-in-progress queue,
> which I would rather avoid. If you create a branch (lets call it A) off
> v4.3 and apply that one patch there and merge branch A it into your main
> branch (even though you already have a version of the same patch in your
> main branch) and I will merge that branch A into my tree then we are
> guaranteed not to have conflicts as git will resolve everything nicely.

I can't do that, as this patch is not applying cleanly on v4.3.

It applies cleanly to v4.4-rc1 though, would you like an immutable
branch like such based on v4.4-rc1 instead?

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
Dmitry Torokhov Dec. 13, 2015, 4:44 p.m. UTC | #7
On Sun, Dec 13, 2015 at 8:37 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Dec 9, 2015 at 6:21 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>
>>> > Hmm, I'd like to get this patch into my tree as well to apply Irina's
>>> > goodix pieces on top to make sure they do not break. Would it be OK for
>>> > me to just apply the patch and hope git will sort it out when we merge
>>> > into next/mainline, you maybe you could prepare a 4.3-based branch with
>>> > the patch for me to pull?
>>>
>>> I had to fuzz the patch so then you need to get the version I applied out
>>> of my tree.
>>>
>>> But I'm not going to rebase the tree so you can pull commit
>>> 9c3c9bc9cc980d8981f75109f3921576daf75723
>>> from the GPIO tree if you like. Or I can make a tag there if
>>> you prefer, just tell me.
>>
>> Both would mean I will end up with parts of your work-in-progress queue,
>> which I would rather avoid. If you create a branch (lets call it A) off
>> v4.3 and apply that one patch there and merge branch A it into your main
>> branch (even though you already have a version of the same patch in your
>> main branch) and I will merge that branch A into my tree then we are
>> guaranteed not to have conflicts as git will resolve everything nicely.
>
> I can't do that, as this patch is not applying cleanly on v4.3.
>
> It applies cleanly to v4.4-rc1 though, would you like an immutable
> branch like such based on v4.4-rc1 instead?

Yep, that would work too.

Thank you!
Linus Walleij Dec. 15, 2015, 1:24 p.m. UTC | #8
On Sun, Dec 13, 2015 at 5:44 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Sun, Dec 13, 2015 at 8:37 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Wed, Dec 9, 2015 at 6:21 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>
>>>> > Hmm, I'd like to get this patch into my tree as well to apply Irina's
>>>> > goodix pieces on top to make sure they do not break. Would it be OK for
>>>> > me to just apply the patch and hope git will sort it out when we merge
>>>> > into next/mainline, you maybe you could prepare a 4.3-based branch with
>>>> > the patch for me to pull?
>>>>
>>>> I had to fuzz the patch so then you need to get the version I applied out
>>>> of my tree.
>>>>
>>>> But I'm not going to rebase the tree so you can pull commit
>>>> 9c3c9bc9cc980d8981f75109f3921576daf75723
>>>> from the GPIO tree if you like. Or I can make a tag there if
>>>> you prefer, just tell me.
>>>
>>> Both would mean I will end up with parts of your work-in-progress queue,
>>> which I would rather avoid. If you create a branch (lets call it A) off
>>> v4.3 and apply that one patch there and merge branch A it into your main
>>> branch (even though you already have a version of the same patch in your
>>> main branch) and I will merge that branch A into my tree then we are
>>> guaranteed not to have conflicts as git will resolve everything nicely.
>>
>> I can't do that, as this patch is not applying cleanly on v4.3.
>>
>> It applies cleanly to v4.4-rc1 though, would you like an immutable
>> branch like such based on v4.4-rc1 instead?
>
> Yep, that would work too.

OK here is a for-dmitry branch:
https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=for-dmitry

Absolutely nothing happens when I merge it to my devel branch so I guess
git will cope, but whatever, this one is safe :)

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
Dmitry Torokhov Dec. 15, 2015, 6:47 p.m. UTC | #9
On Tue, Dec 15, 2015 at 02:24:05PM +0100, Linus Walleij wrote:
> On Sun, Dec 13, 2015 at 5:44 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Sun, Dec 13, 2015 at 8:37 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> >> On Wed, Dec 9, 2015 at 6:21 PM, Dmitry Torokhov
> >> <dmitry.torokhov@gmail.com> wrote:
> >>
> >>>> > Hmm, I'd like to get this patch into my tree as well to apply Irina's
> >>>> > goodix pieces on top to make sure they do not break. Would it be OK for
> >>>> > me to just apply the patch and hope git will sort it out when we merge
> >>>> > into next/mainline, you maybe you could prepare a 4.3-based branch with
> >>>> > the patch for me to pull?
> >>>>
> >>>> I had to fuzz the patch so then you need to get the version I applied out
> >>>> of my tree.
> >>>>
> >>>> But I'm not going to rebase the tree so you can pull commit
> >>>> 9c3c9bc9cc980d8981f75109f3921576daf75723
> >>>> from the GPIO tree if you like. Or I can make a tag there if
> >>>> you prefer, just tell me.
> >>>
> >>> Both would mean I will end up with parts of your work-in-progress queue,
> >>> which I would rather avoid. If you create a branch (lets call it A) off
> >>> v4.3 and apply that one patch there and merge branch A it into your main
> >>> branch (even though you already have a version of the same patch in your
> >>> main branch) and I will merge that branch A into my tree then we are
> >>> guaranteed not to have conflicts as git will resolve everything nicely.
> >>
> >> I can't do that, as this patch is not applying cleanly on v4.3.
> >>
> >> It applies cleanly to v4.4-rc1 though, would you like an immutable
> >> branch like such based on v4.4-rc1 instead?
> >
> > Yep, that would work too.
> 
> OK here is a for-dmitry branch:
> https://git.kernel.org/cgit/linux/kernel/git/linusw/linux-gpio.git/log/?h=for-dmitry
> 
> Absolutely nothing happens when I merge it to my devel branch so I guess
> git will cope, but whatever, this one is safe :)

Thank you Linus!

Patch
diff mbox

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 143a9bd..d5f3008 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -838,3 +838,46 @@  int acpi_gpio_count(struct device *dev, const char *con_id)
 	}
 	return count;
 }
+
+struct acpi_crs_lookup {
+	struct list_head node;
+	struct acpi_device *adev;
+	const char *con_id;
+};
+
+static DEFINE_MUTEX(acpi_crs_lookup_lock);
+static LIST_HEAD(acpi_crs_lookup_list);
+
+bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id)
+{
+	struct acpi_crs_lookup *l, *lookup = NULL;
+
+	/* Never allow fallback if the device has properties */
+	if (adev->data.properties || adev->driver_gpios)
+		return false;
+
+	mutex_lock(&acpi_crs_lookup_lock);
+
+	list_for_each_entry(l, &acpi_crs_lookup_list, node) {
+		if (l->adev == adev) {
+			lookup = l;
+			break;
+		}
+	}
+
+	if (!lookup) {
+		lookup = kmalloc(sizeof(*lookup), GFP_KERNEL);
+		if (lookup) {
+			lookup->adev = adev;
+			lookup->con_id = con_id;
+			list_add_tail(&lookup->node, &acpi_crs_lookup_list);
+		}
+	}
+
+	mutex_unlock(&acpi_crs_lookup_lock);
+
+	return lookup &&
+		((!lookup->con_id && !con_id) ||
+		 (lookup->con_id && con_id &&
+		  strcmp(lookup->con_id, con_id) == 0));
+}
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 5db3445..0faf4b7 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -1765,6 +1765,9 @@  static struct gpio_desc *acpi_find_gpio(struct device *dev, const char *con_id,
 
 	/* Then from plain _CRS GPIOs */
 	if (IS_ERR(desc)) {
+		if (!acpi_can_fallback_to_crs(adev, con_id))
+			return ERR_PTR(-ENOENT);
+
 		desc = acpi_get_gpiod_by_index(adev, NULL, idx, &info);
 		if (IS_ERR(desc))
 			return desc;
diff --git a/drivers/gpio/gpiolib.h b/drivers/gpio/gpiolib.h
index bf34300..986eb5c 100644
--- a/drivers/gpio/gpiolib.h
+++ b/drivers/gpio/gpiolib.h
@@ -44,6 +44,8 @@  struct gpio_desc *acpi_get_gpiod_by_index(struct acpi_device *adev,
 					  struct acpi_gpio_info *info);
 
 int acpi_gpio_count(struct device *dev, const char *con_id);
+
+bool acpi_can_fallback_to_crs(struct acpi_device *adev, const char *con_id);
 #else
 static inline void acpi_gpiochip_add(struct gpio_chip *chip) { }
 static inline void acpi_gpiochip_remove(struct gpio_chip *chip) { }
@@ -65,6 +67,12 @@  static inline int acpi_gpio_count(struct device *dev, const char *con_id)
 {
 	return -ENODEV;
 }
+
+static inline bool acpi_can_fallback_to_crs(struct acpi_device *adev,
+					    const char *con_id)
+{
+	return false;
+}
 #endif
 
 struct gpio_desc *of_get_named_gpiod_flags(struct device_node *np,