Hi Michael,
On 26/01/2018 at 08:51, Michael Trimarchi wrote:
On Thu, Jan 25, 2018 at 10:48:42PM +0100, Benoît Thébaudeau wrote:
On Thu, Jan 25, 2018 at 2:06 PM, Michael Trimarchi michael@amarulasolutions.com wrote:
SION bit should be used in the situation that we need to read back the value of a pin and should not be set by default macro.
We get some malfunction as raised by following thread
https://www.spinics.net/lists/linux-usb/msg162574.html
As reported by this application note: https://www.nxp.com/docs/en/application-note/AN5078.pdf
The software input on (SION) bit is an option to force an input path to be active regardless of the value driven by the corresponding module. It is used when the nature direction of a pin depending on selected alternative function is an output, but it is needed to read the real logic value on a pin.
The SION bit can be used in: • Loopback: the module of a selected alternative function drives the pad and also receives the pad value as an input • GPIO capture: the module of a selected alternative function drives the pin and the value is captured by the GPIO
SION bit is not necessary when the pin is configured as a peripheral apart specific silicon bug. If an application needs to have this set, this should be done in board file or in dts file
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com
[...]
I have now tested the following peripherals on i.MX25 without any SION bit set: AUDMUX/SSI, eSDHC, FEC (except PHY access for which MDIO might have an issue without SION; Ethernet works but I have not checked whether this is possible at least partially without access to the PHY registers), GPIO, I2C, SLCDC, UART, and USB. Everything worked fine except the lack of SION for eSDHC CMD, which is true for all eSDHC instances (and not only for eSDHC1, so Linux's arch/arm/boot/dts/imx25-pinfunc.h should be fixed). Therefore, SION is mandatory for all the eSDHC CMD functions in
I have checked and u-boot does not have this pin-muxing. I think that this patch should go as it is and a separate patch like this one following should be applied after that. What do you think?
Your patch is just fine as is. We do not need to add support for eSDHC2 in U-Boot unless some mainline board needs it. But eSDHC2 CMD should have SION set by default in Linux.
arch/arm/include/asm/arch-mx25/iomux-mx25.h, but it can be removed for everything else (I still have to carry out one more test to confirm this for FEC MDIO), and moved to the board files if necessary (but I don't think that any mainline board code is doing something fancy enough to need it).
If the bus-status detection example in the documentation of SION in the reference manual is useful, it means that SION is probably required for all the signals requiring simultaneous output and input (such as I²C for device clock stretching or multi-master bus arbitration, except if the IP toggles between input and output low at each clock edge rather than between open-drain output high and output low), because there are no automatic SION signals between the peripherals and the pads but only direction signals that can request a single direction at a time. For bidirectional signals that do not require simultaneous output and input because they work in turns (such as FEC MDIO), SION can be required or not depending on whether the IP toggles the direction signal for each turn or always expects an input feedback while driving an open-drain output high. Even if SION is required for the I²C example mentioned above (which is unlikely as basic I²C transfers work fine and clock stretching detection is automatic and would always need the input state), the need for these advanced I²C features can be considered board-specific, so SION would still not be required in iomux-mx25.h.
In the end, for this patch, apart from the pending test for FEC MDIO: Reviewed-by: Benoît Thébaudeau benoit.thebaudeau.dev@gmail.com
Best regards, Benoît
From d6b5bf4f1b6a0bcbcfd6ff2597fd2a7b3884cb3b Mon Sep 17 00:00:00 2001
From: Michael Trimarchi michael@amarulasolutions.com Date: Fri, 26 Jan 2018 08:42:54 +0100 Subject: [PATCH] imx: mx25: Add pinmux definition for ESDHC2
Add pin mux of second esdhc interface. Set SION bin on the CMD muxing in order to avoid sdcard fail detection
Signed-off-by: Michael Trimarchi michael@amarulasolutions.com
arch/arm/include/asm/arch-mx25/iomux-mx25.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/arch/arm/include/asm/arch-mx25/iomux-mx25.h b/arch/arm/include/asm/arch-mx25/iomux-mx25.h index 437f122..1b38d16 100644 --- a/arch/arm/include/asm/arch-mx25/iomux-mx25.h +++ b/arch/arm/include/asm/arch-mx25/iomux-mx25.h @@ -225,15 +225,18 @@ enum {
MX25_PAD_LD8__LD8 = IOMUX_PAD(0x2e0, 0x0e8, 0x00, 0, 0, PAD_CTL_SRE_FAST), MX25_PAD_LD8__FEC_TX_ERR = IOMUX_PAD(0x2e0, 0x0e8, 0x05, 0, 0, NO_PAD_CTRL),
- MX25_PAD_LD8__SDHC2_CMD = IOMUX_PAD(0x2e0, 0x0e8, 0x06, 0x4e0, 0, NO_PAD_CTRL),
SD CMD, so SION is required here.
MX25_PAD_LD9__LD9 = IOMUX_PAD(0x2e4, 0x0ec, 0x00, 0, 0, PAD_CTL_SRE_FAST), MX25_PAD_LD9__FEC_COL = IOMUX_PAD(0x2e4, 0x0ec, 0x05, 0x504, 1, NO_PAD_CTRL),
MX25_PAD_LD9__SDHC2_CLK = IOMUX_PAD(0x2e4, 0x0ec, 0x06, 0x4dc, 0, NO_PAD_CTRL),
MX25_PAD_LD10__LD10 = IOMUX_PAD(0x2e8, 0x0f0, 0x00, 0, 0, PAD_CTL_SRE_FAST), MX25_PAD_LD10__FEC_RX_ER = IOMUX_PAD(0x2e8, 0x0f0, 0x05, 0x518, 1, NO_PAD_CTRL),
MX25_PAD_LD11__LD11 = IOMUX_PAD(0x2ec, 0x0f4, 0x00, 0, 0, PAD_CTL_SRE_FAST), MX25_PAD_LD11__FEC_RDATA2 = IOMUX_PAD(0x2ec, 0x0f4, 0x05, 0x50c, 1, NO_PAD_CTRL),
MX25_PAD_LD11__SDHC2_DAT1 = IOMUX_PAD(0x2ec, 0x0f4, 0x06, 0x4e8, 0, NO_PAD_CTRL),
MX25_PAD_LD12__LD12 = IOMUX_PAD(0x2f0, 0x0f8, 0x00, 0, 0, PAD_CTL_SRE_FAST), MX25_PAD_LD12__FEC_RDATA3 = IOMUX_PAD(0x2f0, 0x0f8, 0x05, 0x510, 1, NO_PAD_CTRL),
@@ -287,8 +290,10 @@ enum {
MX25_PAD_CSI_D6__CSI_D6 = IOMUX_PAD(0x328, 0x130, 0x00, 0, 0, NO_PAD_CTRL), MX25_PAD_CSI_D6__GPIO_1_31 = IOMUX_PAD(0x328, 0x130, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_D6__SDHC2_CMD = IOMUX_PAD(0x328, 0x130, 0x12, 0x4e0, 1, NO_PAD_CTRL),
MX25_PAD_CSI_D7__CSI_D7 = IOMUX_PAD(0x32c, 0x134, 0x00, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_D7__SDHC2_DAT_CLK = IOMUX_PAD(0x32C, 0x134, 0x02, 0x4dc, 1, NO_PAD_CTRL), MX25_PAD_CSI_D7__GPIO_1_6 = IOMUX_PAD(0x32c, 0x134, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_D8__CSI_D8 = IOMUX_PAD(0x330, 0x138, 0x00, 0, 0, NO_PAD_CTRL),
@@ -298,15 +303,18 @@ enum { MX25_PAD_CSI_D9__GPIO_4_21 = IOMUX_PAD(0x334, 0x13c, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_MCLK__CSI_MCLK = IOMUX_PAD(0x338, 0x140, 0x00, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_MCLK__SDHC2_DAT0 = IOMUX_PAD(0x338, 0x140, 0x02, 0x4e4, 1, NO_PAD_CTRL), MX25_PAD_CSI_MCLK__GPIO_1_8 = IOMUX_PAD(0x338, 0x140, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_VSYNC__CSI_VSYNC = IOMUX_PAD(0x33c, 0x144, 0x00, 0, 0, NO_PAD_CTRL), MX25_PAD_CSI_VSYNC__GPIO_1_9 = IOMUX_PAD(0x33c, 0x144, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_HSYNC__CSI_HSYNC = IOMUX_PAD(0x340, 0x148, 0x00, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_HSYNC__SDHC2_DAT2 = IOMUX_PAD(0x340, 0x148, 0x02, 0x4ec, 1, NO_PAD_CTRL), MX25_PAD_CSI_HSYNC__GPIO_1_10 = IOMUX_PAD(0x340, 0x148, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_PIXCLK__CSI_PIXCLK = IOMUX_PAD(0x344, 0x14c, 0x00, 0, 0, NO_PAD_CTRL),
MX25_PAD_CSI_PIXCLK__SDHC2_DAT3 = IOMUX_PAD(0x344, 0x14c, 0x02, 0x4f0, 1, NO_PAD_CTRL), MX25_PAD_CSI_PIXCLK__GPIO_1_11 = IOMUX_PAD(0x344, 0x14c, 0x05, 0, 0, NO_PAD_CTRL),
MX25_PAD_I2C1_CLK__I2C1_CLK = IOMUX_PAD(0x348, 0x150, 0x00, 0, 0, NO_PAD_CTRL),
Best regards, Benoît