linusw/devel bisection: baseline.bootrr.mediatek-mt8173-pinctrl-probed on mt8173-elm-hana

* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

* This automated bisection report was sent to you on the basis  *
* that you may be involved with the breaking commit it has      *
* found.  No manual investigation has been done to verify it,   *
* and the root cause of the problem may be somewhere else.      *
*                                                               *
* If you do send a fix, please include this trailer:            *
*   Reported-by: "kernelci.org bot" <[email protected]>          *
*                                                               *
* Hope this helps!                                              *

* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *

linusw/devel bisection: baseline.bootrr.mediatek-mt8173-pinctrl-probed on mt8173-elm-hana

Summary:

  Start:      65efb43ac94b gpiolib: Disallow identical line names in the same chip
  Plain log:  https://storage.kernelci.org/linusw/devel/v5.10-rc4-91-g65efb43ac94b/arm64/defconfig/gcc-8/lab-collabora/baseline-mt8173-elm-hana.txt
  HTML log:   https://storage.kernelci.org/linusw/devel/v5.10-rc4-91-g65efb43ac94b/arm64/defconfig/gcc-8/lab-collabora/baseline-mt8173-elm-hana.html
  Result:     65efb43ac94b gpiolib: Disallow identical line names in the same chip

Checks:
revert: PASS
verify: PASS

Parameters:

  Tree:       linusw
  URL:        https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/
  Branch:     devel
  Target:     mt8173-elm-hana
  CPU arch:   arm64
  Lab:        lab-collabora
  Compiler:   gcc-8
  Config:     defconfig
  Test case:  baseline.bootrr.mediatek-mt8173-pinctrl-probed

Breaking commit found:

-------------------------------------------------------------------------------
commit 65efb43ac94bffeb652cddba4106817bb38c5e71

Author: Linus Walleij <[email protected]>
Date:   Sat Dec 12 01:34:47 2020 +0100

gpiolib: Disallow identical line names in the same chip

We need to make this namespace hierarchical: at least do not allow two lines on the same chip to have the same name, this is just too much flexibility. If we name a line on a chip, name it uniquely on that chip.

I don't know what happens if we just apply this, I *hope* there are not a lot of systems out there breaking this simple and intuitive rule.

As a side effect, this makes the device tree naming code scream a bit if names are not globally unique.

I think there are not super-many device trees out there naming their lines so let's fix this before the problem becomes widespread.

Cc Geert Uytterhoeven <[email protected]>

Cc Johan Hovold <[email protected]>

Signed-off-by Linus Walleij <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

drivers/gpio/gpiolib.c +26 -9

index 5ce0c14c637b..fe1b96b7f127 100644
@@ -330,11 +330,9 @@ static struct gpio_desc *gpio_name_to_desc(const char * const name)
 
 /*
  * Take the names from gc->names and assign them to their GPIO descriptors.
- * Warn if a name is already used for a GPIO line on a different GPIO chip.
  *
- * Note that:
- *   1. Non-unique names are still accepted,
- *   2. Name collisions within the same GPIO chip are not reported.
+ * - Fail if a name is already used for a GPIO line on the same chip.
+ * - Allow names to not be globally unique but warn about it.
  */
 static int gpiochip_set_desc_names(struct gpio_chip *gc)
 {

@@ -343,13 +341,19 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc)
 
 	/* First check all names if they are unique */
 	for (i = 0; i != gc->ngpio; ++i) {
-		struct gpio_desc *gpio;
+		struct gpio_desc *gpiod;
 
-		gpio = gpio_name_to_desc(gc->names[i]);
-		if (gpio)
+		gpiod = gpio_name_to_desc(gc->names[i]);
+		if (gpiod) {
 			dev_warn(&gdev->dev,
 				 "Detected name collision for GPIO name '%s'\n",
 				 gc->names[i]);
+			if (gpiod->gdev == gdev) {
+				dev_err(&gdev->dev,
+					"GPIO name collision on the same chip, this is not allowed, fix all lines on the chip to have unique names\n");
+				return -EEXIST;
+			}
+		}
 	}
 
 	/* Then add all names to the GPIO descriptors */

@@ -402,8 +406,22 @@ static int devprop_gpiochip_set_names(struct gpio_chip *chip)
 		return ret;
 	}
 
