Re: [PATCH v7] regulator: fixed: Convert to use GPIO descriptor only

[Linus Walleij]

On Sat, Sep 29, 2018 at 1:32 AM John Stultz <john.stultz@xxxxxxxxxx> wrote:

>   Anders recently noted a regression in -next when running HiKey,
> where USB fails to function.  I took a look and could reproduce this
> as well, and after some unsuccessful muddling about in the usb
> changes, I bisected it down to your commit here (specifically
> efdfeb079cc3 in -next).
>
> I'm not sure exactly why this would cause trouble, but I suspect it
> has something to do w/ the cable-detect OTG to host-hub switch on the
> HiKey.
>
> Anyway, I just wanted to raise this with you so it can get sorted out
> before that patch hits mainline.

OK how typical, let's see if we can figure it out. I looked at it but
I can't see what it is right off.

I guess it is this from
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts:

        reg_5v_hub: regulator@2 {
                compatible = "regulator-fixed";
                regulator-name = "5V_HUB";
                regulator-min-microvolt = <5000000>;
                regulator-max-microvolt = <5000000>;
                regulator-boot-on;
                gpio = <&gpio0 7 0>;
                regulator-always-on;
                vin-supply = <&reg_sys_5v>;
        };

It's a regulator with one GPIO.

It used to be fetched here:

+       cfg.ena_gpiod = devm_gpiod_get_optional(&pdev->dev, NULL, gflags);
+       if (IS_ERR(cfg.ena_gpiod))
+               return PTR_ERR(cfg.ena_gpiod);

So first would be to put in a print like this:

if (IS_ERR(cfg.ena_gpiod)) {
    dev_info(&pdev->dev, "error fetching enable GPIO for %s, %d\n",
dev_name(&pdev->dev), PTR_ERR(cfg.ena_gpiod));
    return PTR_ERR(cfg.ena_gpiod);
}
if (!cfg.ena_gpiod)
    dev_info(&pdev->dev, "no enable GPIO for %s\n", dev_name(&pdev->dev));
else
    dev_info(&pdev->dev, "fetched valid enable GPIO for %s\n",
dev_name(&pdev->dev));

So we make sure we get the enable GPIO handle.

Else we need to troubleshoot that.
Look if -ENOENT or -EPROBE_DEFER gets returned for example.

If those works we need to check the flags. Since this GPIO is specified
with gpio = <&gpio0 7 0>; it would nominally mean it is active high.

But there is special code in drivers/gpio/gpiolib-of.c to deal with
regulators, since those lines are by default active low and ignore
the flags in the second cell from the device tree.
In of_gpio_flags_quirks(), we force the GPIO to be active low
unless "enable-active-high" is set. So we need to look
in /sys/kernel/debug/gpio or just lsgpio to see if the active edge
is the same before/after the patch.

Sadly I don't have this board myself!

Yours,
Linus Walleij