Skip to contentSkip to conversation

i.MX31: mx31_gpio_mux() problem

U-Boot-Users Magnus Lilja 2 participants 5 messages Jun 18, 2008 – Jun 20, 2008 Arm i.MX and Layerscape

lore
Share & tools
lore

Magnus Lilja

author
#0 lore raw

Hi all

While I was using the mx31_gpio_mux() function in cpu/arm1136/mx31/generic.c to modify the IOMUX for some IO pads I discovered that only the first 256 pads can be modified by this function.

It's pretty easy to fix this (see patch below) but I wonder if it's really the right way to fix this problem. Currently mx31_gpio_mux() takes only one argument, mode, which contains both the pad number and the new mode.

Current use:
mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);

(the patch below changes << 8 to << 9 and modifies the mx31_gpiomux() function accordingly)

Perhaps it's better to split the above into two arguments, pad number and pad mode, the function will then be called with:
mx31_gpio_mux(MUX_CTL_CSPI2_MISO, MUX_CTL_FUNC);

I prefer the second approach (i.e. not the one in the patch below).

Any comments on which solution we prefer?

Regards, Magnus Lilja

---

 board/imx31_litekit/imx31_litekit.c   |   14 +++++++-------
 board/mx31ads/mx31ads.c               |   14 +++++++-------
 cpu/arm1136/mx31/generic.c            |    4 ++--
 include/asm-arm/arch-mx31/mx31-regs.h |   12 ++++++------
 4 files changed, 22 insertions(+), 22 deletions(-)
board/imx31_litekit/imx31_litekit.c +7 -7
index 263dd9f..09ce6f8 100644
@@ -53,13 +53,13 @@ int board_init (void)
 	mx31_gpio_mux(MUX_RTS1__UART1_CTS_B);
 
 	/* SPI2 */
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS2);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SCLK);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SPI_RDY);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MOSI);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS0);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS1);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS2);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SCLK);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SPI_RDY);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MOSI);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MISO);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS0);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS1);
 
 	/* start SPI2 clock */
 	__REG(CCM_CGR2) = __REG(CCM_CGR2) | (3 << 4);
board/mx31ads/mx31ads.c +7 -7
index dd0e150..b29d1b0 100644
@@ -58,13 +58,13 @@ int board_init (void)
 	mx31_gpio_mux(MUX_RTS1__UART1_CTS_B);
 
 	/* SPI2 */
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS2);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SCLK);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SPI_RDY);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MOSI);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS0);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS1);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS2);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SCLK);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SPI_RDY);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MOSI);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MISO);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS0);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS1);
 
 	/* start SPI2 clock */
 	__REG(CCM_CGR2) = __REG(CCM_CGR2) | (3 << 4);
cpu/arm1136/mx31/generic.c +2 -2
index 29c08c1..f897a2a 100644
@@ -81,12 +81,12 @@ void mx31_gpio_mux(unsigned long mode)
 {
 	unsigned long reg, shift, tmp;
 
-	reg = IOMUXC_BASE + (mode & 0xfc);
+	reg = IOMUXC_BASE + (mode & 0x1fc);
 	shift = (~mode & 0x3) * 8;
 
 	tmp = __REG(reg);
 	tmp &= ~(0xff << shift);
-	tmp |= ((mode >> 8) & 0xff) << shift;
+	tmp |= ((mode >> 9) & 0xff) << shift;
 	__REG(reg) = tmp;
 }
include/asm-arm/arch-mx31/mx31-regs.h +6 -6
index 02b7dcb..b1c6327 100644
@@ -130,13 +130,13 @@
  * these macros can be used in mx31_gpio_mux() and have the form
  * MUX_[contact name]__[pin function]
  */
