* Re: linusw/devel bisection: baseline.bootrr.mediatek-mt8173-pinctrl-probed on mt8173-elm-hana
[not found] <5fd76cf2.1c69fb81.6f19b.b16a@mx.google.com>
@ 2020-12-14 22:28 ` Guillaume Tucker
2020-12-15 12:20 ` Linus Walleij
0 siblings, 1 reply; 6+ messages in thread
From: Guillaume Tucker @ 2020-12-14 22:28 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij
Cc: open list:GPIO SUBSYSTEM, open list, kernelci-results,
Geert Uytterhoeven, Johan Hovold, Enric Balletbo i Serra,
Collabora Kernel ML, Sean Wang, linux-mediatek
Hi Linus,
Please see the bisection report below about the pinctrl driver
failing to probe on the arm64 mt8173-elm-hana platform.
Reports aren't automatically sent to the public while we're
trialing new bisection features on kernelci.org but this one
looks valid.
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
and the full log:
https://storage.kernelci.org/linusw/devel/v5.10-rc4-91-g65efb43ac94b/arm64/defconfig/gcc-8/lab-collabora/baseline-mt8173-elm-hana.html#L492
I guess some GPIO now needs to be renamed following your patch
which enforces uniqueness, so it's not a problem with the patch
per se. As I'm not sure if it's something you would want to fix
yourself, I've also CC-ed MediaTek and others such as Enric who
knows about this platform and helped enable the test in KernelCI.
Best wishes,
Guillaume
On 14/12/2020 13:47, KernelCI bot wrote:
> * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
> * 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" <bot@kernelci.org> *
> * *
> * 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 <linus.walleij@linaro.org>
> 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 <geert+renesas@glider.be>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> Link: https://lore.kernel.org/r/20201212003447.238474-1-linus.walleij@linaro.org
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 5ce0c14c637b..fe1b96b7f127 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -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: kernelci-results+owner@groups.io
> Unsubscribe: https://groups.io/g/kernelci-results/unsub [guillaume.tucker@collabora.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread