arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

PATCH New Hannes Schmelzer 4 participants 8 messages Jun 22, 2016 – Jul 21, 2016 Arm (32-bit)

lore

Share & tools

lore

Hannes Schmelzer

author

2016-06-22 10:07 UTC #0 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.

Signed-off-by Hannes Schmelzer <[email protected]>

---

 arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/include/asm/arch-mx6/mx6-pins.h +2 -2

index 4b6bb18..3465205 100644
@@ -18,7 +18,7 @@ enum {
 #include "mx6q_pins.h"
 #undef MX6_PAD_DECL
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6DL_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };
 #elif defined(CONFIG_MX6Q)

@@ -30,7 +30,7 @@ enum {
 #elif defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
 enum {
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };
 #elif defined(CONFIG_MX6SL)

Signature · 1 line

1.9.1

Stefano Babic

2016-07-11 14:42 UTC #1 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

Quoted 24 lines · Hannes Schmelzer

if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.

Signed-off-by Hannes Schmelzer <[email protected]>

---

 arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1

index 4b6bb18..3465205 100644
@@ -18,7 +18,7 @@ enum {
 #include "mx6q_pins.h"
 #undef MX6_PAD_DECL
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6DL_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };
 #elif defined(CONFIG_MX6Q)

arch/arm/include/asm/arch-mx6/mx6-pins.h enum { · line 36

Quoted 8 lines

arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1

@@ -30,7 +30,7 @@ enum {
 #elif defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
 enum {
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };
 #elif defined(CONFIG_MX6SL)

Applied to u-boot-imx, -next branch, thanks !

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

Otavio Salvador

2016-07-11 14:44 UTC #2 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

In reply to Hannes Schmelzer

Quoted 4 lines · Hannes Schmelzer

if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.
Signed-off-by Hannes Schmelzer <[email protected]>

Please put this for the current release; this is a bugfix and a critical one.

Signature · 3 lines

Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750

Stefano Babic

2016-07-20 07:30 UTC #3 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

In reply to Hannes Schmelzer

Hi Hannes,

this patch breaks most i.MX6 boards (the not DL) and I revert it. Maybe I had to ask better before, anyway:

Quoted 23 lines · Hannes Schmelzer

if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.

Signed-off-by Hannes Schmelzer <[email protected]>

---

 arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1

index 4b6bb18..3465205 100644
@@ -18,7 +18,7 @@ enum {
 #include "mx6q_pins.h"
 #undef MX6_PAD_DECL
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6DL_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };
 #elif defined(CONFIG_MX6Q)

arch/arm/include/asm/arch-mx6/mx6-pins.h enum { · line 35

Quoted 7 lines

arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1

@@ -30,7 +30,7 @@ enum {
 #elif defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
 enum {
 #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
 #include "mx6dl_pins.h"
 };

Can you better explain the problem you had ? The name is not decisive - the important thing is that the correct include file with the right layout is included, that means mx6dl_pins.h. And this was mainlined since a lot of time.

We have several boards with 6DL into mainline, so I am missing which is your problem.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

Hannes Schmelzer

2016-07-20 20:51 UTC #4 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

Quoted 1 line · Stefano Babic

Hi Hannes,

Hi Stefano,

Quoted 2 lines

this patch breaks most i.MX6 boards (the not DL) and I revert it. Maybe I had to ask better before, anyway:

sorry for inconvenience, i should have done more testing on this. I just tried to compile several i.mx6 boards and found out that I did not covered the MX6S which are in trouble now.

Quoted 27 lines · Hannes Schmelzer

Quoted 26 lines

if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.

Signed-off-by Hannes Schmelzer<[email protected]>

---

  arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1

index 4b6bb18..3465205 100644
@@ -18,7 +18,7 @@ enum {
  #include "mx6q_pins.h"
  #undef MX6_PAD_DECL
  #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-	MX6_PAD_DECLARE(MX6DL_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
  #include "mx6dl_pins.h"
  };
  #elif defined(CONFIG_MX6Q)

@@ -30,7 +30,7 @@ enum {
  #elif defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
  enum {
  #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \

Quoted 8 lines

arch/arm/include/asm/arch-mx6/mx6-pins.h enum { · line 35
Quoted 4 lines
arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1
-	MX6_PAD_DECLARE(MX6_PAD_,name, pco, mc, mm, sio, si, pc),
+	MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
  #include "mx6dl_pins.h"
  };
Can you better explain the problem you had ? The name is not decisive - the important thing is that the correct include file with the right layout is included, that means mx6dl_pins.h. And this was mainlined since a lot of time.

Maybe all the time nobody had to use I2C #4 on an i.mx6 duallite chip, doing so i encountered this problem.

The name is decisive for sure, have a closer look to the "MX6_PAD_DECLARE" macro,
In conjunction with the correct include file the prefix used to form the final register table declaration.

Next the iomux-v3.h is from interest,
the "#define IOMUX_PADS" has dependency on CONFIG_MX6nn, here the previous definition out from mx6-pins.h is used.

I will send some V2 to address this topic fully, ok?

Quoted 5 lines

We have several boards with 6DL into mainline, so I am missing which is your problem.
Best regards,
Stefano Babic

cheers,
Hannes

Hannes Schmelzer

2016-07-21 06:10 UTC #5 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

Quoted 56 lines · Hannes Schmelzer

Quoted 1 line · Stefano Babic
Hi Hannes,
Hi Stefano,
Quoted 2 lines
this patch breaks most i.MX6 boards (the not DL) and I revert it. Maybe I had to ask better before, anyway:
sorry for inconvenience, i should have done more testing on this. I just tried to compile several i.mx6 boards and found out that I did not covered the MX6S which are in trouble now.
Quoted 28 lines · Hannes Schmelzer
Quoted 27 lines
if we build for an i.mx6 (d)ual(l)ite CONFIC_MX6DL we shall use MX6DL_PAD instead the common MX6_PAD.
Signed-off-by Hannes Schmelzer<[email protected]>
---
  arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1
index 4b6bb18..3465205 100644
@@ -18,7 +18,7 @@ enum {
  #include "mx6q_pins.h"
  #undef MX6_PAD_DECL
  #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
-    MX6_PAD_DECLARE(MX6DL_PAD_,name, pco, mc, mm, sio, si, pc),
+    MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
  #include "mx6dl_pins.h"
  };
  #elif defined(CONFIG_MX6Q)
@@ -30,7 +30,7 @@ enum {
  #elif defined(CONFIG_MX6DL) || defined(CONFIG_MX6S)
  enum {
  #define MX6_PAD_DECL(name, pco, mc, mm, sio, si, pc) \
Quoted 8 lines
arch/arm/include/asm/arch-mx6/mx6-pins.h enum { · line 35
Quoted 4 lines
arch/arm/include/asm/arch-mx6/mx6-pins.h +1 -1
-    MX6_PAD_DECLARE(MX6_PAD_,name, pco, mc, mm, sio, si, pc),
+    MX6_PAD_DECLARE(MX6DL_PAD_, name, pco, mc, mm, sio, si, pc),
  #include "mx6dl_pins.h"
  };
Can you better explain the problem you had ? The name is not decisive - the important thing is that the correct include file with the right layout is included, that means mx6dl_pins.h. And this was mainlined since a lot of time.
Maybe all the time nobody had to use I2C #4 on an i.mx6 duallite chip, doing so i encountered this problem.
The name is decisive for sure, have a closer look to the "MX6_PAD_DECLARE" macro,
In conjunction with the correct include file the prefix used to form the final register table declaration.
Next the iomux-v3.h is from interest,
the "#define IOMUX_PADS" has dependency on CONFIG_MX6nn, here the previous definition out from mx6-pins.h is used.

Quoted 8 lines

I will send some V2 to address this topic fully, ok?
Quoted 5 lines
We have several boards with 6DL into mainline, so I am missing which is your problem.
Best regards,
Stefano Babic
cheers,
Hannes

Just looked around a bit more about this.
Root cause for failing this patch is, that many boards do not use the 'IOMUX_PADS' macro, instead they just directly use the definition out of "mx6dl_pins.h" for example.
So we get in trouble there if we change the MX6_PAD_DECLARE macro for having MX6DL pads instead MX6 pads.

At one point of view it would make sense to me changing all boards to use the IOMUX_PADS macro for accessing pads register, because afterwards the real accessed register would be fully in dependence of CONFIG_MX6nn. On the other hand i cannot fully predict every case could happen if we simply change that with search/replace.

So it would be OK for me to drop this patch and i will use on my board:

MX6DL_PAD_ENET_TX_EN__I2C4_SCL
MX6DL_PAD_ENET_TXD1__I2C4_SDA

instead

IOMUX_PADS(PAD_ENET_TX_EN__I2C4_SCL)
IOMUX_PADS(MX6DL_PAD_ENET_TXD1__I2C4_SDA )

Whats your thinking about this?

cheers,
Hannes

Stefano Babic

2016-07-21 08:28 UTC #6 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

Hi Hannes,

Quoted 4 lines · Hannes Schmelzer

Just looked around a bit more about this.
Root cause for failing this patch is, that many boards do not use the 'IOMUX_PADS' macro, instead they just directly use the definition out of "mx6dl_pins.h" for example.

Both are allowed. IOMUX_PADS *must* be used in case the board supports multiple variant of the processor (DL, Quad,..). If the board has just one variant, the MX6 defines from the corresponding header can be used.

Quoted 2 lines

So we get in trouble there if we change the MX6_PAD_DECLARE macro for having MX6DL pads instead MX6 pads.

I am not getting where is the trouble, because there are already a lot of boards using it. Let's see....

Quoted 7 lines

At one point of view it would make sense to me changing all boards to use the IOMUX_PADS macro for accessing pads register, because afterwards the real accessed register would be fully in dependence of CONFIG_MX6nn. On the other hand i cannot fully predict every case could happen if we simply change that with search/replace.

Quoted 5 lines

So it would be OK for me to drop this patch and i will use on my board:
MX6DL_PAD_ENET_TX_EN__I2C4_SCL
MX6DL_PAD_ENET_TXD1__I2C4_SDA

Now I get the point - and yes, there is an exception for I2C in the pinmux. This was discussed at the beginning when IOMUX_PADS was introduced and how to support the different layout of the SOC variants.

We agreed to tread differently I2C. This means that a i2c_pads_info structure must be set for each variant of the SOC that board supports. With help of the is_cpu_type() macro (or one of this family), the correct structure is selected and the pinmux can be set.

The right way to do is:

static struct i2c_pads_info i2c_pad = {
        .scl = {
                .i2c_mode = MX6DL_PAD_ENET_TX_EN__I2C4_SCL | <pull up>,
                .gpio_mode = MX6DL_PAD_ENET_TX_EN__GPIO1_IO28 | <..>,
                .gp = IMX_GPIO_NR(1, 28)
        },
        .sda = {
                .i2c_mode = MX6DL_PAD_ENET_TXD1__I2C4_SDA | <pull >,
                .gpio_mode = MX6DL_PAD_ENET_TXD1__GPIO1_IO29 | <pull>,
                .gp = IMX_GPIO_NR(1, 29)
        }
};

and then you call setup_i2c() with the structure.

Best regards,
Stefano Babic

Signature · 5 lines

=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
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
=====================================================================

Hannes Schmelzer

2016-07-21 19:02 UTC #7 lore raw

PATCH 1/1 arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

Quoted 1 line · Stefano Babic

Hi Hannes,

Hi Stefano,

Quoted 53 lines · Hannes Schmelzer

Quoted 5 lines
Just looked around a bit more about this.
Root cause for failing this patch is, that many boards do not use the 'IOMUX_PADS' macro, instead they just directly use the definition out of "mx6dl_pins.h" for example.
Both are allowed. IOMUX_PADS *must* be used in case the board supports multiple variant of the processor (DL, Quad,..). If the board has just one variant, the MX6 defines from the corresponding header can be used.
Quoted 2 lines
So we get in trouble there if we change the MX6_PAD_DECLARE macro for having MX6DL pads instead MX6 pads.
I am not getting where is the trouble, because there are already a lot of boards using it. Let's see....
Quoted 11 lines
At one point of view it would make sense to me changing all boards to use the IOMUX_PADS macro for accessing pads register, because afterwards the real accessed register would be fully in dependence of CONFIG_MX6nn. On the other hand i cannot fully predict every case could happen if we simply change that with search/replace.
So it would be OK for me to drop this patch and i will use on my board:
MX6DL_PAD_ENET_TX_EN__I2C4_SCL
MX6DL_PAD_ENET_TXD1__I2C4_SDA
Now I get the point - and yes, there is an exception for I2C in the pinmux. This was discussed at the beginning when IOMUX_PADS was introduced and how to support the different layout of the SOC variants.
We agreed to tread differently I2C. This means that a i2c_pads_info structure must be set for each variant of the SOC that board supports. With help of the is_cpu_type() macro (or one of this family), the correct structure is selected and the pinmux can be set.
The right way to do is:
static struct i2c_pads_info i2c_pad = {
         .scl = {
                 .i2c_mode = MX6DL_PAD_ENET_TX_EN__I2C4_SCL | <pull up>,
                 .gpio_mode = MX6DL_PAD_ENET_TX_EN__GPIO1_IO28 | <..>,
                 .gp = IMX_GPIO_NR(1, 28)
         },
         .sda = {
                 .i2c_mode = MX6DL_PAD_ENET_TXD1__I2C4_SDA | <pull >,
                 .gpio_mode = MX6DL_PAD_ENET_TXD1__GPIO1_IO29 | <pull>,
                 .gp = IMX_GPIO_NR(1, 29)
         }
};

Quoted 1 line

and then you call setup_i2c() with the structure.

Yeah! Now i understand the thinkings behind/around that. Many thanks for this, i will implement this for my board.

Quoted 2 lines

Best regards,
Stefano Babic

cheers,
Hannes

Quoted 1 line