linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* GPIO regression on mvebu and/or max310x since 2ab73c6d8323
@ 2020-06-10  0:39 Jan Kundrát
  2020-06-10  8:03 ` Thierry Reding
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Kundrát @ 2020-06-10  0:39 UTC (permalink / raw)
  To: linux-gpio; +Cc: Thierry Reding, Linus Walleij, Vidya Sagar

Hi,
after upgrading from v5.6.7 to v5.7.1, my bit-banged I2C bus no longer 
works. I've run a bisection and it led me to commit 2ab73c6d8323 (gpio: 
Support GPIO controllers without pin-ranges). If I make 
gpiochip_generic_request() call pinctrl_gpio_request() unconditionally 
again, stuff gets back to working.

My HW setup is "unusual" as there's also an analog switch in the path of 
the bit-banged bus, and that one is controlled by another GPIO from 
MAX14830 (drivers/tty/serial/max310x.c). The I2C bit-banging is driven by a 
Solidrun ClearFog Base (mvebu, Armada AM385) pins MPP24 and MPP25 which are 
configured as GPIOs. In terms of a DTS snippet, here's how it looks like:

        /* Bit-banged I2C instead of the default UART function from the SoC 
*/
        &uart1_pins {
                status = "disabled";
        };
        &uart1 {
                status = "disabled";
        };
        gpio_i2c {
                compatible = "i2c-gpio";
                sda-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                scl-gpios = <&gpio0 24 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                i2c-gpio.delay-us = <1>;
                #address-cells = <1>;
                #size-cells = <0>;
        };
        
        /* Analog switch control via a GPIO hog. This drives that analog 
switch
           so that the signals from the SoC actually reach the I2C slaves 
*/
        max14830@2 {
                // ...
                i2c_bitbang_enable {
                        gpio-hog;
                        gpios = <7 GPIO_ACTIVE_HIGH>;
                        output-high;
                        line-name = "I2C bitbang bus";
                };
        };

This means that for my bit-banged I2C to work, I need both the mvebu's 
gpio+pinctrl and the MAX14830 GPIOs. I hope that the max310x.c path is OK 
(max310x_gpio_direction_output() and friends are being called when kernel 
sets up GPIO hogs). According to some dev_dbg()s in i2c/busses/i2c-gpio.c 
and i2c/algos/i2c-algo-bit.c, the I2C slaves never respond with an ACK. The 
machine is remote so I have not had a chance to attach a logical analyzer 
yet -- but I *think* that the root cause is somewhere in pinconf. When 
stuff doesn't work, I get this:

# grep -e mpp24 -e mpp25 
/sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups
24 (mpp24): current: ua1(rxd), available = [ gpio(io) spi0(miso) ua0(cts) 
sd0(d4) dev(ready) ]
25 (mpp25): current: ua1(txd), available = [ gpio(io) spi0(cs0) ua0(rts) 
sd0(d5) dev(cs0) ]

...whereas on the old kernel, it looks like this:

# grep -e mpp24 -e mpp25 
/sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups 
24 (mpp24): current: gpio(io), available = [ spi0(miso) ua0(cts) ua1(rxd) 
sd0(d4) dev(ready) ]
25 (mpp25): current: gpio(io), available = [ spi0(cs0) ua0(rts) ua1(txd) 
sd0(d5) dev(cs0) ]

There's also some difference in 
/sys/kernel/debug/pinctrl/f1018000.pinctrl/pinmux-pins says, for example:

 pin 19 (PIN19): f1072004.mdio (GPIO UNCLAIMED) function gpio group mpp19

...whereas in the old kernel it is:

 pin 19 (PIN19): f1072004.mdio mvebu-gpio:19 function gpio group mpp19

...and the same for 22, 24, 29, 44, 54, all of these are supposed to be 
GPIOs I think. At this time I'm deep in the bowels of pinconf/pinmux that I 
do not understand.

TL;DR: 2ab73c6d8323 breaks GPIOs on mvebu on my system. I'll be happy to 
test patches which fix this in some better way than just a revert.

With kind regards,
Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-06-10  0:39 GPIO regression on mvebu and/or max310x since 2ab73c6d8323 Jan Kundrát
@ 2020-06-10  8:03 ` Thierry Reding
  2020-06-10 21:00   ` Jan Kundrát
  2020-06-11 16:12   ` Jan Kundrát
  0 siblings, 2 replies; 7+ messages in thread
