Skip to contentSkip to conversation

mx5 iomux: Fix GPIO with SION

PATCH New Benoît Thébaudeau 2 participants 5 messages Aug 14, 2012 – Aug 17, 2012 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 mx5 iomux: Fix GPIO with SION

IOMUX_CONFIG_GPIO may be used together with IOMUX_CONFIG_SION, so don't break the iomux function value set to the register in this case.

Define the value of IOMUX_CONFIG_GPIO in a way showing that it should have only 1 bit set.

---
 .../arch/arm/cpu/armv7/mx5/iomux.c                 |    5 +++--
 .../arch/arm/include/asm/arch-mx5/iomux.h          |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/mx5/iomux.c +3 -2
index d4e3bbb..7adf08f 100644
@@ -124,8 +124,9 @@ static void iomux_config_mux(iomux_pin_name_t pin, iomux_pin_cfg_t cfg)
 
 	if ((mux_reg > get_mux_end()) || (mux_reg < IOMUXSW_MUX_CTL))
 		return ;
-	if (cfg == IOMUX_CONFIG_GPIO)
-		writel(PIN_TO_ALT_GPIO(pin), mux_reg);
+	if (cfg & IOMUX_CONFIG_GPIO)
+		writel((cfg & ~IOMUX_CONFIG_GPIO) | PIN_TO_ALT_GPIO(pin),
+			mux_reg);
 	else
 		writel(cfg, mux_reg);
 }
arch/arm/include/asm/arch-mx5/iomux.h +1 -1
index e3765a3..8b6e56a 100644
@@ -40,7 +40,7 @@ typedef enum iomux_config {
 	IOMUX_CONFIG_ALT5,	/*!< used as alternate function 5 */
 	IOMUX_CONFIG_ALT6,	/*!< used as alternate function 6 */
 	IOMUX_CONFIG_ALT7,	/*!< used as alternate function 7 */
-	IOMUX_CONFIG_GPIO,	/*!< added to help user use GPIO mode */
+	IOMUX_CONFIG_GPIO = 0x1 << 3,	/*!< added to help user use GPIO mode */
 	IOMUX_CONFIG_SION = 0x1 << 4,	/*!< used as LOOPBACK:MUX SION bit */
 } iomux_pin_cfg_t;
 
#1 lore raw

PATCH 1/1 mx5 iomux: Fix GPIO with SION

Quoted 9 lines · Beno?t Th?baudeau

IOMUX_CONFIG_GPIO may be used together with IOMUX_CONFIG_SION, so don't break the iomux function value set to the register in this case.

Define the value of IOMUX_CONFIG_GPIO in a way showing that it should have only 1 bit set.

---

Hi Beno?t,

to my understanding. The SION bit is used to set a predefined function, independently from the value of the iomux. So I have the choice to set the SION bit or I select the function I want, including GPIO, clearing the SION bit, and setting optionally the daisy chain register if the GPIO requires.

So why should we use both SION and GPIO ?

Regards,
Stefano

Quoted 34 lines
 .../arch/arm/cpu/armv7/mx5/iomux.c                 |    5 +++--
 .../arch/arm/include/asm/arch-mx5/iomux.h          |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)
arch/arm/cpu/armv7/mx5/iomux.c +3 -2
index d4e3bbb..7adf08f 100644
@@ -124,8 +124,9 @@ static void iomux_config_mux(iomux_pin_name_t pin, iomux_pin_cfg_t cfg)
 
 	if ((mux_reg > get_mux_end()) || (mux_reg < IOMUXSW_MUX_CTL))
 		return ;
-	if (cfg == IOMUX_CONFIG_GPIO)
-		writel(PIN_TO_ALT_GPIO(pin), mux_reg);
+	if (cfg & IOMUX_CONFIG_GPIO)
+		writel((cfg & ~IOMUX_CONFIG_GPIO) | PIN_TO_ALT_GPIO(pin),
+			mux_reg);
 	else
 		writel(cfg, mux_reg);
 }
arch/arm/include/asm/arch-mx5/iomux.h +1 -1
index e3765a3..8b6e56a 100644
@@ -40,7 +40,7 @@ typedef enum iomux_config {
 	IOMUX_CONFIG_ALT5,	/*!< used as alternate function 5 */
 	IOMUX_CONFIG_ALT6,	/*!< used as alternate function 6 */
 	IOMUX_CONFIG_ALT7,	/*!< used as alternate function 7 */
-	IOMUX_CONFIG_GPIO,	/*!< added to help user use GPIO mode */
+	IOMUX_CONFIG_GPIO = 0x1 << 3,	/*!< added to help user use GPIO mode */
 	IOMUX_CONFIG_SION = 0x1 << 4,	/*!< used as LOOPBACK:MUX SION bit */
 } iomux_pin_cfg_t;
 
Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#2 lore raw

PATCH 1/1 mx5 iomux: Fix GPIO with SION

Hi Stefano,

Quoted 24 lines · Beno?t Th?baudeau
Quoted 11 lines

IOMUX_CONFIG_GPIO may be used together with IOMUX_CONFIG_SION, so don't break
the iomux function value set to the register in this case.

