IMX53: Floating GPIO Pins due to MUX_PAD_CTRL(NO_PAD_CTRL) bug.

TomE · ‎01-24-2016

Has this problem been found and fixed already? It is so serious it might be causing all sorts of problems to a lot of products.

Scroll down to "RESULT" if you want to see what this does.

This applies to the 2.6.35 Linux that is the last one Freescale released for the i.MX53.

(Edit) This also affects Android distributions "imx_2.6.38_11.09.01", but was fixed in "11.11.01" and "12.01.01"

This most likely doesn't affect i.MX6, but I don't have a build to test this with.

I was verifying the levels and timing of the PWRDWN and RESET GPIO pins we have driving an ADV7180 on an i.MX53-based product.

The board stopped working when I put an oscilloscope probe on those signals!

The reason turned out to be that the PWRDN pin was floating, and the 10M probe pulled it to ground. The CPU's default configuration is to have a 100k pullup on the pin, but that had been turned off by something!

The "something" turned out to be a serious, widespread bug in the arch/arm/plat-mxc/include/mach/iomux-mx53.h file, and in other ones. One way to describe the bug is to say that there are 986 buggy lines in that file, like this one:

#define MX53_PAD_SD2_CLK__GPIO1_10 (_MX53_PAD_SD2_CLK__GPIO1_10 | MUX_PAD_CTRL(NO_PAD_CTRL))

The other way to put it is that the definition of "NO_PAD_CTRL" is wrong.

"NO_PAD_CTRL" means to leave the control bits for the port (that control hysteresis, drive strength and the pullup or pulldown value) alone at their Reset Default condition.

The problem is pretty obvious looking at the definitions of the above macros in this file:

arch/arm/plat-mxc/include/mach/iomux-v3.h:

#define MUX_PAD_CTRL_SHIFT 41
#define MUX_PAD_CTRL_MASK ((iomux_v3_cfg_t)0x1ffff << MUX_PAD_CTRL_SHIFT)
#define NO_PAD_CTRL ((iomux_v3_cfg_t)1 << (MUX_PAD_CTRL_SHIFT + 16))
#define MUX_PAD_CTRL(x) ((iomux_v3_cfg_t)(x) << MUX_PAD_CTRL_SHIFT)

Allowing the compiler to expand some definitions in the board file for me:

arch/arm/mach-mx5/mx53_evk.c or mx53_loco.c:

#include <mach/iomux-mx53.h>

static iomux_v3_cfg_t tx53_pads[] = {
...
MX53_PAD_GPIO_2__GPIO1_2,
MX53_PAD_GPIO_5__GPIO1_5,
...
};

arch/arm/plat-mxc/include/mach/iomux-mx53.h:

#include <mach/iomux-v3.h>

#define MX53_PAD_GPIO_2__GPIO1_2 (_MX53_PAD_GPIO_2__GPIO1_2 | MUX_PAD_CTRL(NO_PAD_CTRL))
#define MX53_PAD_GPIO_5__GPIO1_5 (_MX53_PAD_GPIO_5__GPIO1_5 | MUX_PAD_CTRL(NO_PAD_CTRL))

The above results (using "gcc -E" and using exactly the same command as the normal build uses):

static iomux_v3_cfg_t tx53_fec_gpio_pads[] __attribute__ ((__section__(".init.data"))
) = {
...
((((iomux_v3_cfg_t)(0x30) << 0) | ((iomux_v3_cfg_t)(1) << 36) |
((iomux_v3_cfg_t)(0x358) << 12) | ((iomux_v3_cfg_t)(0) << 41) |
((iomux_v3_cfg_t)(0x0) << 24) | ((iomux_v3_cfg_t)(0) << 58)) |
((iomux_v3_cfg_t)(((iomux_v3_cfg_t)1 << (41 + 16))) << 41)),
((((iomux_v3_cfg_t)(0x328) << 0) | ((iomux_v3_cfg_t)(1) << 36) |
((iomux_v3_cfg_t)(0x6B8) << 12) | ((iomux_v3_cfg_t)(0) << 41) |
((iomux_v3_cfg_t)(0x0) << 24) | ((iomux_v3_cfg_t)(0) << 58)) |
((iomux_v3_cfg_t)(((iomux_v3_cfg_t)1 << (41 + 16))) << 41)),};