From: Thierry Reding @ 2020-06-10  8:03 UTC (permalink / raw)
  To: Jan Kundrát; +Cc: linux-gpio, Linus Walleij, Vidya Sagar

[-- Attachment #1: Type: text/plain, Size: 5239 bytes --]

On Wed, Jun 10, 2020 at 02:39:22AM +0200, Jan Kundrát wrote:
> Hi,
> after upgrading from v5.6.7 to v5.7.1, my bit-banged I2C bus no longer
> works. I've run a bisection and it led me to commit 2ab73c6d8323 (gpio:
> Support GPIO controllers without pin-ranges). If I make
> gpiochip_generic_request() call pinctrl_gpio_request() unconditionally
> again, stuff gets back to working.
> 
> My HW setup is "unusual" as there's also an analog switch in the path of the
> bit-banged bus, and that one is controlled by another GPIO from MAX14830
> (drivers/tty/serial/max310x.c). The I2C bit-banging is driven by a Solidrun
> ClearFog Base (mvebu, Armada AM385) pins MPP24 and MPP25 which are
> configured as GPIOs. In terms of a DTS snippet, here's how it looks like:
> 
>        /* Bit-banged I2C instead of the default UART function from the SoC
> */
>        &uart1_pins {
>                status = "disabled";
>        };
>        &uart1 {
>                status = "disabled";
>        };
>        gpio_i2c {
>                compatible = "i2c-gpio";
>                sda-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>                scl-gpios = <&gpio0 24 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
>                i2c-gpio.delay-us = <1>;
>                #address-cells = <1>;
>                #size-cells = <0>;
>        };
>                /* Analog switch control via a GPIO hog. This drives that
> analog switch
>           so that the signals from the SoC actually reach the I2C slaves */
>        max14830@2 {
>                // ...
>                i2c_bitbang_enable {
>                        gpio-hog;
>                        gpios = <7 GPIO_ACTIVE_HIGH>;
>                        output-high;
>                        line-name = "I2C bitbang bus";
>                };
>        };
> 
> This means that for my bit-banged I2C to work, I need both the mvebu's
> gpio+pinctrl and the MAX14830 GPIOs. I hope that the max310x.c path is OK
> (max310x_gpio_direction_output() and friends are being called when kernel
> sets up GPIO hogs). According to some dev_dbg()s in i2c/busses/i2c-gpio.c
> and i2c/algos/i2c-algo-bit.c, the I2C slaves never respond with an ACK. The
> machine is remote so I have not had a chance to attach a logical analyzer
> yet -- but I *think* that the root cause is somewhere in pinconf. When stuff
> doesn't work, I get this:
> 
> # grep -e mpp24 -e mpp25
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups
> 24 (mpp24): current: ua1(rxd), available = [ gpio(io) spi0(miso) ua0(cts)
> sd0(d4) dev(ready) ]
> 25 (mpp25): current: ua1(txd), available = [ gpio(io) spi0(cs0) ua0(rts)
> sd0(d5) dev(cs0) ]
> 
> ...whereas on the old kernel, it looks like this:
> 
> # grep -e mpp24 -e mpp25
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinconf-groups 24 (mpp24):
> current: gpio(io), available = [ spi0(miso) ua0(cts) ua1(rxd) sd0(d4)
> dev(ready) ]
> 25 (mpp25): current: gpio(io), available = [ spi0(cs0) ua0(rts) ua1(txd)
> sd0(d5) dev(cs0) ]
> 
> There's also some difference in
> /sys/kernel/debug/pinctrl/f1018000.pinctrl/pinmux-pins says, for example:
> 
> pin 19 (PIN19): f1072004.mdio (GPIO UNCLAIMED) function gpio group mpp19
> 
> ...whereas in the old kernel it is:
> 
> pin 19 (PIN19): f1072004.mdio mvebu-gpio:19 function gpio group mpp19
> 
> ...and the same for 22, 24, 29, 44, 54, all of these are supposed to be
> GPIOs I think. At this time I'm deep in the bowels of pinconf/pinmux that I
> do not understand.
> 
> TL;DR: 2ab73c6d8323 breaks GPIOs on mvebu on my system. I'll be happy to
> test patches which fix this in some better way than just a revert.

