[U-Boot] [PATCH] 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 oe5hpm@oevsv.at ---
arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/mx6-pins.h b/arch/arm/include/asm/arch-mx6/mx6-pins.h index 4b6bb18..3465205 100644 --- a/arch/arm/include/asm/arch-mx6/mx6-pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6-pins.h @@ -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)
On 22/06/2016 12:07, Hannes Schmelzer wrote:
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 oe5hpm@oevsv.at
arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/mx6-pins.h b/arch/arm/include/asm/arch-mx6/mx6-pins.h index 4b6bb18..3465205 100644 --- a/arch/arm/include/asm/arch-mx6/mx6-pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6-pins.h @@ -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)
Applied to u-boot-imx, -next branch, thanks !
Best regards, Stefano Babic
On Wed, Jun 22, 2016 at 7:07 AM, Hannes Schmelzer oe5hpm@oevsv.at wrote:
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 oe5hpm@oevsv.at
Please put this for the current release; this is a bugfix and a critical one.
Hi Hannes,
this patch breaks most i.MX6 boards (the not DL) and I revert it. Maybe I had to ask better before, anyway:
On 22/06/2016 12:07, Hannes Schmelzer wrote:
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 oe5hpm@oevsv.at
arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/mx6-pins.h b/arch/arm/include/asm/arch-mx6/mx6-pins.h index 4b6bb18..3465205 100644 --- a/arch/arm/include/asm/arch-mx6/mx6-pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6-pins.h @@ -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" };
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
On 07/20/2016 09:30 AM, Stefano Babic wrote:
Hi Hannes,
Hi Stefano,
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.
On 22/06/2016 12:07, Hannes Schmelzer wrote:
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 Schmelzeroe5hpm@oevsv.at
arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/mx6-pins.h b/arch/arm/include/asm/arch-mx6/mx6-pins.h index 4b6bb18..3465205 100644 --- a/arch/arm/include/asm/arch-mx6/mx6-pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6-pins.h @@ -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" };
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?
We have several boards with 6DL into mainline, so I am missing which is your problem.
Best regards, Stefano Babic
cheers, Hannes
On 07/20/2016 10:51 PM, Hannes Schmelzer wrote:
On 07/20/2016 09:30 AM, Stefano Babic wrote:
Hi Hannes,
Hi Stefano,
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.
On 22/06/2016 12:07, Hannes Schmelzer wrote:
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 Schmelzeroe5hpm@oevsv.at
arch/arm/include/asm/arch-mx6/mx6-pins.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/arch-mx6/mx6-pins.h b/arch/arm/include/asm/arch-mx6/mx6-pins.h index 4b6bb18..3465205 100644 --- a/arch/arm/include/asm/arch-mx6/mx6-pins.h +++ b/arch/arm/include/asm/arch-mx6/mx6-pins.h @@ -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" };
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?
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
Hi Hannes,
On 21/07/2016 08:10, Hannes Schmelzer wrote:
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.
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....
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) } };
and then you call setup_i2c() with the structure.
Best regards, Stefano Babic
On 07/21/2016 10:28 AM, Stefano Babic wrote:
Hi Hannes,
Hi Stefano,
On 21/07/2016 08:10, Hannes Schmelzer wrote:
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.
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....
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) } };
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.
Best regards, Stefano Babic
cheers, Hannes
participants (4)
-
Hannes Schmelzer -
Hannes Schmelzer -
Otavio Salvador -
Stefano Babic