The red bit results in 1 being left-shifted 98 bit positions and then being stored in a 64-bit word.

EITHER "NO_PAD_CTRL" shouldn't be shifted, or it shouldn't be inside MUX_PAD_CTRL(). One or the other.

There's another bug that prevents this from working. The following code is meant to check to see if "NO_PAD_CTRL" is set, and if it is, to NOT write to the control register:

arch/arm/plat-mxc/iomux-v3.c:
int mxc_iomux_v3_setup_multiple_pads(...)
    mxc_iomux_v3_setup_pad(*p);
        u32 pad_ctrl_ofs = (pad & MUX_PAD_CTRL_OFS_MASK) >> MUX_PAD_CTRL_OFS_SHIFT;
        u32 pad_ctrl = (pad & MUX_PAD_CTRL_MASK) >> MUX_PAD_CTRL_SHIFT;

if (!(pad_ctrl & NO_PAD_CTRL) && pad_ctrl_ofs)
__raw_writel(pad_ctrl, base + pad_ctrl_ofs);

The test in the second last line comes down to:

((pad & (0x1ffff << 41)) >> 41) & (1 << (41 + 16))

It is comparing the right-shifted pad value with the left-shifted mask. That's never true either.

RESULT

The result of all of the above is that all pads "mentioned" anywhere in the Board file will be corrupted by having the IOMUXC_SW_PAD_CTL_PAD register written to ZERO. This means:

The Hysteresis is turned off,
The Pullup or Pulldown is disabled,
The Drive Strength will be changed from the default "High" to "Low",
The Slew Rate (if the pin has that configured) will be set to "Low".

The board file listing GPIOs that will be affected may be one of:

arch/arm/mach-mx5/

mx51_3stack.c
mx51_babbage.c
mx53_ard.c
mx53_evk.c
mx53_loco.c
mx53_smd.c

Your_Board_Here.c

So if you have any problems with floating pins that you thought were safely pulled high or low, or the drive strength of an GPIO pin is a lot lower than you expect (and your hardware requires), then this might be the cause. If you have any EM or ESD susceptibility problems then it may be due to this.

Has this been fixed anywhere?

(Edit) The bug was released as a patch into a Pengutronix tree where it was fixed 7 months later. The buggy patch was included in Freescale's tree, but the fix never was.

It would be worth checking the i.MX6 files to see if they have the same problem (unlikely, i.MX6 changes fixed this bug as a side effect).

EASY TEST

You can see if this affects you from the Console by reading the IOMUXC_SW_PAD_CTL_PAD registers for the GPIOs you're using:

# devmem 0x53fa8688   # IOMUXC_SW_PAD_CTL_PAD_SD2_CLK
0x00000000
# devmem 0x53fa868c   # IOMUXC_SW_PAD_CTL_PAD_SD2_CMD
0x000001E4
# devmem 0x53fa8690   # IOMUXC_SW_PAD_CTL_PAD_SD2_DATA3 : GPIO1_12 VID_IN_PWRDWN
0x00000000
# devmem 0x53fa8694   # IOMUXC_SW_PAD_CTL_PAD_SD2_DATA2
0x000001E4
# devmem 0x53fa8698   # IOMUXC_SW_PAD_CTL_PAD_SD2_DATA1
0x000001E4
# devmem 0x53fa869c   # IOMUXC_SW_PAD_CTL_PAD_SD2_DATA0 : GPIO1_15 uP_WD_SET1
0x00000000

The Green ones are unaffected as they aren't listed in my board file.