All of the above seems to indicate that there are no GPIO ranges
associated with the pinmux controller, because that's the only reason
why gpiochip_generic_request() wouldn't call into pinctrl to request
the pin for GPIO.

However, the reason why I sent that patch was because the absence of
GPIO ranges would actually cause pinctrl_gpio_request() to crash, so in
order to be able to use gpiochip_generic_request() on chips that don't
have GPIO ranges we have to call pinctrl_gpio_request() conditionally.

So I don't understand how this could've worked for you before. Unless
perhaps if there's some other difference between v5.6.7 and v5.7.1 that
introduces the crash that I was seeing.

I think a good first step would be to try that reverting the change
would actually fix the issue for you. Something like the below should do
the trick. Also, can you point out which exact DTS file it is that you
see this problem with?

--- >8 ---
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index c14f0784274a..2befc4dba7f3 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2783,11 +2783,6 @@ static inline void gpiochip_irqchip_free_valid_mask(struct gpio_chip *gc)
  */
 int gpiochip_generic_request(struct gpio_chip *gc, unsigned offset)
 {
-#ifdef CONFIG_PINCTRL
- if (list_empty(&gc->gpiodev->pin_ranges))
-  return 0;
-#endif
-
  return pinctrl_gpio_request(gc->gpiodev->base + offset);
 }
 EXPORT_SYMBOL_GPL(gpiochip_generic_request);
