gpiolib: tighten up ACPI legacy gpio lookups

PATCH In review Dmitry Torokhov 3 participants 10 messages Nov 11, 2015 – Dec 15, 2015 GPIO

Share & tools

Dmitry Torokhov

author

2015-11-11 19:45 UTC #0 lore raw

PATCH v2 1/1 gpiolib: tighten up ACPI legacy gpio lookups

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 <[email protected]>

---

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(+)

drivers/gpio/gpiolib-acpi.c +43 -0

index 143a9bd..d5f3008 100644
@@ -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));
+}

drivers/gpio/gpiolib.c +3 -0

index 5db3445..0faf4b7 100644
@@ -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;

drivers/gpio/gpiolib.h +8 -0

index bf34300..986eb5c 100644
@@ -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,

Signature · 5 lines

2.6.0.rc2.230.g3dd15c0


-- 
Dmitry

Mika Westerberg

Acked-by on v2 1/1

2015-11-12 09:02 UTC #1 lore raw

Quoted 6 lines · Dmitry Torokhov

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 <[email protected]>

Acked-by Mika Westerberg <[email protected]>

Linus Walleij

2015-11-17 13:43 UTC #2 lore raw

In reply to Dmitry Torokhov

Quoted 6 lines · Dmitry Torokhov

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 <[email protected]>

Quoted 8 lines

---
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

Dmitry Torokhov

author

2015-11-27 17:37 UTC #3 lore raw

Quoted 14 lines · Linus Walleij

Quoted 11 lines · Dmitry Torokhov
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 <[email protected]>
---
v1: incorporated changes suggested by Mika Westerberg in response to the draft patch I posted in Goodix thread.

Quoted 7 lines

Quoted 3 lines
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!

Signature · 1 line

Dmitry

Linus Walleij

2015-12-09 15:11 UTC #4 lore raw

Hi Dmitry,

Sorry for me being so damned slow.

Quoted 21 lines · Dmitry Torokhov

Quoted 20 lines · Linus Walleij
Quoted 6 lines · Dmitry Torokhov
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 <[email protected]>
Quoted 8 lines
---
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.

Quoted 7 lines

Quoted 1 line
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

Dmitry Torokhov

author

2015-12-09 17:21 UTC #5 lore raw

Quoted 3 lines · Linus Walleij

Hi Dmitry,
Sorry for me being so damned slow.

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

Quoted 32 lines

Quoted 21 lines · Dmitry Torokhov
Quoted 20 lines · Linus Walleij
Quoted 6 lines · Dmitry Torokhov
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 <[email protected]>
Quoted 8 lines
---
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.
Quoted 7 lines
Quoted 1 line
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?

Quoted 7 lines

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.

Signature · 1 line

Dmitry

Linus Walleij

2015-12-13 16:37 UTC #6 lore raw

Quoted 14 lines · Dmitry Torokhov

Quoted 6 lines
Quoted 5 lines
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?
Quoted 7 lines
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.

Quoted 6 lines

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

Dmitry Torokhov

author

2015-12-13 16:44 UTC #7 lore raw

Quoted 24 lines · Linus Walleij

Quoted 14 lines · Dmitry Torokhov
Quoted 6 lines
Quoted 5 lines
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?
Quoted 7 lines
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.
Quoted 6 lines
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.

Quoted 4 lines

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!

Signature · 1 line

Dmitry

Linus Walleij

2015-12-15 13:24 UTC #8 lore raw

Quoted 25 lines · Dmitry Torokhov

Quoted 24 lines · Linus Walleij
Quoted 20 lines · Dmitry Torokhov
Quoted 6 lines
Quoted 5 lines
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?
Quoted 7 lines
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.

Quoted 6 lines

Quoted 4 lines
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

Dmitry Torokhov

author

2015-12-15 18:47 UTC #9 lore raw

Quoted 32 lines · Linus Walleij

Quoted 30 lines · Dmitry Torokhov
Quoted 28 lines · Linus Walleij
Quoted 14 lines · Dmitry Torokhov
Quoted 6 lines
Quoted 5 lines
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?
Quoted 7 lines
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.
Quoted 6 lines
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?

Quoted 7 lines

Quoted 1 line
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!

Signature · 1 line

Dmitry