[U-Boot-Users] i.MX31: mx31_gpio_mux() problem
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(-)
diff --git a/board/imx31_litekit/imx31_litekit.c b/board/imx31_litekit/imx31_litekit.c index 263dd9f..09ce6f8 100644 --- a/board/imx31_litekit/imx31_litekit.c +++ b/board/imx31_litekit/imx31_litekit.c @@ -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); diff --git a/board/mx31ads/mx31ads.c b/board/mx31ads/mx31ads.c index dd0e150..b29d1b0 100644 --- a/board/mx31ads/mx31ads.c +++ b/board/mx31ads/mx31ads.c @@ -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); diff --git a/cpu/arm1136/mx31/generic.c b/cpu/arm1136/mx31/generic.c index 29c08c1..f897a2a 100644 --- a/cpu/arm1136/mx31/generic.c +++ b/cpu/arm1136/mx31/generic.c @@ -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; }
diff --git a/include/asm-arm/arch-mx31/mx31-regs.h b/include/asm-arm/arch-mx31/mx31-regs.h index 02b7dcb..b1c6327 100644 --- a/include/asm-arm/arch-mx31/mx31-regs.h +++ b/include/asm-arm/arch-mx31/mx31-regs.h @@ -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
On Wed, Jun 18, 2008 at 10:21:10PM +0200, Magnus Lilja wrote:
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).
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.
/* 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);
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.
-#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
On Thu, Jun 19, 2008 at 2:34 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Wed, Jun 18, 2008 at 10:21:10PM +0200, Magnus Lilja wrote:
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).
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
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
On Thu, Jun 19, 2008 at 11:02:02PM +0200, Magnus Lilja wrote:
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:
Seems to prove my point ;)
#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.
Ack
Sascha
participants (2)
-
Magnus Lilja -
Sascha Hauer