--- >8 ---

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-06-10  8:03 ` Thierry Reding
@ 2020-06-10 21:00   ` Jan Kundrát
  2020-06-11 16:12   ` Jan Kundrát
  1 sibling, 0 replies; 7+ messages in thread
From: Jan Kundrát @ 2020-06-10 21:00 UTC (permalink / raw)
  To: Thierry Reding; +Cc: linux-gpio, Linus Walleij, Vidya Sagar

> I think a good first step would be to try that reverting the change
> would actually fix the issue for you. Something like the below should do
> the trick.

That's indeed the case, reverting that commit makes everything work again 
on 5.7.1. We have five other unrelated patches as well, our tree is 
available at 
https://gerrit.cesnet.cz/plugins/gitiles/github/torvalds/linux/+log/aabb023ef197c4b365a4404bca2fd8cd0c227835 
.

> Also, can you point out which exact DTS file it is that you
> see this problem with?

It's armada-388-clearfog-base.dts, and we have some small additions on top 
of that. The only modification which looks relevant that we ship is:

/ {
        gpio_i2c {
                compatible = "i2c-gpio";
                sda-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                scl-gpios = <&gpio0 24 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                i2c-gpio.delay-us = <1>;
                #address-cells = <1>;
                #size-cells = <0>;
        };
};

&uart1_pins {
        status = "disabled";
};

&uart1 {
        status = "disabled";
};

With kind regards,
Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-06-10  8:03 ` Thierry Reding
  2020-06-10 21:00   ` Jan Kundrát
@ 2020-06-11 16:12   ` Jan Kundrát
  2020-06-23 21:11     ` Jan Kundrát
  1 sibling, 1 reply; 7+ messages in thread
From: Jan Kundrát @ 2020-06-11 16:12 UTC (permalink / raw)
  To: Thierry Reding, linux-gpio; +Cc: Linus Walleij, Vidya Sagar

> All of the above seems to indicate that there are no GPIO ranges
> associated with the pinmux controller, because that's the only reason
> why gpiochip_generic_request() wouldn't call into pinctrl to request
> the pin for GPIO.

I think it's the other way round: there are no pin_ranges associated with 
the mvebu's gpiochip, but there definitely are GPIOs registered for the 
pinctrl. I went looking further, and indeed: nothing calls 
gpiochip_add_pingroup_range() or gpiochip_add_pin_range().

From the docs it seems that this is supposed to be called by the GPIO (or 
pinctrl? or pinmux?) core upon the "gpio-ranges" DT property. 
Unfortunately, the in-the-tree DTS files have no such property in 
arch/arm/boot/dts/armada-388-clearfog-base.dts. Is that a bug in these DTS 
files?

> However, the reason why I sent that patch was because the absence of
> GPIO ranges would actually cause pinctrl_gpio_request() to crash,

It's interesting because it doesn't crash for me. Are you referring to 
pinctrl_get_device_gpio_range() which gets called from 
pinctrl_gpio_request()? If so, I think that that one iterates over all 
pinctrl devices in the system, and it doesn't seem to care about a 
particular gpiodev's pin_ranges. It does check a pctldev's gpio_ranges 
later on in pinctrl_match_gpio_range(), but that's a different thing (a 
mapping in the opposite direction, right?).

Anyway, I added a few more print/debugs, and here's roughly how it looks on 
5.7.1 with 2ab73c6d8323 reverted (and extra debugging):

[    2.313257] i2c-gpio gpio_i2c: GPIO lookup for consumer sda
[    2.313261] i2c-gpio gpio_i2c: using device tree for GPIO lookup
[    2.313278] gpio gpiochip0: gpiochip_generic_request for offset 25: 
empty list of ranges, proceeding anyway
[    2.323070] pinctrl_gpio_request: 25:
[    2.326762] pinctrl_get_device_gpio_range: for gpio 25
[    2.331916] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 25
[    2.341514] pinctrl_gpio_request 25: requesting via _request_gpio
[    2.347628] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio pin 
25 gpio 25
[    2.355313] armada-38x-pinctrl f1018000.pinctrl: request pin 25 (PIN25) 
for mvebu-gpio:25
[    2.355318] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio ret 
0
[    2.362211]  got 0
[    2.364233] pinctrl_get_device_gpio_range: for gpio 25
[    2.369386] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 25
[    2.378987] i2c-gpio gpio_i2c: GPIO lookup for consumer scl
[    2.378989] i2c-gpio gpio_i2c: using device tree for GPIO lookup
[    2.379000] gpio gpiochip0: gpiochip_generic_request for offset 24: 
empty list of ranges, proceeding anyway
[    2.388771] pinctrl_gpio_request: 24:
[    2.392441] pinctrl_get_device_gpio_range: for gpio 24
[    2.397596] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 24
[    2.407192] pinctrl_gpio_request 24: requesting via _request_gpio
[    2.413302] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio pin 
24 gpio 24
[    2.420985] armada-38x-pinctrl f1018000.pinctrl: request pin 24 (PIN24) 
for mvebu-gpio:24
[    2.420989] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio ret 
0
[    2.427884]  got 0
[    2.429900] pinctrl_get_device_gpio_range: for gpio 24
[    2.435055] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 24
[    2.444806] pinctrl_get_device_gpio_range: for gpio 25
[    2.449964] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 25
[    2.459576] pinctrl_get_device_gpio_range: for gpio 24
[    2.464736] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 24
[    2.474337] pinctrl_get_device_gpio_range: for gpio 25
[    2.479489] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 25

(these lines are reported about 200 times...)

[    8.081207] pinctrl_get_device_gpio_range: for gpio 24
[    8.086362] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 24
[    8.095965] pinctrl_get_device_gpio_range: for gpio 25
[    8.101117] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 25
[    8.110723] i2c-gpio gpio_i2c: using lines 25 (SDA) and 24 (SCL)

I'm not saying that this is perfect (why is that repeated about 200 
times?), but it works. And here's the initial probe along with the in-DTS 
gpio-hog definitions:

[    0.050828] armada-38x-pinctrl f1018000.pinctrl: try to register 60 pins 
...
[    0.050832] pinctrl core: registered pin 0 (PIN0) on f1018000.pinctrl
...
[    0.050971] pinctrl core: registered pin 59 (PIN59) on f1018000.pinctrl
[    0.050975] armada-38x-pinctrl f1018000.pinctrl: no hogs found
[    0.051002] armada-38x-pinctrl f1018000.pinctrl: registered pinctrl 
driver
[    0.051014] armada-38x-pinctrl f1018000.pinctrl: pinctrl_add_gpio_range
[    0.051295] armada-38x-pinctrl f1018000.pinctrl: pinctrl_add_gpio_range
[    0.051740] gpio gpiochip0: gpiochip_generic_request for offset 19: 
empty list of ranges, proceeding anyway
[    0.051753] pinctrl_gpio_request: 19:
[    0.051758] pinctrl_get_device_gpio_range: for gpio 19
[    0.051766] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 19
[    0.051775] pinctrl_gpio_request 19: requesting via _request_gpio
[    0.051783] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio pin 
19 gpio 19
[    0.051794] armada-38x-pinctrl f1018000.pinctrl: request pin 19 (PIN19) 
for mvebu-gpio:19
[    0.051799] armada-38x-pinctrl f1018000.pinctrl: pinmux_request_gpio ret 
0
[    0.051805]  got 0
[    0.051813] pinctrl_get_device_gpio_range: for gpio 19
[    0.051821] armada-38x-pinctrl f1018000.pinctrl: 
pinctrl_get_device_gpio_range: found a range for GPIO 19
[    0.051833] GPIO line 19 (phy1-reset) hogged as output/low

Unfortunately I'm largely ignorant of the relation between gpio, pinctrl 
and pinmux, so I'm afraid I cannot dig much further. Should I try to 
blindly add the "gpio-ranges" into the DTS, for example?

With kind regards,
Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-06-11 16:12   ` Jan Kundrát
@ 2020-06-23 21:11     ` Jan Kundrát
  2020-07-07 12:28       ` Linus Walleij
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Kundrát @ 2020-06-23 21:11 UTC (permalink / raw)
  To: Thierry Reding, Linus Walleij; +Cc: linux-gpio, Vidya Sagar