-	for (i = 0; i < count; i++)
+	for (i = 0; i < count; i++) {
+		struct gpio_desc *gpiod;
+
+		gpiod = gpio_name_to_desc(names[i]);
+		if (gpiod) {
+			dev_warn(&gdev->dev,
+                                 "Detected name collision for GPIO name '%s'\n",
+                                 names[i]);
+			if (gpiod->gdev == gdev) {
+				dev_err(&gdev->dev,
+					"GPIO name collision on the same chip, this is not allowed, fix all lines on the chip to have unique names\n");
+				return -EEXIST;
+			}
+		}
 		gdev->descs[i].name = names[i];
+	}
 
 	kfree(names);
-------------------------------------------------------------------------------

Git bisection log:

-------------------------------------------------------------------------------
git bisect start
# good: [9777d0bfdae796de3f8d73879a43bc00145dc8ee] gpio: cs5535: Simplify the return expression of cs5535_gpio_probe() git bisect good 9777d0bfdae796de3f8d73879a43bc00145dc8ee
# bad: [65efb43ac94bffeb652cddba4106817bb38c5e71] gpiolib: Disallow identical line names in the same chip git bisect bad 65efb43ac94bffeb652cddba4106817bb38c5e71
# good: [a8f25236e6e3d945139b62da0c4398778f77a5b3] MAINTAINERS: Add maintainer for HiSilicon GPIO driver git bisect good a8f25236e6e3d945139b62da0c4398778f77a5b3

# first bad commit: [65efb43ac94bffeb652cddba4106817bb38c5e71] gpiolib: Disallow identical line names in the same chip
-------------------------------------------------------------------------------

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#4626): https://groups.io/g/kernelci-results/message/4626
Mute This Topic: https://groups.io/mt/78950269/924702
Group Owner: [email protected]
Unsubscribe: https://groups.io/g/kernelci-results/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Please see the bisection report below about the pinctrl driver failing to probe on the arm64 mt8173-elm-hana platform.

This is the error message:

[    0.051788] gpio gpiochip0: Detected name collision for GPIO name ''
[    0.051813] gpio gpiochip0: GPIO name collision on the same chip, this is not allowed, fix all lines on the chip to have unique names
[    0.051832] gpiochip_add_data_with_key: GPIOs 377..511 (1000b000.pinctrl) failed to register, -17
[    0.051946] mediatek-mt8173-pinctrl: probe of 1000b000.pinctrl failed with error -22

Quoted 2 lines · Guillaume Tucker

Please see the bisection report below about the pinctrl driver failing to probe on the arm64 mt8173-elm-hana platform.

That's an excellent, helpful report which helps a lot! Thank you for doing this!

Quoted 6 lines

This is the error message:

[    0.051788] gpio gpiochip0: Detected name collision for GPIO name ''
[    0.051813] gpio gpiochip0: GPIO name collision on the same chip, this is not allowed, fix all lines on the chip to have unique names
[    0.051832] gpiochip_add_data_with_key: GPIOs 377..511 (1000b000.pinctrl) failed to register, -17
[    0.051946] mediatek-mt8173-pinctrl: probe of 1000b000.pinctrl failed with error -22

It seems we need to teach the core to ignore the name (empty string).

Quoted 1 line

It seems we need to teach the core to ignore the name (empty string).

OK great, I see you've sent a patch for that. I'll check if we can confirm it fixes the issue (something I'd like to also automate...).

Quoted 5 lines · Guillaume Tucker

Quoted 1 line

It seems we need to teach the core to ignore the name (empty string).

OK great, I see you've sent a patch for that. I'll check if we can confirm it fixes the issue (something I'd like to also automate...).

Yups would love to hear if this solves it, it should be in today's -next.

Quoted 9 lines · Linus Walleij

Quoted 5 lines · Guillaume Tucker

Quoted 1 line

It seems we need to teach the core to ignore the name (empty string).

OK great, I see you've sent a patch for that. I'll check if we can confirm it fixes the issue (something I'd like to also automate...).

Quoted 2 lines

Yups would love to hear if this solves it, it should be in today's -next.

Yes in fact it appears to be all fixed on your for-next branch:

https://kernelci.org/test/case/id/5fda32f92738afa48dc94ce1/