-#define MUX_RXD1__UART1_RXD_MUX	((MUX_CTL_FUNC << 8) | MUX_CTL_RXD1)
-#define MUX_TXD1__UART1_TXD_MUX	((MUX_CTL_FUNC << 8) | MUX_CTL_TXD1)
-#define MUX_RTS1__UART1_RTS_B	((MUX_CTL_FUNC << 8) | MUX_CTL_RTS1)
-#define MUX_RTS1__UART1_CTS_B	((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)
+#define MUX_RXD1__UART1_RXD_MUX	((MUX_CTL_FUNC << 9) | MUX_CTL_RXD1)
+#define MUX_TXD1__UART1_TXD_MUX	((MUX_CTL_FUNC << 9) | MUX_CTL_TXD1)
+#define MUX_RTS1__UART1_RTS_B	((MUX_CTL_FUNC << 9) | MUX_CTL_RTS1)
+#define MUX_RTS1__UART1_CTS_B	((MUX_CTL_FUNC << 9) | MUX_CTL_CTS1)
 
-#define MUX_CSPI2_MOSI__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MOSI)
-#define MUX_CSPI2_MISO__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MISO)
+#define MUX_CSPI2_MOSI__I2C2_SCL ((MUX_CTL_ALT1 << 9) | MUX_CTL_CSPI2_MOSI)
+#define MUX_CSPI2_MISO__I2C2_SCL ((MUX_CTL_ALT1 << 9) | MUX_CTL_CSPI2_MISO)
 
 /*
  * Memory regions and CS
maintainer
#1 lore raw
Quoted 10 lines · Magnus Lilja

Hi all

While I was using the mx31_gpio_mux() function in cpu/arm1136/mx31/generic.c to modify the IOMUX for some IO pads I discovered that only the first 256 pads can be modified by this function.

It's pretty easy to fix this (see patch below) but I wonder if it's really the right way to fix this problem. Currently mx31_gpio_mux() takes only one argument, mode, which contains both the pad number and the new mode.

Current use:
mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);

Quoted 6 lines

(the patch below changes << 8 to << 9 and modifies the mx31_gpiomux() function accordingly)

Perhaps it's better to split the above into two arguments, pad number and pad mode, the function will then be called with:
mx31_gpio_mux(MUX_CTL_CSPI2_MISO, MUX_CTL_FUNC);

I prefer the second approach (i.e. not the one in the patch below).

The first approach has the advantage that you can define convenience macros like MUX_RXD1__UART1_RXD_MUX which makes it easy to setup new boards by only reading the definition instead of crawling the datasheet for Alternate function assignments once the list of defined is somewhat completed. Looking in the datasheet for every new pin is a pain and error prone.

Quoted 5 lines
board/imx31_litekit/imx31_litekit.c +0 -4
 	/* SPI2 */
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS2);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SCLK);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SPI_RDY);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MOSI);

board/imx31_litekit/imx31_litekit.c int board_init (void) · line 62

Quoted 10 lines
board/imx31_litekit/imx31_litekit.c +7 -3
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS0);
-	mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_SS1);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS2);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SCLK);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SPI_RDY);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MOSI);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_MISO);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS0);
+	mx31_gpio_mux((MUX_CTL_FUNC << 9) | MUX_CTL_CSPI2_SS1);

And of course my intention was to move such things into include/asm-arm/arch-mx31/mx31-regs.h so that other people can make use of it.

include/asm-arm/arch-mx31/mx31-regs.h line 136

Quoted 8 lines
include/asm-arm/arch-mx31/mx31-regs.h +4 -4
-#define MUX_RXD1__UART1_RXD_MUX	((MUX_CTL_FUNC << 8) | MUX_CTL_RXD1)
-#define MUX_TXD1__UART1_TXD_MUX	((MUX_CTL_FUNC << 8) | MUX_CTL_TXD1)
-#define MUX_RTS1__UART1_RTS_B	((MUX_CTL_FUNC << 8) | MUX_CTL_RTS1)
-#define MUX_RTS1__UART1_CTS_B	((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)
+#define MUX_RXD1__UART1_RXD_MUX	((MUX_CTL_FUNC << 9) | MUX_CTL_RXD1)
+#define MUX_TXD1__UART1_TXD_MUX	((MUX_CTL_FUNC << 9) | MUX_CTL_TXD1)
+#define MUX_RTS1__UART1_RTS_B	((MUX_CTL_FUNC << 9) | MUX_CTL_RTS1)
+#define MUX_RTS1__UART1_CTS_B	((MUX_CTL_FUNC << 9) | MUX_CTL_CTS1)

...like these examples here.

Sascha