Tom

Message was edited by: Tom Evans. Problem may affect i.MX25 and i.MX50 as well. From the code it also looks to affect 'iomux-mx35.h", but there's no "Category" for that chip.

igorpadykov · ‎01-25-2016

Hi Tom

SD2 pads (MX53_PAD_SD2_xx) are not used as gpios on i.MX53 reference boards,

for this reason NXP/FSL BSPs do not initialize them. If they are used as gpios,

it is responsibility of software engineer to add such codes on his software.

Guidelines for code modifications are given in Chapter 12 Configuring the IOMUX

Controller (IOMUXC) i.MX53 System Development User’s Guide (rev.1, 3/2011)

http://www.freescale.com/files/32bit/doc/user_guide/MX53UG.pdf

Best regards

igor

-----------------------------------------------------------------------------------------------------------------------

Note: If this post answers your question, please click the Correct Answer button. Thank you!

-----------------------------------------------------------------------------------------------------------------------

TomE · ‎01-26-2016

Igor wrote:

> Guidelines for code modifications are given in Chapter 12 Configuring the IOMUX

> Controller (IOMUXC) i.MX53 System Development User’s Guide (rev.1, 3/2011)

The link you gave me is to (Rev. 2, 06/2015). That doesn't make any difference as Chapter 12 didn't get changed anyway.

Shame, because it doesn't match the code. The GPIO setting code in linux changed a long time ago, and that document never got updated to match. It doesn't match the current Freescale software releases. I won't clog this thread with that problem, but will start another o this observation, here:

https://community.freescale.com/message/608838#608838

I've read through that document. I'm doing exactly what it says (correcting for it being out of date). It doesn't work properly because the bug in Freescale's Linux release.

I realise I committed the modern crime of reporting my problem in more that 140 characters, so I guess I can expect "TL;DR".

I think you "pattern matched" a few words in my post. If you read it you'll see my use of "MX53_PAD_GPIO_2__GPIO1_2" exactly matches what Chapter 12 in MX53UF.pdf says to do.

Could you please show this report to one of the engineers or programmers who has worked with the linux releases? From the commit comments around the time of the introduction of the bug I'd suggest showing it to:

Alan Tull <alan.tull@freescale.com> (he committed the patch with the bug)

Dinh Nguyen <Dinh.Nguyen@freescale.com>

Richard Zhao <richard.zhao@freescale.com>

Especially show it to these people who FIXED THIS BUG in the i.MX6 code release:

Mahesh Mahadevan <r9aadq@freescale.com>

Nitin Garg <nitin.garg@freescale.com>

The patch chain that caused the bug in the 11.09.01 release is listed here:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/log/arch/arm/plat-mxc/include/mach/iomux...

The patches are:

* ENGR00139455 Add support for pad settings in groups Dinh Nguyen 2011-02-21
* arm: mxc: move IOMUX_CONFIG_SION definition to iomux-v3.h Richard Zhao 2011-02-16
* ENGR00138024-1 Add full IOMUX support for MX53 and MX51 Dinh Nguyen 2011-02-03
* ARM i.MX51: Full iomux support Sascha Hauer 2011-02-03
* MXC IOMUX-V3 replace struct pad_desc with bitmapped cookie (step 2) Lothar Waßmann 2011-02-03
* ARM: i.MX IOMUX-V3 replace struct pad_desc with bitmapped cookie Lothar Waßmann 2011-02-03
* ENGR00137924-1: Added API to read IOMUX settings Ranjani Vaidyanathan 2011-02-03
* ENGR00127265-1 MX3X: Upgrade kernel to 2.6.35 Jason Liu 2010-09-09
* ENGR00126692-2: MX5X: Upgrade kernel to 2.6.35

The patch that caused the bug was:

Author: Lothar Waßmann <LW@KARO-electronics.de> 2010-11-22 19:39:51
Committer: Alan Tull <alan.tull@freescale.com> 2011-02-04 09:45:37
Parent: 9021fba6e46ff6c22e8802cabffd9334b25d4f99 (ARM: i.MX IOMUX-V3 replace struct pad_desc with bitmapped cookie)
Child: b9bca45d8192df5fd76459b373069cf244305932 (ARM i.MX51: Full iomux support)
Branches: remotes/origin/remote/freescale/imx_2.6.35, remotes/origin/remote/freescale/imx_2.6.35_11.04.01,

remotes/origin/remote/freescale/imx_2.6.35_11.05.01, remotes/origin/remote/freescale/imx_2.6.35_11.09.01,

remotes/origin/remote/freescale/imx_2.6.35_android_r10.3, remotes/origin/remote/freescale/imx_2.6.35_maintain
Follows: v2.6.35.3
Precedes: imx-android-r10.4, rel_imx_2.6.35_11.03.00

    MXC IOMUX-V3 replace struct pad_desc with bitmapped cookie (step 2)

    This patch actually replaces the 'struct pad_desc' with a u64 cookie
    to facilitate adding platform specific pad_ctrl settings to an
    existing pad definition.

    So, instead of:
        iomux_v3_cfg_t power_key = MX51_PAD_EIM_A27__GPIO_2_21;
        power_key.pad_ctrl = MX51_GPIO_PAD_CTRL_2;
        mxc_iomux_v3_setup_pad(&power_key);
    one can write:
        mxc_iomux_v3_setup_pad((MX51_PAD_EIM_A27__GPIO_2_21 & ~MUX_PAD_CTRL_MASK) | MX51_GPIO_PAD_CTRL_2);

    Patch applies to branch 'imx-for-2.6.38' of git://git.pengutronix.de/git/imx/linux-2.6

    Signed-Off-By: Lothar Waßmann <LW@KARO-electronics.de>
    Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Apart from a lot of other changes in that patch, the critical one was:

- #define NO_PAD_CTRL (1 << 16)

+ #define NO_PAD_CTRL ((iomux_v3_cfg_t)1 << (MUX_PAD_CTRL_SHIFT + 16))

Note that Lothar didn't commit this patch to Freescale's code. He committed it to a repository at pengutronix. Alan Tull copied it into the Freescale code.

This bug was fixed in the Pengutronix tree 7 months later, but nobody at Freescale picked it up:

https://lkml.org/lkml/2011/9/28/573

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/054695.html

