Hi,
This is to report what I think is a problem in the pinctrl-icelake
driver. It seems that when trying to control GPIO pins GPP_A* and
GPIO_B*, the driver ends up writing to incorrect PADCFG registers.
I've reached this conclusion by putting debug prints in the driver,
although this can be seen by the following commands too. Please let me
know if something is wrong in my experiments. For example, when trying
to control GPP_B8/ISH_I2C1_SCL, the driver ends up writing to
GPP_A6/ESPI_RESETB registers.
I want to control the pin GPP_B8 i.e. ISH_I2C1_SCL and drive it low or
high. I looked at the driver code, and it tells me that I need to use
pin number 16:
static const struct pinctrl_pin_desc icllp_pins[] = {
....
/* GPP_B */
....
PINCTRL_PIN(16, "ISH_I2C1_SCL"),
.....
The pin number 16 is mapped to GPIO 224 as per the below output:
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/gpio-ranges
GPIO ranges handled:
0: INT3455:00 GPIOS [184 - 191] PINS [0 - 7]
32: INT3455:00 GPIOS [216 - 241] PINS [8 - 33]
64: INT3455:00 GPIOS [248 - 272] PINS [34 - 58]
96: INT3455:00 GPIOS [280 - 303] PINS [59 - 82]
128: INT3455:00 GPIOS [312 - 332] PINS [83 - 103]
160: INT3455:00 GPIOS [344 - 363] PINS [104 - 123]
192: INT3455:00 GPIOS [376 - 404] PINS [124 - 152]
224: INT3455:00 GPIOS [408 - 431] PINS [153 - 176]
256: INT3455:00 GPIOS [440 - 463] PINS [183 - 206]
288: INT3455:00 GPIOS [472 - 479] PINS [216 - 223]
320: INT3455:00 GPIOS [504 - 511] PINS [224 - 231]
localhost /sys/class/gpio #
So I did this:
localhost /sys/class/gpio # ls
export gpiochip184 unexport
localhost /sys/class/gpio # cat gpiochip184/label
INT3455:00
localhost /sys/class/gpio # cat gpiochip184/ngpio
328
localhost /sys/class/gpio # echo 224 > export
localhost /sys/class/gpio # ls
export gpio224 gpiochip184 unexport
localhost /sys/class/gpio #
Before trying to change the value, I decided to take a dump of the
PADCFG of the pins before and after I tried to change it. Here is the
dump I took before changing:
localhost /sys/class/gpio # cat gpio224/active_low
0
localhost /sys/class/gpio # cat gpio224/direction
in
localhost /sys/class/gpio # cat gpio224/edge
none
localhost /sys/class/gpio # cat gpio224/value
0
localhost /sys/class/gpio #
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"ISH_I2C1_SCL\|ESPI_RESETB"
pin 16 (ISH_I2C1_SCL) GPIO 0x44000102 0x00000028 0x00000000 [ACPI]
pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
localhost /sys/class/gpio #
Here is when I tried to change it and then dump the values again:
localhost /sys/class/gpio # echo out > gpio224/direction
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"ISH_I2C1_SCL\|ESPI_RESETB"
pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
localhost /sys/class/gpio #
Note that things look good so far (the PADCFG DW0 for pin 16 is
corrrectly changed).
But now when I try to write the value, it is the PADCFG DW0 for pin 40
that gets written to, while the pin16 value does not change:
localhost /sys/class/gpio #
localhost /sys/class/gpio # echo 1 > gpio224/value
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"ISH_I2C1_SCL\|ESPI_RESETB"
pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
pin 40 (ESPI_RESETB) mode 1 0x44000701 0x0000003e 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio #
localhost /sys/class/gpio # echo 0 > gpio224/value
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"ISH_I2C1_SCL\|ESPI_RESETB"
pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio # echo 1 > gpio224/value
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"ISH_I2C1_SCL\|ESPI_RESETB"
pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
pin 40 (ESPI_RESETB) mode 1 0x44000701 0x0000003e 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio #
I have also verified this using an oscilloscope on the external pin
that the ISH_I2C1_SCL does not change. Also the gpio224/value stays
stuck at 0.
localhost /sys/class/gpio # cat gpio224/value
0
localhost /sys/class/gpio #
I failed to understand how could it be that the correct pin (16) is
picked up when setting direction but incorrect pin (40) is picked up
when setting value. I suspect some mux related call is being missed
out while setting the value, may be? Can you please check this at your
end?
Thanks,
Rajat
On Fri, Sep 14, 2018 at 05:18:34PM -0700, Rajat Jain wrote:
> Hi,
>
> This is to report what I think is a problem in the pinctrl-icelake
> driver. It seems that when trying to control GPIO pins GPP_A* and
> GPIO_B*, the driver ends up writing to incorrect PADCFG registers.
> I've reached this conclusion by putting debug prints in the driver,
> although this can be seen by the following commands too. Please let me
> know if something is wrong in my experiments. For example, when trying
> to control GPP_B8/ISH_I2C1_SCL, the driver ends up writing to
> GPP_A6/ESPI_RESETB registers.
>
> I want to control the pin GPP_B8 i.e. ISH_I2C1_SCL and drive it low or
> high. I looked at the driver code, and it tells me that I need to use
> pin number 16:
>
> static const struct pinctrl_pin_desc icllp_pins[] = {
> ....
> /* GPP_B */
> ....
> PINCTRL_PIN(16, "ISH_I2C1_SCL"),
> .....
>
> The pin number 16 is mapped to GPIO 224 as per the below output:
>
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/gpio-ranges
> GPIO ranges handled:
> 0: INT3455:00 GPIOS [184 - 191] PINS [0 - 7]
> 32: INT3455:00 GPIOS [216 - 241] PINS [8 - 33]
> 64: INT3455:00 GPIOS [248 - 272] PINS [34 - 58]
> 96: INT3455:00 GPIOS [280 - 303] PINS [59 - 82]
> 128: INT3455:00 GPIOS [312 - 332] PINS [83 - 103]
> 160: INT3455:00 GPIOS [344 - 363] PINS [104 - 123]
> 192: INT3455:00 GPIOS [376 - 404] PINS [124 - 152]
> 224: INT3455:00 GPIOS [408 - 431] PINS [153 - 176]
> 256: INT3455:00 GPIOS [440 - 463] PINS [183 - 206]
> 288: INT3455:00 GPIOS [472 - 479] PINS [216 - 223]
> 320: INT3455:00 GPIOS [504 - 511] PINS [224 - 231]
> localhost /sys/class/gpio #
>
> So I did this:
>
> localhost /sys/class/gpio # ls
> export gpiochip184 unexport
> localhost /sys/class/gpio # cat gpiochip184/label
> INT3455:00
> localhost /sys/class/gpio # cat gpiochip184/ngpio
> 328
> localhost /sys/class/gpio # echo 224 > export
> localhost /sys/class/gpio # ls
> export gpio224 gpiochip184 unexport
> localhost /sys/class/gpio #
>
> Before trying to change the value, I decided to take a dump of the
> PADCFG of the pins before and after I tried to change it. Here is the
> dump I took before changing:
>
> localhost /sys/class/gpio # cat gpio224/active_low
> 0
> localhost /sys/class/gpio # cat gpio224/direction
> in
> localhost /sys/class/gpio # cat gpio224/edge
> none
> localhost /sys/class/gpio # cat gpio224/value
> 0
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "ISH_I2C1_SCL\|ESPI_RESETB"
> pin 16 (ISH_I2C1_SCL) GPIO 0x44000102 0x00000028 0x00000000 [ACPI]
> pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
> localhost /sys/class/gpio #
>
> Here is when I tried to change it and then dump the values again:
>
> localhost /sys/class/gpio # echo out > gpio224/direction
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "ISH_I2C1_SCL\|ESPI_RESETB"
> pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
> pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
> localhost /sys/class/gpio #
>
> Note that things look good so far (the PADCFG DW0 for pin 16 is
> corrrectly changed).
> But now when I try to write the value, it is the PADCFG DW0 for pin 40
> that gets written to, while the pin16 value does not change:
>
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # echo 1 > gpio224/value
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "ISH_I2C1_SCL\|ESPI_RESETB"
> pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
> pin 40 (ESPI_RESETB) mode 1 0x44000701 0x0000003e 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # echo 0 > gpio224/value
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "ISH_I2C1_SCL\|ESPI_RESETB"
> pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
> pin 40 (ESPI_RESETB) mode 1 0x44000700 0x0000003e 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # echo 1 > gpio224/value
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "ISH_I2C1_SCL\|ESPI_RESETB"
> pin 16 (ISH_I2C1_SCL) GPIO 0x44000200 0x00000028 0x00000000 [ACPI]
> pin 40 (ESPI_RESETB) mode 1 0x44000701 0x0000003e 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio #
>
> I have also verified this using an oscilloscope on the external pin
> that the ISH_I2C1_SCL does not change. Also the gpio224/value stays
> stuck at 0.
>
> localhost /sys/class/gpio # cat gpio224/value
> 0
> localhost /sys/class/gpio #
>
> I failed to understand how could it be that the correct pin (16) is
> picked up when setting direction but incorrect pin (40) is picked up
> when setting value. I suspect some mux related call is being missed
> out while setting the value, may be? Can you please check this at your
> end?
Do you have schematics of the system? It could be that the ISH_I2C1_SCL
is just pulled low.
Can you try some other pin that you could measure and is wired a pin
header?
On Fri, Sep 14, 2018 at 05:18:34PM -0700, Rajat Jain wrote:
> This is to report what I think is a problem in the pinctrl-icelake
> driver. It seems that when trying to control GPIO pins GPP_A* and
> GPIO_B*, the driver ends up writing to incorrect PADCFG registers.
> I've reached this conclusion by putting debug prints in the driver,
> although this can be seen by the following commands too. Please let me
> know if something is wrong in my experiments. For example, when trying
> to control GPP_B8/ISH_I2C1_SCL, the driver ends up writing to
> GPP_A6/ESPI_RESETB registers.
Hmm, when you add debug prints to the driver and you access GPIO 224
(GPP_B8/ISH_I2C1_SCL) from userspace you can see that the driver
actually uses PADCFG registers of GPP_A6/ESPI_RESETB? So that it is not
just a side-effect of how the pins are wired on your board.
On Mon, Sep 17, 2018 at 1:13 AM Mika Westerberg
<[email protected]> wrote:
>
> On Fri, Sep 14, 2018 at 05:18:34PM -0700, Rajat Jain wrote:
> > This is to report what I think is a problem in the pinctrl-icelake
> > driver. It seems that when trying to control GPIO pins GPP_A* and
> > GPIO_B*, the driver ends up writing to incorrect PADCFG registers.
> > I've reached this conclusion by putting debug prints in the driver,
> > although this can be seen by the following commands too. Please let me
> > know if something is wrong in my experiments. For example, when trying
> > to control GPP_B8/ISH_I2C1_SCL, the driver ends up writing to
> > GPP_A6/ESPI_RESETB registers.
>
> Hmm, when you add debug prints to the driver and you access GPIO 224
> (GPP_B8/ISH_I2C1_SCL) from userspace you can see that the driver
> actually uses PADCFG registers of GPP_A6/ESPI_RESETB? So that it is not
> just a side-effect of how the pins are wired on your board.
Right, also I don't have to use put debug prints as it can be seen
from the following debug files. And it is not isolated to just this
pin. For instance, in this examples you can see that another pin 18
(GPP_B10/I2C5_SCL) that I'm trying to write , the value actually gets
written to the PADCFG for (GPP_A8/I2S2_SFRM) i.e. pin42. So there is
clearly a pattern:
static const struct pinctrl_pin_desc icllp_pins[] = {
...
/* GPP_B */
..
PINCTRL_PIN(18, "I2C5_SCL"), <----- GPP_B10/I2C5_SCL = Pin 18
....
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/gpio-ranges
GPIO ranges handled:
0: INT3455:00 GPIOS [184 - 191] PINS [0 - 7]
32: INT3455:00 GPIOS [216 - 241] PINS [8 - 33] <---- pin 18 = gpio 226
....
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"I2C5_SCL\|(I2S2_SFRM)"
pin 18 (I2C5_SCL) mode 1 0x44000702 0x0000002a 0x00000000 [ACPI]
pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio # echo 226 > export
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"I2C5_SCL\|(I2S2_SFRM)"
pin 18 (I2C5_SCL) GPIO 0x44000102 0x0000002a 0x00000000 [ACPI]
pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio #
localhost /sys/class/gpio # cat gpio226/value
0
localhost /sys/class/gpio # cat gpio226/direction
in
localhost /sys/class/gpio # echo out > gpio226/direction
localhost /sys/class/gpio # cat gpio226/direction
out
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"I2C5_SCL\|(I2S2_SFRM)"
pin 18 (I2C5_SCL) GPIO 0x44000200 0x0000002a 0x00000000 [ACPI]
pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
localhost /sys/class/gpio #
localhost /sys/class/gpio #
localhost /sys/class/gpio # cat gpio226/value
0
localhost /sys/class/gpio # echo 1 > gpio226/value
localhost /sys/class/gpio # cat gpio226/value
0
localhost /sys/class/gpio # cat
/sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
"I2C5_SCL\|(I2S2_SFRM)"
pin 18 (I2C5_SCL) GPIO 0x44000200 0x0000002a 0x00000000 [ACPI]
pin 42 (I2S2_SFRM) mode 2 0x44000b01 0x00000040 0x00000100 [ACPI]
localhost /sys/class/gpio #
As you can see in the above example, when I export the pins and change
the directions from "in" to "out" PADCFG get updated correctly for pin
18, but when writing the value, it is the PADCFG for pin 42 that gets
updated which is incorrect.
So this looks like a driver issue to me. Please let me know if I need
to file a bug on bugzilla for this.
Thanks,
Rajat
On Mon, Sep 17, 2018 at 11:16:41AM -0700, Rajat Jain wrote:
> On Mon, Sep 17, 2018 at 1:13 AM Mika Westerberg
> <[email protected]> wrote:
> >
> > On Fri, Sep 14, 2018 at 05:18:34PM -0700, Rajat Jain wrote:
> > > This is to report what I think is a problem in the pinctrl-icelake
> > > driver. It seems that when trying to control GPIO pins GPP_A* and
> > > GPIO_B*, the driver ends up writing to incorrect PADCFG registers.
> > > I've reached this conclusion by putting debug prints in the driver,
> > > although this can be seen by the following commands too. Please let me
> > > know if something is wrong in my experiments. For example, when trying
> > > to control GPP_B8/ISH_I2C1_SCL, the driver ends up writing to
> > > GPP_A6/ESPI_RESETB registers.
> >
> > Hmm, when you add debug prints to the driver and you access GPIO 224
> > (GPP_B8/ISH_I2C1_SCL) from userspace you can see that the driver
> > actually uses PADCFG registers of GPP_A6/ESPI_RESETB? So that it is not
> > just a side-effect of how the pins are wired on your board.
>
> Right, also I don't have to use put debug prints as it can be seen
> from the following debug files. And it is not isolated to just this
> pin. For instance, in this examples you can see that another pin 18
> (GPP_B10/I2C5_SCL) that I'm trying to write , the value actually gets
> written to the PADCFG for (GPP_A8/I2S2_SFRM) i.e. pin42. So there is
> clearly a pattern:
>
> static const struct pinctrl_pin_desc icllp_pins[] = {
> ...
> /* GPP_B */
> ..
> PINCTRL_PIN(18, "I2C5_SCL"), <----- GPP_B10/I2C5_SCL = Pin 18
> ....
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/gpio-ranges
> GPIO ranges handled:
> 0: INT3455:00 GPIOS [184 - 191] PINS [0 - 7]
> 32: INT3455:00 GPIOS [216 - 241] PINS [8 - 33] <---- pin 18 = gpio 226
> ....
>
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "I2C5_SCL\|(I2S2_SFRM)"
> pin 18 (I2C5_SCL) mode 1 0x44000702 0x0000002a 0x00000000 [ACPI]
> pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # echo 226 > export
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "I2C5_SCL\|(I2S2_SFRM)"
> pin 18 (I2C5_SCL) GPIO 0x44000102 0x0000002a 0x00000000 [ACPI]
> pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # cat gpio226/value
> 0
> localhost /sys/class/gpio # cat gpio226/direction
> in
> localhost /sys/class/gpio # echo out > gpio226/direction
> localhost /sys/class/gpio # cat gpio226/direction
> out
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "I2C5_SCL\|(I2S2_SFRM)"
> pin 18 (I2C5_SCL) GPIO 0x44000200 0x0000002a 0x00000000 [ACPI]
> pin 42 (I2S2_SFRM) mode 2 0x44000b00 0x00000040 0x00000100 [ACPI]
> localhost /sys/class/gpio #
> localhost /sys/class/gpio #
> localhost /sys/class/gpio # cat gpio226/value
> 0
> localhost /sys/class/gpio # echo 1 > gpio226/value
> localhost /sys/class/gpio # cat gpio226/value
> 0
> localhost /sys/class/gpio # cat
> /sys/kernel/debug/pinctrl/INT3455\:00/pins | grep
> "I2C5_SCL\|(I2S2_SFRM)"
> pin 18 (I2C5_SCL) GPIO 0x44000200 0x0000002a 0x00000000 [ACPI]
> pin 42 (I2S2_SFRM) mode 2 0x44000b01 0x00000040 0x00000100 [ACPI]
> localhost /sys/class/gpio #
>
> As you can see in the above example, when I export the pins and change
> the directions from "in" to "out" PADCFG get updated correctly for pin
> 18, but when writing the value, it is the PADCFG for pin 42 that gets
> updated which is incorrect.
It looks like we are missing translation (call intel_gpio_to_pin()) in
intel_gpio_set(), intel_gpio_get() and intel_gpio_get_direction(). IIRC
gpiolib handles the translation but here it seems not. Strange.
> So this looks like a driver issue to me. Please let me know if I need
> to file a bug on bugzilla for this.
I agree, definitely driver issue. Please file bugzilla about this (add
me and Andy there as well) and we'll investigate.
Thanks!
On Tue, Sep 18, 2018 at 11:31:57AM +0300, Mika Westerberg wrote:
> > As you can see in the above example, when I export the pins and change
> > the directions from "in" to "out" PADCFG get updated correctly for pin
> > 18, but when writing the value, it is the PADCFG for pin 42 that gets
> > updated which is incorrect.
>
> It looks like we are missing translation (call intel_gpio_to_pin()) in
> intel_gpio_set(), intel_gpio_get() and intel_gpio_get_direction(). IIRC
> gpiolib handles the translation but here it seems not. Strange.
>
> > So this looks like a driver issue to me. Please let me know if I need
> > to file a bug on bugzilla for this.
>
> I agree, definitely driver issue. Please file bugzilla about this (add
> me and Andy there as well) and we'll investigate.
If you have not yet filed bugzilla about this then I don't think there
is need anymore as I think I have a fix already. It turns out with the
custom GPIO base (Ice Lake, Cannon Lake) the translation is missing
completely in ->get(), ->set() and ->get_direction() operations.
I'll send a proper patch shortly.
Good bug hunting in this thread!
Please see if you can use libgpiod or the tools from the kernel
tools/gpio directory to test GPIO lines, I usually use that since
the global GPIO numberspace only constantly confuse me.
Especially when there is a really many many GPIOs the local
numberspace and the global GPIO numberspace approach the
same values and I get totally lost sometimes.
Yours,
Linus Walleij