diff mbox

[U-Boot,v3,1/3] iomux-v3: Place pad control definitions into common file

Message ID 5165be73.ea80ec0a.04b3.ffffb7fd@mx.google.com
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show

Commit Message

Fabio Estevam April 10, 2013, 7:32 p.m. UTC 
From: Fabio Estevam <fabio.estevam@freescale.com>

Instead of having the same PAD control definition in each MX6 variant pin file,
place it into a common location.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
Changes since v2:
- None
Changes since v1:
- Add missing IOMUX_CONFIG_SION definition to avoid breakage on mx6dl

 arch/arm/include/asm/arch-mx6/mx6dl_pins.h |   27 ---------------------------
 arch/arm/include/asm/arch-mx6/mx6q_pins.h  |   27 ---------------------------
 arch/arm/include/asm/imx-common/iomux-v3.h |   27 ++++++++++++++++++++++++++-
 3 files changed, 26 insertions(+), 55 deletions(-)

Comments

Stefano Babic April 25, 2013, 8:27 p.m. UTC | #1 
On 10/04/2013 21:32, festevam@gmail.com wrote:
> From: Fabio Estevam <fabio.estevam@freescale.com>
> 
> Instead of having the same PAD control definition in each MX6 variant pin file,
> place it into a common location.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---

Hi Fabio,

> Changes since v2:
> - None
> Changes since v1:
> - Add missing IOMUX_CONFIG_SION definition to avoid breakage on mx6dl
> 
>  arch/arm/include/asm/arch-mx6/mx6dl_pins.h |   27 ---------------------------
>  arch/arm/include/asm/arch-mx6/mx6q_pins.h  |   27 ---------------------------
>  arch/arm/include/asm/imx-common/iomux-v3.h |   27 ++++++++++++++++++++++++++-
>  3 files changed, 26 insertions(+), 55 deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> index 9846f1b..0ed12f3 100644
> --- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> +++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> @@ -22,33 +22,6 @@
>  
>  #include <asm/imx-common/iomux-v3.h>
>  
> -/* Use to set PAD control */
> -#define PAD_CTL_HYS		(1 << 16)
> -#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
> -#define PAD_CTL_PUS_47K_UP	(1 << 14)
> -#define PAD_CTL_PUS_100K_UP	(2 << 14)
> -#define PAD_CTL_PUS_22K_UP	(3 << 14)
> -
> -#define PAD_CTL_PUE		(1 << 13)
> -#define PAD_CTL_PKE		(1 << 12)
> -#define PAD_CTL_ODE		(1 << 11)
> -#define PAD_CTL_SPEED_LOW	(1 << 6)
> -#define PAD_CTL_SPEED_MED	(2 << 6)
> -#define PAD_CTL_SPEED_HIGH	(3 << 6)
> -#define PAD_CTL_DSE_DISABLE	(0 << 3)
> -#define PAD_CTL_DSE_240ohm	(1 << 3)
> -#define PAD_CTL_DSE_120ohm	(2 << 3)
> -#define PAD_CTL_DSE_80ohm	(3 << 3)
> -#define PAD_CTL_DSE_60ohm	(4 << 3)
> -#define PAD_CTL_DSE_48ohm	(5 << 3)
> -#define PAD_CTL_DSE_40ohm	(6 << 3)
> -#define PAD_CTL_DSE_34ohm	(7 << 3)
> -#define PAD_CTL_SRE_FAST	(1 << 0)
> -#define PAD_CTL_SRE_SLOW	(0 << 0)
> -

I have applied this patch and I have found now that this breaks some
MX51 boards, such as efika. In fact, the same defines are set also in
/home/stefano/Projects/imx/u-boot-imx/include/asm/arch/iomux-mx51.h.

Checking in the manual, I see that values in iomux-mx51.h are correct,
and they simply differs from the ones of MX6. We can simply add a #ifdef
CONFIG_MX51 in arch/arm/include/asm/imx-common/iomux-v3.h to fix it, but
anyway I suppose that the mx51evk does not work correctly. Can you take
a look at this issue ?

