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

[Benoît Thébaudeau]

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.

Best regards,
Benoît