[U-Boot] [PATCH] mx6: Add IOMUX_CONFIG_SION flag to all GPIO pins

[Otavio Salvador]

On Tue, Oct 1, 2013 at 4:50 PM, Benoît Thébaudeau
<benoit.thebaudeau at advansee.com> wrote:
> Hi Eric,
>
> On Tuesday, October 1, 2013 6:17:06 PM, Eric Nelson wrote:
>> Hi Stefano,
>>
>> On 10/01/2013 09:10 AM, Stefano Babic wrote:
>> > Hi Eric,
>> >
>> > On 01/10/2013 17:56, Eric Nelson wrote:
>> >> Hi Stefano,
>> >>
>> >> On 10/01/2013 07:49 AM, Stefano Babic wrote:
>> >>> Hi Eric,
>> >>>
>> >>> On 01/10/2013 16:26, Eric Nelson wrote:
>> >>>
>> >>>> I'm not sure where you're seeing this in the RM, but in
>> >>>> order to read the pad state when not configured as an input,
>> >>>> the SION bit needs to be set in the pad mux register on i.MX51/53.
>> >>>
>> >>> I have checked inside the "37.3.2.2 GPIO Write" for i.MX53 and "
>> >>> 35.4.2.1 Read Value from Pad" for i:MX51, I have not read anything about
>> >>> SION. If someone has found where it is described, please mail !
>> >>>
>> >>
>> >> That documentation seems to imply that there's no dependency
>> >> (i.e. there's no reference to SION), but I think that's an omission.
>> >
>> > Ok - this is surely not the first time we find errors in the documentation.
>> >
>> >> I've tested this many times, since it's a really handy way of
>> >> debugging hardware setups.
>> >>
>> >> That said, I'm not sure that there's a huge difference between
>> >> a single patch or multiple patches for each arch unless there's
>> >> some functionality dependent on being able to read the actual
>> >> value of a pin configured as a GPIO output.
>> >
>> > Ok - then I tend to apply Otavio's patch, and we will fix for the other
>> > i.MXes if we really find the same issues.
>> >
>> >>
>> >> Did I miss something in this thread that does actually require
>> >> that ability? It seems a pretty obscure thing in the normal case
>> >> to drive an output without confidence that it will succeed.
>> >
>> > Yes, it seems quite strange, but it helps to debug the hardware. It is
>> > not the first time we see that, even driving the output, the signal does
>> > not go to the expected value, due for example to a conflict (another
>> > peripheral driving the signal) or to a wrong pull up resistor. As U-Boot
>> > is a great tool for hardware debugging, reading the signal back let
>> > check that the output is set to the desired value.
>> >
>>
>> I agree with all of that, though this only covers the case of a
>> pin set up as a GPIO output, and that same debugging approach
>> is often used for other functions (display data pins, clock inputs
>> and outputs, et cetera).
>>
>> You probably wouldn't just set SION on all pins, right? I suspect
>> that there'd be some ramification in terms of power consumption if
>> nothing else.
>
> Right. Well, instead of adding SION to the pin definition header files, then we
> could just add SION where needed on a per-pin basis, e.g.:
> ---
> imx_iomux_v3_setup_pad(MX6_PAD_NANDF_D1__GPIO_2_1 |
>                         IOMUX_CONFIG_SION << MUX_MODE_SHIFT);
> ---
>
> A helper macro could be defined in arch/arm/include/asm/imx-common/iomux-v3.h in
> order to simplify the writing, e.g.:
> ---
> #define MUX_MODE_SION   (IOMUX_CONFIG_SION << MUX_MODE_SHIFT)
> ---
> imx_iomux_v3_setup_pad(MX6_PAD_NANDF_D1__GPIO_2_1 | MUX_MODE_SION);
> ---
>
> On the Linux side of things, the CONFIG field in the pinctrl DT bindings already
> provides bit 30 for SION. Hence, using MUX_MODE_SION like above would be close
> to Linux's pin config.

Thus dropping my patch, right? Or do you think for GPIO we ought to have them?

-- 
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750