Best regards,
Stefano
Benoît Thébaudeau April 25, 2013, 8:32 p.m. UTC | #2 
Hi Stefano,

On Thursday, April 25, 2013 10:27:12 PM, Stefano Babic wrote:
> On 10/04/2013 21:32, festevam@gmail.com wrote:
> > From: Fabio Estevam <fabio.estevam@freescale.com>
> > 
> > Instead of having the same PAD control definition in each MX6 variant pin
> > file,
> > place it into a common location.
> > 
> > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> 
> Hi Fabio,
> 
> > Changes since v2:
> > - None
> > Changes since v1:
> > - Add missing IOMUX_CONFIG_SION definition to avoid breakage on mx6dl
> > 
> >  arch/arm/include/asm/arch-mx6/mx6dl_pins.h |   27
> >  ---------------------------
> >  arch/arm/include/asm/arch-mx6/mx6q_pins.h  |   27
> >  ---------------------------
> >  arch/arm/include/asm/imx-common/iomux-v3.h |   27
> >  ++++++++++++++++++++++++++-
> >  3 files changed, 26 insertions(+), 55 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > index 9846f1b..0ed12f3 100644
> > --- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > +++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > @@ -22,33 +22,6 @@
> >  
> >  #include <asm/imx-common/iomux-v3.h>
> >  
> > -/* Use to set PAD control */
> > -#define PAD_CTL_HYS		(1 << 16)
> > -#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
> > -#define PAD_CTL_PUS_47K_UP	(1 << 14)
> > -#define PAD_CTL_PUS_100K_UP	(2 << 14)
> > -#define PAD_CTL_PUS_22K_UP	(3 << 14)
> > -
> > -#define PAD_CTL_PUE		(1 << 13)
> > -#define PAD_CTL_PKE		(1 << 12)
> > -#define PAD_CTL_ODE		(1 << 11)
> > -#define PAD_CTL_SPEED_LOW	(1 << 6)
> > -#define PAD_CTL_SPEED_MED	(2 << 6)
> > -#define PAD_CTL_SPEED_HIGH	(3 << 6)
> > -#define PAD_CTL_DSE_DISABLE	(0 << 3)
> > -#define PAD_CTL_DSE_240ohm	(1 << 3)
> > -#define PAD_CTL_DSE_120ohm	(2 << 3)
> > -#define PAD_CTL_DSE_80ohm	(3 << 3)
> > -#define PAD_CTL_DSE_60ohm	(4 << 3)
> > -#define PAD_CTL_DSE_48ohm	(5 << 3)
> > -#define PAD_CTL_DSE_40ohm	(6 << 3)
> > -#define PAD_CTL_DSE_34ohm	(7 << 3)
> > -#define PAD_CTL_SRE_FAST	(1 << 0)
> > -#define PAD_CTL_SRE_SLOW	(0 << 0)
> > -
> 
> I have applied this patch and I have found now that this breaks some
> MX51 boards, such as efika. In fact, the same defines are set also in
> /home/stefano/Projects/imx/u-boot-imx/include/asm/arch/iomux-mx51.h.
> 
> Checking in the manual, I see that values in iomux-mx51.h are correct,
> and they simply differs from the ones of MX6. We can simply add a #ifdef
> CONFIG_MX51 in arch/arm/include/asm/imx-common/iomux-v3.h to fix it, but
> anyway I suppose that the mx51evk does not work correctly. Can you take
> a look at this issue ?

I have already made a patch for that among other things. I will post it
tomorrow.