On čtvrtek 11. června 2020 18:12:07 CEST, Jan Kundrát wrote:
>> All of the above seems to indicate that there are no GPIO ranges
>> associated with the pinmux controller, because that's the only reason
>> why gpiochip_generic_request() wouldn't call into pinctrl to request
>> the pin for GPIO.
>
> I think it's the other way round: there are no pin_ranges 
> associated with the mvebu's gpiochip, but there definitely are 
> GPIOs registered for the pinctrl. I went looking further, and 
> indeed: nothing calls gpiochip_add_pingroup_range() or 
> gpiochip_add_pin_range().
>
> From the docs it seems that this is supposed to be called by 
> the GPIO (or pinctrl? or pinmux?) core upon the "gpio-ranges" DT 
> property. Unfortunately, the in-the-tree DTS files have no such 
> property in arch/arm/boot/dts/armada-388-clearfog-base.dts. Is 
> that a bug in these DTS files?
>
>> However, the reason why I sent that patch was because the absence of
>> GPIO ranges would actually cause pinctrl_gpio_request() to crash,
>
> It's interesting because it doesn't crash for me. Are you 
> referring to pinctrl_get_device_gpio_range() which gets called 
> from pinctrl_gpio_request()? If so, I think that that one 
> iterates over all pinctrl devices in the system, and it doesn't 
> seem to care about a particular gpiodev's pin_ranges. It does 
> check a pctldev's gpio_ranges later on in 
> pinctrl_match_gpio_range(), but that's a different thing (a 
> mapping in the opposite direction, right?).
>
> Anyway, I added a few more print/debugs, and here's roughly how 
> it looks on 5.7.1 with 2ab73c6d8323 reverted (and extra 
> debugging):
>
> [    2.313257] i2c-gpio gpio_i2c: GPIO lookup for consumer sda
> [    2.313261] i2c-gpio gpio_i2c: using device tree for GPIO lookup
> [    2.313278] gpio gpiochip0: gpiochip_generic_request for 
> offset 25: empty list of ranges, proceeding anyway
> [    2.323070] pinctrl_gpio_request: 25:
> [    2.326762] pinctrl_get_device_gpio_range: for gpio 25
> [    2.331916] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 25
> [    2.341514] pinctrl_gpio_request 25: requesting via _request_gpio
> [    2.347628] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio pin 25 gpio 25
> [    2.355313] armada-38x-pinctrl f1018000.pinctrl: request pin 
> 25 (PIN25) for mvebu-gpio:25
> [    2.355318] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio ret 0
> [    2.362211]  got 0
> [    2.364233] pinctrl_get_device_gpio_range: for gpio 25
> [    2.369386] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 25
> [    2.378987] i2c-gpio gpio_i2c: GPIO lookup for consumer scl
> [    2.378989] i2c-gpio gpio_i2c: using device tree for GPIO lookup
> [    2.379000] gpio gpiochip0: gpiochip_generic_request for 
> offset 24: empty list of ranges, proceeding anyway
> [    2.388771] pinctrl_gpio_request: 24:
> [    2.392441] pinctrl_get_device_gpio_range: for gpio 24
> [    2.397596] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 24
> [    2.407192] pinctrl_gpio_request 24: requesting via _request_gpio
> [    2.413302] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio pin 24 gpio 24
> [    2.420985] armada-38x-pinctrl f1018000.pinctrl: request pin 
> 24 (PIN24) for mvebu-gpio:24
> [    2.420989] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio ret 0
> [    2.427884]  got 0
> [    2.429900] pinctrl_get_device_gpio_range: for gpio 24
> [    2.435055] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 24
> [    2.444806] pinctrl_get_device_gpio_range: for gpio 25
> [    2.449964] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 25
> [    2.459576] pinctrl_get_device_gpio_range: for gpio 24
> [    2.464736] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 24
> [    2.474337] pinctrl_get_device_gpio_range: for gpio 25
> [    2.479489] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 25
>
> (these lines are reported about 200 times...)
>
> [    8.081207] pinctrl_get_device_gpio_range: for gpio 24
> [    8.086362] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 24
> [    8.095965] pinctrl_get_device_gpio_range: for gpio 25
> [    8.101117] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 25
> [    8.110723] i2c-gpio gpio_i2c: using lines 25 (SDA) and 24 (SCL)
>
> I'm not saying that this is perfect (why is that repeated about 
> 200 times?), but it works. And here's the initial probe along 
> with the in-DTS gpio-hog definitions:
>
> [    0.050828] armada-38x-pinctrl f1018000.pinctrl: try to 
> register 60 pins ...
> [    0.050832] pinctrl core: registered pin 0 (PIN0) on f1018000.pinctrl
> ...
> [    0.050971] pinctrl core: registered pin 59 (PIN59) on f1018000.pinctrl
> [    0.050975] armada-38x-pinctrl f1018000.pinctrl: no hogs found
> [    0.051002] armada-38x-pinctrl f1018000.pinctrl: registered 
> pinctrl driver
> [    0.051014] armada-38x-pinctrl f1018000.pinctrl: pinctrl_add_gpio_range
> [    0.051295] armada-38x-pinctrl f1018000.pinctrl: pinctrl_add_gpio_range
> [    0.051740] gpio gpiochip0: gpiochip_generic_request for 
> offset 19: empty list of ranges, proceeding anyway
> [    0.051753] pinctrl_gpio_request: 19:
> [    0.051758] pinctrl_get_device_gpio_range: for gpio 19
> [    0.051766] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 19
> [    0.051775] pinctrl_gpio_request 19: requesting via _request_gpio
> [    0.051783] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio pin 19 gpio 19
> [    0.051794] armada-38x-pinctrl f1018000.pinctrl: request pin 
> 19 (PIN19) for mvebu-gpio:19
> [    0.051799] armada-38x-pinctrl f1018000.pinctrl: 
> pinmux_request_gpio ret 0
> [    0.051805]  got 0
> [    0.051813] pinctrl_get_device_gpio_range: for gpio 19
> [    0.051821] armada-38x-pinctrl f1018000.pinctrl: 
> pinctrl_get_device_gpio_range: found a range for GPIO 19
> [    0.051833] GPIO line 19 (phy1-reset) hogged as output/low
>
> Unfortunately I'm largely ignorant of the relation between 
> gpio, pinctrl and pinmux, so I'm afraid I cannot dig much 
> further. Should I try to blindly add the "gpio-ranges" into the 
> DTS, for example?

