imx27: pinctrl: GPIO set output value failed

[Chris Ruehl]

On Wednesday, January 15, 2014 04:56 PM, Markus Pargmann wrote:
> Hi Chris,
>
> On Tue, Jan 14, 2014 at 05:13:25PM +0800, Chris Ruehl wrote:
>> On Tuesday, January 14, 2014 04:57 PM, Markus Pargmann wrote:
>>> Hi,
>>>
>>> On Tue, Jan 14, 2014 at 01:39:22PM +0800, Chris Ruehl wrote:
>>>> Just fall over the Reference manual for the GPIO ports.
>>>> Seams the iconfb0 is not addressed right.
>>>>
>>>> On Tuesday, January 14, 2014 12:13 PM, Chris Ruehl wrote:
>>>>> Hi,
>>>>>
>>>>> some kind of weired problem:
>>>>> Some GPIO's can modified via /sysfs  others cannot
>>>>> in my case gpio9 works, and gpio10,12 not.
>>>>> The pinctrl debug out looks good to me.
>>>>>
>>>>> running kernel 3.13-rc
>>>>>
>>>>> Working OK
>>>> Table 6.1 of the imx27 reference manual says (Page 6-5,6-6)
>>>>> [ 1.553601] imx27-pinctrl 10015000.iomuxc: imx1_pmx_enable, pin
>>>>> 0x9, function 0, gpio 1, direction 1, oconf 3, iconfa 0, iconfb 0
>>>>> [    1.553657] imx27-pinctrl 10015000.iomuxc: write: register
>>>>> 0xf4415004 offset 18 value 0x3
>>>> PTA_OCR1  1001_5004 OK
>>>>> [    1.553712] imx27-pinctrl 10015000.iomuxc: write: register
>>>>> 0xf441500c offset 18 value 0x0
>>>> PTA_ICONFA1 1001_500C  OK
>>>>> [    1.553766] imx27-pinctrl 10015000.iomuxc: write: register
>>>>> 0xf4415010 offset 18 value 0x0
>>>> PTA_ICONFB1 1001_5014  --->  0xf4415010 seams the wrong address
>>>> 1001_5010 is the PTA_ICONFA2
>>> Yes you are right, it is wrong by 0x4, pinctrl-imx1-core.c:
>>> #define MX1_ICONFB 0x10
>>>
>>> Can you send a patch?
>>>
>>> Thanks,
>>>
>>> Markus
>>>
>> Hi Markus,
>>
>> I found the wrong define already and recompile the kernel, but still
>> not working for me. Very strange.
>> gpio9,13  and 14,16 as input works OK. Only this buggers 10 and 12
>> are bugging around.
>>
>> a) I replaced a debug output statement after the address correction
>> when the pin-id > 16 to get correct
>> address output. Here a preview of the patch.
>> b) add pin soft reset before set the gpio parameter.
> gpios 10 and 12 are working correctly with a previous pin soft reset?
>
>> diff --git a/drivers/pinctrl/pinctrl-imx1-core.c
>> b/drivers/pinctrl/pinctrl-imx1-core.c
>> index 17aecde..fbaa70e 100644
>> --- a/drivers/pinctrl/pinctrl-imx1-core.c
>> +++ b/drivers/pinctrl/pinctrl-imx1-core.c
>> @@ -27,6 +27,7 @@
>>   #include <linux/pinctrl/pinctrl.h>
>>   #include <linux/pinctrl/pinmux.h>
>>   #include <linux/slab.h>
>> +#include <linux/delay.h>
>>
>>   #include "core.h"
>>   #include "pinctrl-imx1.h"
>> @@ -45,9 +46,10 @@ struct imx1_pinctrl {
>>   #define MX1_DDIR 0x00
>>   #define MX1_OCR 0x04
>>   #define MX1_ICONFA 0x0c
>> -#define MX1_ICONFB 0x10
>> +#define MX1_ICONFB 0x14
>>   #define MX1_GIUS 0x20
>>   #define MX1_GPR 0x38
>> +#define MX1_SWR 0x3C
>>   #define MX1_PUEN 0x40
>>
>>   #define MX1_PORT_STRIDE 0x100
>> @@ -97,13 +99,13 @@ static void imx1_write_2bit(struct imx1_pinctrl
>> *ipctl, unsigned int pin_id,
>>          u32 old_val;
>>          u32 new_val;
>>
>> -       dev_dbg(ipctl->dev, "write: register 0x%p offset %d value 0x%x\n",
>> -                       reg, offset, value);
>> -
>>          /* Use the next register if the pin's port pin number is >=16 */
>>          if (pin_id % 32 >= 16)
>>                  reg += 0x04;
>>
>> +       dev_dbg(ipctl->dev, "write: register 0x%p offset %d value 0x%x\n",
>> +                       reg, offset, value);
>> +
>>          /* Get current state of pins */
>>          old_val = readl(reg);
>>          old_val &= mask;
>> @@ -334,6 +336,9 @@ static int imx1_pmx_enable(struct pinctrl_dev
>> *pctldev, unsigned selector,
>>                                  direction, gpio_oconf, gpio_iconfa,
>>                                  gpio_iconfb);
>>
>> +               imx1_write_bit(ipctl, pin_id, 1, MX1_SWR);  /* Soft
>> reset the pin */
> This is over 80 lines, simply put the comment above. Would also be
> great if you could add some more description here why a soft reset is
> necessary. It is not listed in the reference manual, so this may be
> confusing otherwise.
>
>> +               /* reset done within 6 cycles */
>> +               usleep_range(10000,20000);
>>                  imx1_write_bit(ipctl, pin_id, gpio_in_use, MX1_GIUS);
>>                  imx1_write_bit(ipctl, pin_id, direction, MX1_DDIR);
>>
>>
>> I will do some more testing and send the patch after, OK for you?
> Yes that's ok. You could also seperate the ICONFB bugfix into a seperate
> patch.
>
> Regards,
>
> Markus
>


Hi Markus,

Its very confusing what the mxc driver is doing, I'd enabled the 
debugging and the
kernel spits out this:

root at gtsir20:~# dmesg | tail -2
[   43.683766] mxc: set GPIO 9 to high trigger
[   44.766186] mxc: switch GPIO 9 to low trigger
root at gtsir20:~# echo 0 > /sys/class/gpio/gpio9/value
root at gtsir20:~# echo 1 > /sys/class/gpio/gpio9/value
root at gtsir20:~# dmesg | tail -6
[   40.753967] Bluetooth: RFCOMM socket layer initialized
[   40.759277] Bluetooth: RFCOMM ver 1.11
[   43.683766] mxc: set GPIO 9 to high trigger
[   44.766186] mxc: switch GPIO 9 to low trigger
[ 3120.825803] mxc: switch GPIO 9 to high trigger
[ 3126.281689] mxc: switch GPIO 9 to low trigger

But GPIO9 is defined as output not as IRQ
and act as output so enable/disable an external device
connected to it.

Why its sets the interrupts?

When I switch gpio10/12 on/off there is no output and its defined
the same way as gpio9

                pinctrl_userio1: userio1-1 {
                         fsl,pins = <
                                 /* 2x GPIO in */
                                 MX27_PAD_LD8__GPIO1_14 0x0
                                 MX27_PAD_LD10__GPIO1_16 0x0
                                 /* 3x GPIO out
                                 MX27_PAD_LD3__GPIO1_9 0x0
                                 MX27_PAD_LD4__GPIO1_10 0x0
                                 MX27_PAD_LD6__GPIO1_12 0x0 */
                                 0x09 0x036 0x0
                                 0x0a 0x036 0x0
                                 0x0c 0x036 0x0

(linux-next-20140115)

Chris