Signature · 4 lines
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-
author
#2 lore raw
Quoted 16 lines · Sascha Hauer
Quoted 15 lines · Magnus Lilja

Hi all

While I was using the mx31_gpio_mux() function in cpu/arm1136/mx31/generic.c to modify the IOMUX for some IO pads I discovered that only the first 256 pads can be modified by this function.

It's pretty easy to fix this (see patch below) but I wonder if it's really the right way to fix this problem. Currently mx31_gpio_mux() takes only one argument, mode, which contains both the pad number and the new mode.

Current use:
mx31_gpio_mux((MUX_CTL_FUNC << 8) | MUX_CTL_CSPI2_MISO);

(the patch below changes << 8 to << 9 and modifies the mx31_gpiomux() function accordingly)

Perhaps it's better to split the above into two arguments, pad number and pad mode, the function will then be called with:
mx31_gpio_mux(MUX_CTL_CSPI2_MISO, MUX_CTL_FUNC);

Quoted 8 lines
Quoted 1 line

I prefer the second approach (i.e. not the one in the patch below).

The first approach has the advantage that you can define convenience macros like MUX_RXD1__UART1_RXD_MUX which makes it easy to setup new boards by only reading the definition instead of crawling the datasheet for Alternate function assignments once the list of defined is somewhat completed. Looking in the datasheet for every new pin is a pain and error prone.

Thanks for the background info. I'll try to submit a patch in a couple of days (using the first approach).

/Magnus

author
#3 lore raw
Quoted 10 lines
Quoted 6 lines

The first approach has the advantage that you can define convenience macros like MUX_RXD1__UART1_RXD_MUX which makes it easy to setup new boards by only reading the definition instead of crawling the datasheet for Alternate function assignments once the list of defined is somewhat completed. Looking in the datasheet for every new pin is a pain and error prone.

Thanks for the background info. I'll try to submit a patch in a couple of days (using the first approach).

Seems to like there are some typos in the current convenience macros already:

#define MUX_RTS1__UART1_RTS_B   ((MUX_CTL_FUNC << 8) | MUX_CTL_RTS1)
#define MUX_RTS1__UART1_CTS_B   ((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)

The last one should be
#define MUX_CTS1__UART1_CTS_B ((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)

More..
#define MUX_CSPI2_MOSI__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MOSI) #define MUX_CSPI2_MISO__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MISO)

The last one should be:
#define MUX_CSPI2_MISO__I2C2_SDA ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MISO)

I'll include that in my patch as well if there are no objections.

/Magnus

maintainer
#4 lore raw
Quoted 8 lines · Magnus Lilja
Quoted 8 lines
Quoted 6 lines

The first approach has the advantage that you can define convenience macros like MUX_RXD1__UART1_RXD_MUX which makes it easy to setup new boards by only reading the definition instead of crawling the datasheet for Alternate function assignments once the list of defined is somewhat completed. Looking in the datasheet for every new pin is a pain and error prone.

Quoted 4 lines
Quoted 2 lines

Thanks for the background info. I'll try to submit a patch in a couple of days (using the first approach).

Seems to like there are some typos in the current convenience macros already:

Seems to prove my point ;)

Quoted 11 lines
#define MUX_RTS1__UART1_RTS_B   ((MUX_CTL_FUNC << 8) | MUX_CTL_RTS1)
#define MUX_RTS1__UART1_CTS_B   ((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)

The last one should be
#define MUX_CTS1__UART1_CTS_B ((MUX_CTL_FUNC << 8) | MUX_CTL_CTS1)

More..
#define MUX_CSPI2_MOSI__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MOSI) #define MUX_CSPI2_MISO__I2C2_SCL ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MISO)

Quoted 5 lines

The last one should be:
#define MUX_CSPI2_MISO__I2C2_SDA ((MUX_CTL_ALT1 << 8) | MUX_CTL_CSPI2_MISO)

I'll include that in my patch as well if there are no objections.

Ack

Sascha

Signature · 4 lines
Pengutronix e.K. - Linux Solutions for Science and Industry
-----------------------------------------------------------
Kontakt-Informationen finden Sie im Header dieser Mail oder
auf der Webseite -> http://www.pengutronix.de/impressum/ <-