Best regards,
Benoît
Benoît Thébaudeau April 25, 2013, 9:51 p.m. UTC | #3 
On Thursday, April 25, 2013 10:32:50 PM, Benoît Thébaudeau wrote:
> Hi Stefano,
> 
> On Thursday, April 25, 2013 10:27:12 PM, Stefano Babic wrote:
> > On 10/04/2013 21:32, festevam@gmail.com wrote:
> > > From: Fabio Estevam <fabio.estevam@freescale.com>
> > > 
> > > Instead of having the same PAD control definition in each MX6 variant pin
> > > file,
> > > place it into a common location.
> > > 
> > > Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> > > ---
> > 
> > Hi Fabio,
> > 
> > > Changes since v2:
> > > - None
> > > Changes since v1:
> > > - Add missing IOMUX_CONFIG_SION definition to avoid breakage on mx6dl
> > > 
> > >  arch/arm/include/asm/arch-mx6/mx6dl_pins.h |   27
> > >  ---------------------------
> > >  arch/arm/include/asm/arch-mx6/mx6q_pins.h  |   27
> > >  ---------------------------
> > >  arch/arm/include/asm/imx-common/iomux-v3.h |   27
> > >  ++++++++++++++++++++++++++-
> > >  3 files changed, 26 insertions(+), 55 deletions(-)
> > > 
> > > diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > > b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > > index 9846f1b..0ed12f3 100644
> > > --- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > > +++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
> > > @@ -22,33 +22,6 @@
> > >  
> > >  #include <asm/imx-common/iomux-v3.h>
> > >  
> > > -/* Use to set PAD control */
> > > -#define PAD_CTL_HYS		(1 << 16)
> > > -#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
> > > -#define PAD_CTL_PUS_47K_UP	(1 << 14)
> > > -#define PAD_CTL_PUS_100K_UP	(2 << 14)
> > > -#define PAD_CTL_PUS_22K_UP	(3 << 14)
> > > -
> > > -#define PAD_CTL_PUE		(1 << 13)
> > > -#define PAD_CTL_PKE		(1 << 12)
> > > -#define PAD_CTL_ODE		(1 << 11)
> > > -#define PAD_CTL_SPEED_LOW	(1 << 6)
> > > -#define PAD_CTL_SPEED_MED	(2 << 6)
> > > -#define PAD_CTL_SPEED_HIGH	(3 << 6)
> > > -#define PAD_CTL_DSE_DISABLE	(0 << 3)
> > > -#define PAD_CTL_DSE_240ohm	(1 << 3)
> > > -#define PAD_CTL_DSE_120ohm	(2 << 3)
> > > -#define PAD_CTL_DSE_80ohm	(3 << 3)
> > > -#define PAD_CTL_DSE_60ohm	(4 << 3)
> > > -#define PAD_CTL_DSE_48ohm	(5 << 3)
> > > -#define PAD_CTL_DSE_40ohm	(6 << 3)
> > > -#define PAD_CTL_DSE_34ohm	(7 << 3)
> > > -#define PAD_CTL_SRE_FAST	(1 << 0)
> > > -#define PAD_CTL_SRE_SLOW	(0 << 0)
> > > -
> > 
> > I have applied this patch and I have found now that this breaks some
> > MX51 boards, such as efika. In fact, the same defines are set also in
> > /home/stefano/Projects/imx/u-boot-imx/include/asm/arch/iomux-mx51.h.
> > 
> > Checking in the manual, I see that values in iomux-mx51.h are correct,
> > and they simply differs from the ones of MX6. We can simply add a #ifdef
> > CONFIG_MX51 in arch/arm/include/asm/imx-common/iomux-v3.h to fix it, but
> > anyway I suppose that the mx51evk does not work correctly. Can you take
> > a look at this issue ?
> 
> I have already made a patch for that among other things. I will post it
> tomorrow.

My patch consists in moving all iomux-v3 imx pad control definitions to
iomux-v3.h, like this:
#ifdef CONFIG_MX6
#define mx6 pad control stuff, which is common to mx6x
#else
#define non-mx6 pad control stuff, which is common to mx25/35/5x
#endif

I have also integrated PKE and PUE to pull value definitions on i.MX6, just like
on i.MX51 and in mainline Linux, which made me notice that some mx6 boards
define a SPI_PAD_CTRL with PAD_CTL_PUS_100K_DOWN but without PAD_CTL_PKE and
PAD_CTL_PUE, which means that no pull is actually enabled. Hence, I have just
dropped the PAD_CTL_PUS_100K_DOWN from those SPI_PAD_CTRL in order to have no
change of behavior, but there might be a bug to fix on those boards.