Author: Troy Kisky <troy.kisky@boundarydevices.com> 2011-06-25 03:52:56
Committer: Sascha Hauer <s.hauer@pengutronix.de> 2011-07-07 17:07:41
Parent: 14c6500e7547704255f94c63fc6016ae5cbed1c9 (arm: mxc: move IOMUX_CONFIG_SION definition to iomux-v3.h)
Child: 6571534b600b8ca1936ff5630b9e0947f21faf16 (plat-mxc: iomux-v3.h: implicitly enable pull-up/down when that's desired)
Child: 7242e24a0f85d1f34286f6e153b119d43691d241 (ARM: i.MX51: Remove _MX51_PAD_xxx references)
Branches: master, remotes/origin/master, remotes/origin/work/efikasb
Follows:
Precedes: for-next

    MXC: iomux-v3: correct NO_PAD_CTRL definition

    iomux-v3.c uses NO_PAD_CTRL as a 32 bit value
    so it should not be shifted left by MUX_PAD_CTRL_SHIFT(41)

    Previously, anything requesting NO_PAD_CTRL would get
    their pad control register set to 0.

    Since it is a pad control mask, place it with the other mask values.

    Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
    Acked-by: Lothar Waßmann <LW@KARO-electronics.de>
    Tested-by: Lothar Waßmann <LW@KARO-electronics.de>
    Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

The bug was fixed in the Freescale i.MX6 code base (but not documented as such) in the following patch:

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/commit/arch/arm/plat-mxc/include/mach/io...

Author: Mahesh Mahadevan <r9aadq@freescale.com> 2011-10-21 21:25:06
Committer: Alan Tull <alan.tull@freescale.com> 2011-10-26 03:51:08
Parent: bf3b5237237001b7a1ecd305ace0c1cae4c83597 (ENGR00139229-1 MX6: Bring up i.MX6 sabreauto with Single core)
Child: d5752b17fb41f49ff01f95bfe38841e724abe6fa (ENGR00161926 Change the PAD settings on SD3 Write Protect)
Branches: remotes/origin/remote/freescale/imx_2.6.38_11.11.01, remotes/origin/remote/freescale/imx_2.6.38_12.01.01,

remotes/origin/remote/freescale/imx_2.6.38_android, remotes/origin/remote/freescale/imx_2.6.38_android_ics14
Follows: rel_imx_2.6.38_11.08.00
Precedes: imx-android-r13.1, imx-android-r14-alpha, rel_imx_2.6.38_11.11.01, rel_imx_2.6.38_12.01.01

ENGR00160602 Fix MX6 IOMUX Pad configuration code

The critical part of the fix (ignore the change from 16 to 17, that's to handle the i.MX6 IOMUX register):

- #define NO_PAD_CTRL ((iomux_v3_cfg_t)1 << (MUX_PAD_CTRL_SHIFT + 16))

+ #define NO_PAD_CTRL (1 << 17)

Here's a quick check to see which 2.6.35 branches are affected by this problem.

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/plat-mxc/include/mach/iomu...
#define NO_PAD_CTRL ((iomux_v3_cfg_t)1 << (MUX_PAD_CTRL_SHIFT + 16))

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/plat-mxc/include/mach/iomu...
#define NO_PAD_CTRL (1 << 16)

http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/tree/arch/arm/plat-mxc/include/mach/iomu...
#define NO_PAD_CTRL ((iomux_v3_cfg_t)1 << (MUX_PAD_CTRL_SHIFT + 16))

Tom

TomE · ‎01-27-2016

As mentioned in the first post, setting the control registers to zero also changes the Drive Strength from the default of "High" to "Low".

That reduces the speed and current ratings of the GPIO pins.

From (according to the IBIS model) from 5.6mA (High) to 1.8mA (Low).

Such that (some of) the I2C pins may not meet the full I2C specifications.

The IC specification calls for 3mA drivers. That supports using 1.2k pullups. Of course if you've used larger pullup resistors in your hardware design then you won't need the pins to sink that much.

The current rating of the pins isn't in the Data Sheet. The only figures given in there are voltages at "0.8mA". You have to read the IBIS model to find out what it really is, as detailed here:

i.MX53 I2C. Can the device pins supply the I2C standard current?

https://community.freescale.com/message/328502

"High" (5.6mA) meets the spec. "Low" (1.8mA) doesn't. Check the resistors you're using.

As detailed here, most of the chip's I2C pins have been overridden in "iomux-mx53.h" to "High" already, and that means the "NO_PAD_CTRL" bug doesn't apply. But some of the available pins seem to have missed out.

i.MX53: Incomplete GPIO Pullup Definitions in iomux-mx53.h

https://community.freescale.com/thread/384340

Tom

TomE · ‎01-26-2016

And once the above are fixed there are other floating pin due to the following that users may have problems with:

https://community.freescale.com/thread/384340

i.MX53: Incomplete GPIO Pullup Definitions in iomux-mx53.h

Tom

IMX53: Floating GPIO Pins due to MUX_PAD_CTRL(NO_PAD_CTRL) bug.

IMX53: Floating GPIO Pins due to MUX_PAD_CTRL(NO_PAD_CTRL) bug.

i.MX2x

i.MX50

i.MX51

i.MX53

Linux

Suspected Software Defect

i.MX53: Incomplete GPIO Pullup Definitions in iomux-mx53.h