| Message ID | 1466590066-15218-1-git-send-email-oe5hpm@oevsv.at |
|---|---|
| State | Accepted |
| Commit | 225126da99dd9ba1478e32468e298085d1e3fb61 |
| Delegated to: | Stefano Babic |
| Headers | show |
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 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
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
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 >
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)
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(-)