I am working on converting mx25/35/5x to imx-common iomux-v3. I have noticed in
arch/arm/cpu/armv7/mx5/iomux.c some "is_soc_rev(CHIP_REV_2_0)" that make this
change quite complicated for mx51. There is no such thing in mainline Linux.
Fabio, is it OK to drop this support for older mx51 revisions in mainline
U-Boot?

Best regards,
Benoît
Fabio Estevam April 25, 2013, 10 p.m. UTC | #4 
Hi Benoît,

On Thu, Apr 25, 2013 at 6:51 PM, Benoît Thébaudeau
<benoit.thebaudeau@advansee.com> wrote:

> I am working on converting mx25/35/5x to imx-common iomux-v3. I have noticed in
> arch/arm/cpu/armv7/mx5/iomux.c some "is_soc_rev(CHIP_REV_2_0)" that make this
> change quite complicated for mx51. There is no such thing in mainline Linux.
> Fabio, is it OK to drop this support for older mx51 revisions in mainline
> U-Boot?

Yes, let's focus only on the production silicon version (TO3).

Thanks,

Fabio Estevam
Stefano Babic April 26, 2013, 6:50 a.m. UTC | #5 
On 25/04/2013 22:32, Benoît Thébaudeau wrote:

>> Checking in the manual, I see that values in iomux-mx51.h are correct,
>> and they simply differs from the ones of MX6. We can simply add a #ifdef
>> CONFIG_MX51 in arch/arm/include/asm/imx-common/iomux-v3.h to fix it, but
>> anyway I suppose that the mx51evk does not work correctly. Can you take
>> a look at this issue ?
> 
> I have already made a patch for that among other things. I will post it
> tomorrow.

You're great ! Thanks to fix problems when they happen (and also before
they happen...) !

Best regards,
Stefano
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
index 9846f1b..0ed12f3 100644
--- a/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
+++ b/arch/arm/include/asm/arch-mx6/mx6dl_pins.h
@@ -22,33 +22,6 @@ 
 
 #include <asm/imx-common/iomux-v3.h>
 