Hi Thierry, Linus,
can you help me with this, please? As it is now, my GPIO-based I2C no 
longer works since 2ab73c6d8323 on this Solidrun Clearfog Base. I suspect 
that this is because of no gpio-ranges property in the DTS files, but I 
don't know how the mvebu gpio/pinctrl/pinmux drivers work internally to 
debug this.

With kind regards,
Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-06-23 21:11     ` Jan Kundrát
@ 2020-07-07 12:28       ` Linus Walleij
  2021-03-09 14:49         ` Jan Kundrát
  0 siblings, 1 reply; 7+ messages in thread
From: Linus Walleij @ 2020-07-07 12:28 UTC (permalink / raw)
  To: Jan Kundrát; +Cc: Thierry Reding, open list:GPIO SUBSYSTEM, Vidya Sagar

On Tue, Jun 23, 2020 at 11:11 PM Jan Kundrát <jan.kundrat@cesnet.cz> wrote:

> > Unfortunately I'm largely ignorant of the relation between
> > gpio, pinctrl and pinmux, so I'm afraid I cannot dig much
> > further. Should I try to blindly add the "gpio-ranges" into the
> > DTS, for example?
>
> Hi Thierry, Linus,
> can you help me with this, please? As it is now, my GPIO-based I2C no
> longer works since 2ab73c6d8323 on this Solidrun Clearfog Base. I suspect
> that this is because of no gpio-ranges property in the DTS files, but I
> don't know how the mvebu gpio/pinctrl/pinmux drivers work internally to
> debug this.

I have as little clue as you, who maintains this driver? Who wrote it?

Can you just send a patch to the Armada DTS file adding these ranges?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: GPIO regression on mvebu and/or max310x since 2ab73c6d8323
  2020-07-07 12:28       ` Linus Walleij