Define the value of IOMUX_CONFIG_GPIO in a way showing that it should have
only 1 bit set.

---

Hi Beno?t,

to my understanding. The SION bit is used to set a predefined function,
independently from the value of the iomux. So I have the choice to set
the SION bit or I select the function I want, including GPIO, clearing
the SION bit, and setting optionally the daisy chain register if the GPIO requires.

Quoted 1 line

So why should we use both SION and GPIO ?

No. See "A.3.2 SW Loopback through SION Bit" and "Figure A-3. IOMUX Cell Block Diagram" in the i.MX51 RM.

Whether SION is set or not, the selected IOMUX function will drive the pin.

If SION is cleared, the input from the pin will be either disabled or go to the selected IOMUX function depending on the activation of the input by this function.

If SION is set, the input from the pin is always enabled and goes to all IOMUX alternate functions at once (if their input connection to this pin is activated through the daisy chain).

So SION does not invalidate the function bit-field.

Then, you could wonder what kind of real life use case could be useful with both SION and GPIO set. This could be used for instance as a workaround to an erratum if an IOMUX function does not drive its output properly, but it needs to read back the pin status to work fine. Thus, the GPIO function output could be used to drive the pin, with SION set so that the flawed IOMUX function can still probe the pin and function properly internally. Note that it's only a theoretical example; I don't remember such an erratum.

Best regards,
Beno?t

#3 lore raw

PATCH 1/1 mx5 iomux: Fix GPIO with SION

Quoted 2 lines · Beno?t Th?baudeau

Hi Stefano,

Hi Beno?t,

Quoted 7 lines
Quoted 2 lines

So why should we use both SION and GPIO ?

No. See "A.3.2 SW Loopback through SION Bit" and "Figure A-3. IOMUX Cell Block Diagram" in the i.MX51 RM.

Whether SION is set or not, the selected IOMUX function will drive the pin.

And this is clear..

Quoted 4 lines

If SION is cleared, the input from the pin will be either disabled or go to the selected IOMUX function depending on the activation of the input by this function.

and also thi point is clear.

Quoted 4 lines

If SION is set, the input from the pin is always enabled and goes to all IOMUX alternate functions at once (if their input connection to this pin is activated through the daisy chain).

but I am asking myself why I should do this, that is the function drive the pin, using the input as source for another funtion.

Quoted 5 lines

So SION does not invalidate the function bit-field.

Then, you could wonder what kind of real life use case could be useful with both SION and GPIO set.

This is exactly the point !

Quoted 6 lines

This could be used for instance as a workaround to an erratum
if an IOMUX function does not drive its output properly, but it needs to read back the pin status to work fine. Thus, the GPIO function output could be used to drive the pin, with SION set so that the flawed IOMUX function can still probe the pin and function properly internally. Note that it's only a theoretical example; I don't remember such an erratum.

I am really impressed about your attention reading the manuals, but we have the rule in u-boot that we add code / feature when we have a use case (the same is in kernel). At the moment, it is pure theory, and nobody will use it. We will reconsider this patch when its introduction will be required to fix a SOC bug, if any.

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================
#4 lore raw

PATCH 1/1 mx5 iomux: Fix GPIO with SION

Hi Stefano,

Quoted 58 lines · Beno?t Th?baudeau
Quoted 2 lines

Hi Stefano,

Hi Beno?t,

Quoted 9 lines
Quoted 2 lines

So why should we use both SION and GPIO ?

No. See "A.3.2 SW Loopback through SION Bit" and "Figure A-3. IOMUX Cell Block
Diagram" in the i.MX51 RM.

Whether SION is set or not, the selected IOMUX function will drive the pin.

And this is clear..

Quoted 6 lines

If SION is cleared, the input from the pin will be either disabled or go to the
selected IOMUX function depending on the activation of the input by this
function.

and also thi point is clear.

Quoted 6 lines

If SION is set, the input from the pin is always enabled and goes to all IOMUX
alternate functions at once (if their input connection to this pin is activated
through the daisy chain).

but I am asking myself why I should do this, that is the function drive
the pin, using the input as source for another funtion.

Quoted 6 lines

So SION does not invalidate the function bit-field.

Then, you could wonder what kind of real life use case could be useful with both
SION and GPIO set.

This is exactly the point !

Quoted 10 lines

This could be used for instance as a workaround to an erratum if an IOMUX function does not drive its output properly, but it needs to read
back the pin status to work fine. Thus, the GPIO function output could be used
to drive the pin, with SION set so that the flawed IOMUX function can still
probe the pin and function properly internally. Note that it's only a
theoretical example; I don't remember such an erratum.

Quoted 7 lines

I am really impressed about your attention reading the manuals, but we
have the rule in u-boot that we add code / feature when we have a use case (the same is in kernel). At the moment, it is pure theory, and nobody will use it. We will reconsider this patch when its introduction
will be required to fix a SOC bug, if any.

Sure, but this is neither new code nor a new feature. It is only a fix that makes sure that the code won't break any potential use case in the future. This is weird to keep a known issue in the code until someone gets in trouble because of it and hence wastes time.

But anyway, this file will probably disappear in the near future because of the new pin definitions, so that won't make a big difference.

Best regards,
Beno?t