-/* Use to set PAD control */
-#define PAD_CTL_HYS		(1 << 16)
-#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
-#define PAD_CTL_PUS_47K_UP	(1 << 14)
-#define PAD_CTL_PUS_100K_UP	(2 << 14)
-#define PAD_CTL_PUS_22K_UP	(3 << 14)
-
-#define PAD_CTL_PUE		(1 << 13)
-#define PAD_CTL_PKE		(1 << 12)
-#define PAD_CTL_ODE		(1 << 11)
-#define PAD_CTL_SPEED_LOW	(1 << 6)
-#define PAD_CTL_SPEED_MED	(2 << 6)
-#define PAD_CTL_SPEED_HIGH	(3 << 6)
-#define PAD_CTL_DSE_DISABLE	(0 << 3)
-#define PAD_CTL_DSE_240ohm	(1 << 3)
-#define PAD_CTL_DSE_120ohm	(2 << 3)
-#define PAD_CTL_DSE_80ohm	(3 << 3)
-#define PAD_CTL_DSE_60ohm	(4 << 3)
-#define PAD_CTL_DSE_48ohm	(5 << 3)
-#define PAD_CTL_DSE_40ohm	(6 << 3)
-#define PAD_CTL_DSE_34ohm	(7 << 3)
-#define PAD_CTL_SRE_FAST	(1 << 0)
-#define PAD_CTL_SRE_SLOW	(0 << 0)
-
-#define IOMUX_CONFIG_SION 0x10
-#define NO_MUX_I                0
-#define NO_PAD_I                0
 enum {
 	MX6_PAD_CSI0_DAT10__UART1_TXD		= IOMUX_PAD(0x0360, 0x004C, 3, 0x0000, 0, 0),
 	MX6_PAD_CSI0_DAT11__UART1_RXD		= IOMUX_PAD(0x0364, 0x0050, 3, 0x08FC, 1, 0),
diff --git a/arch/arm/include/asm/arch-mx6/mx6q_pins.h b/arch/arm/include/asm/arch-mx6/mx6q_pins.h
index 1c1c008..02a40d4 100644
--- a/arch/arm/include/asm/arch-mx6/mx6q_pins.h
+++ b/arch/arm/include/asm/arch-mx6/mx6q_pins.h
@@ -24,33 +24,6 @@ 
 
 #include <asm/imx-common/iomux-v3.h>
 
-/* Use to set PAD control */
-#define PAD_CTL_HYS		(1 << 16)
-#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
-#define PAD_CTL_PUS_47K_UP	(1 << 14)
-#define PAD_CTL_PUS_100K_UP	(2 << 14)
-#define PAD_CTL_PUS_22K_UP	(3 << 14)
-
-#define PAD_CTL_PUE		(1 << 13)
-#define PAD_CTL_PKE		(1 << 12)
-#define PAD_CTL_ODE		(1 << 11)
-#define PAD_CTL_SPEED_LOW	(1 << 6)
-#define PAD_CTL_SPEED_MED	(2 << 6)
-#define PAD_CTL_SPEED_HIGH	(3 << 6)
-#define PAD_CTL_DSE_DISABLE	(0 << 3)
-#define PAD_CTL_DSE_240ohm	(1 << 3)
-#define PAD_CTL_DSE_120ohm	(2 << 3)
-#define PAD_CTL_DSE_80ohm	(3 << 3)
-#define PAD_CTL_DSE_60ohm	(4 << 3)
-#define PAD_CTL_DSE_48ohm	(5 << 3)
-#define PAD_CTL_DSE_40ohm	(6 << 3)
-#define PAD_CTL_DSE_34ohm	(7 << 3)
-#define PAD_CTL_SRE_FAST	(1 << 0)
-#define PAD_CTL_SRE_SLOW	(0 << 0)
-
-#define NO_MUX_I                0
-#define NO_PAD_I                0
-
 enum {
 	MX6_PAD_SD2_DAT1__USDHC2_DAT1		= IOMUX_PAD(0x0360, 0x004C, 0, 0x0000, 0, 0),
 	MX6_PAD_SD2_DAT1__ECSPI5_SS0		= IOMUX_PAD(0x0360, 0x004C, 1, 0x0834, 0, 0),
diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h
index c34bb76..36917c8 100644
--- a/arch/arm/include/asm/imx-common/iomux-v3.h
+++ b/arch/arm/include/asm/imx-common/iomux-v3.h
@@ -95,7 +95,32 @@  typedef u64 iomux_v3_cfg_t;
 #define GPIO_PORTE		(4 << GPIO_PORT_SHIFT)
 #define GPIO_PORTF		(5 << GPIO_PORT_SHIFT)
 
-#define MUX_CONFIG_SION		(0x1 << 4)
+#define PAD_CTL_HYS		(1 << 16)
+#define PAD_CTL_PUS_100K_DOWN	(0 << 14)
+#define PAD_CTL_PUS_47K_UP	(1 << 14)
+#define PAD_CTL_PUS_100K_UP	(2 << 14)
+#define PAD_CTL_PUS_22K_UP	(3 << 14)
+
+#define PAD_CTL_PUE		(1 << 13)
+#define PAD_CTL_PKE		(1 << 12)
+#define PAD_CTL_ODE		(1 << 11)
+#define PAD_CTL_SPEED_LOW	(1 << 6)
+#define PAD_CTL_SPEED_MED	(2 << 6)
+#define PAD_CTL_SPEED_HIGH	(3 << 6)
+#define PAD_CTL_DSE_DISABLE	(0 << 3)
+#define PAD_CTL_DSE_240ohm	(1 << 3)
+#define PAD_CTL_DSE_120ohm	(2 << 3)
+#define PAD_CTL_DSE_80ohm	(3 << 3)
+#define PAD_CTL_DSE_60ohm	(4 << 3)
+#define PAD_CTL_DSE_48ohm	(5 << 3)
+#define PAD_CTL_DSE_40ohm	(6 << 3)
+#define PAD_CTL_DSE_34ohm	(7 << 3)
+#define PAD_CTL_SRE_FAST	(1 << 0)
+#define PAD_CTL_SRE_SLOW	(0 << 0)
+
+#define IOMUX_CONFIG_SION	0x10
+#define NO_MUX_I		0
+#define NO_PAD_I		0
 
 int imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
 int imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,