@ 2021-03-09 14:49         ` Jan Kundrát
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Kundrát @ 2021-03-09 14:49 UTC (permalink / raw)
  To: open list:GPIO SUBSYSTEM; +Cc: Linus Walleij, Thierry Reding, Vidya Sagar

> Can you just send a patch to the Armada DTS file adding these ranges?

I looked at some other November/December-ish commits into the DT, and in 
the end, this is the patch that I needed to add to my board-specific DTS:

 / {
        gpio_i2c {
                compatible = "i2c-gpio";
                sda-gpios = <&gpio0 25 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                scl-gpios = <&gpio0 24 (GPIO_ACTIVE_HIGH | 
GPIO_OPEN_DRAIN)>;
                i2c-gpio.delay-us = <1>;
                #address-cells = <1>;
                #size-cells = <0>;
+               pinctrl-names = "default";
+               pinctrl-0 = <&uart1_pins_i2c_bb>;
        };
 };
 
-&uart1_pins {
+&pinctrl {
+       uart1_pins_i2c_bb: uart1-pins-i2c-bb {
+               marvell,pins = "mpp24", "mpp25";
+               marvell,function = "gpio";
+       };
+};
+
+&mikro_uart_pins {
        status = "disabled";
 };

The missing bit was making sure that the "i2c-gpio" instantiation sees a 
correct pinctrl-names and pinctrl-0 properties.

I think there's probably nothing to fix upstream here; I think it could be 
considered common knowledge than one has to patch the pinctrl block in DT 
in order to re-purpose a set of pins from, say, UART to GPIO. What confused 
me is that pre-5.7, this used to work without setting the pinctrl to "gpio" 
explicitly.

With kind regards,
Jan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-03-09 14:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-10  0:39 GPIO regression on mvebu and/or max310x since 2ab73c6d8323 Jan Kundrát
2020-06-10  8:03 ` Thierry Reding
2020-06-10 21:00   ` Jan Kundrát
2020-06-11 16:12   ` Jan Kundrát
2020-06-23 21:11     ` Jan Kundrát
2020-07-07 12:28       ` Linus Walleij
2021-03-09 14:49         ` Jan Kundrát

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).