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?

I add the software reset for testing only. Its not change the behave and
therefore it will NOT be part of my patch.

>> 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.
(as mentioned will not commit because not change anything)


>> +               /* 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
>
I'd fresh cloned the linux-next and will prepare a patch from 3.13-rc8 
tomorrow.
with fix the ICONFB and debug output.
(I'd have bloody meetings today eat up all my time .. )

Chris