[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[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 <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)
-- 
1.9.1

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Stefano Babic]

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

-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Otavio Salvador]

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.


-- 
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

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Stefano Babic]

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

-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Hannes Schmelzer]

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 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.
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

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Hannes Schmelzer]

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 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.
> 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

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Stefano Babic]

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

-- 
=====================================================================
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
=====================================================================

[U-Boot] [PATCH] arch-mx6: fix MX6_PAD_DECLARE macro to work with MX6 duallite

[Hannes Schmelzer]

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

>