[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Eric Nelson]

This series of patches is needed to allow main-line U-Boot to be used
with Freescale's Linux 2.6.38 non-DT kernel releases.

These releases currently require at least the machine type and
revision atag entries and are configured to load boot scripts from
the ext3 filesystem.

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Eric Nelson]

 Freescale 2.6.38 (Non-DT) kernels require the revision atag to
 enable the VPU.

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
 include/configs/mx6qsabrelite.h               |    1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index db1bea9..590030b 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
 }
 #endif
 
+#ifdef CONFIG_REVISION_TAG
+u32 get_board_rev(void)
+{
+	return 0x63000 ;
+}
+#endif
+
 #ifdef CONFIG_MXC_SPI
 iomux_v3_cfg_t ecspi1_pads[] = {
 	/* SS1 */
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 93000f0..85f6f7a 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -33,6 +33,7 @@
 #define CONFIG_CMDLINE_TAG
 #define CONFIG_SETUP_MEMORY_TAGS
 #define CONFIG_INITRD_TAG
+#define CONFIG_REVISION_TAG
 
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN          (CONFIG_ENV_SIZE + 2 * 1024 * 1024)
-- 
1.7.9

[U-Boot] [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE

[Eric Nelson]

Allow non-dt kernels to boot

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
---
 board/freescale/mx6qsabrelite/mx6qsabrelite.c |    6 ++++--
 include/configs/mx6qsabrelite.h               |    1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
index 590030b..2786482 100644
--- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
+++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
@@ -282,8 +282,10 @@ int board_early_init_f(void)
 
 int board_init(void)
 {
-       /* address of boot parameters */
-       gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
+	/* board id for linux */
+	gd->bd->bi_arch_number = MACH_TYPE_MX6Q_SABRELITE;
+	/* address of boot parameters */
+	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
 
        return 0;
 }
diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 85f6f7a..53869a9 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -27,6 +27,7 @@
 #define CONFIG_SYS_MX6_CLK32           32768
 #define CONFIG_DISPLAY_CPUINFO
 #define CONFIG_DISPLAY_BOARDINFO
+#define MACH_TYPE_MX6Q_SABRELITE       3769
 
 #include <asm/arch/imx-regs.h>
 
-- 
1.7.9

[U-Boot] [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support

[Eric Nelson]

Current Ubuntu releases from Freescale contain a boot script in ext3 filesystem.

Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
---
 include/configs/mx6qsabrelite.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/configs/mx6qsabrelite.h b/include/configs/mx6qsabrelite.h
index 53869a9..51ed791 100644
--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -66,6 +66,7 @@
 #define CONFIG_MMC
 #define CONFIG_CMD_MMC
 #define CONFIG_GENERIC_MMC
+#define CONFIG_CMD_EXT2
 #define CONFIG_CMD_FAT
 #define CONFIG_DOS_PARTITION
 
-- 
1.7.9

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Marek Vasut]

>  Freescale 2.6.38 (Non-DT) kernels require the revision atag to
>  enable the VPU.
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
>  board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
>  include/configs/mx6qsabrelite.h               |    1 +
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b
> 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
>  }
>  #endif
> 
> +#ifdef CONFIG_REVISION_TAG
> +u32 get_board_rev(void)
> +{
> +	return 0x63000 ;
> +}
> +#endif
> +
>  #ifdef CONFIG_MXC_SPI
>  iomux_v3_cfg_t ecspi1_pads[] = {
>  	/* SS1 */
> diff --git a/include/configs/mx6qsabrelite.h
> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -33,6 +33,7 @@
>  #define CONFIG_CMDLINE_TAG
>  #define CONFIG_SETUP_MEMORY_TAGS
>  #define CONFIG_INITRD_TAG
> +#define CONFIG_REVISION_TAG

I think you can avoid this define altogether, you're using it anyway.

M

[U-Boot] [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE

[Marek Vasut]

> Allow non-dt kernels to boot
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  board/freescale/mx6qsabrelite/mx6qsabrelite.c |    6 ++++--
>  include/configs/mx6qsabrelite.h               |    1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index 590030b..2786482
> 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -282,8 +282,10 @@ int board_early_init_f(void)
> 
>  int board_init(void)
>  {
> -       /* address of boot parameters */
> -       gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> +	/* board id for linux */
> +	gd->bd->bi_arch_number = MACH_TYPE_MX6Q_SABRELITE;
> +	/* address of boot parameters */
> +	gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> 
>         return 0;
>  }
> diff --git a/include/configs/mx6qsabrelite.h
> b/include/configs/mx6qsabrelite.h index 85f6f7a..53869a9 100644
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -27,6 +27,7 @@
>  #define CONFIG_SYS_MX6_CLK32           32768
>  #define CONFIG_DISPLAY_CPUINFO
>  #define CONFIG_DISPLAY_BOARDINFO
> +#define MACH_TYPE_MX6Q_SABRELITE       3769

Just use #define CONFIG_MACH_TYPE 3769

M

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Marek Vasut]

> This series of patches is needed to allow main-line U-Boot to be used
> with Freescale's Linux 2.6.38 non-DT kernel releases.
> 
> These releases currently require at least the machine type and
> revision atag entries and are configured to load boot scripts from
> the ext3 filesystem.
> 

Almost looks good, just minor tweaks needed. Thanks for your good work so far, 
keep it up !

M

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Wolfgang Denk]

Dear Eric Nelson,

In message <1330728909-12203-1-git-send-email-eric.nelson@boundarydevices.com> you wrote:
> This series of patches is needed to allow main-line U-Boot to be used
> with Freescale's Linux 2.6.38 non-DT kernel releases.
> 
> These releases currently require at least the machine type and
> revision atag entries and are configured to load boot scripts from
> the ext3 filesystem.

Is this really needed?  I feel we should rather focus on current
mainline kernel code and support that well instead of adding backward
compatibility complexity for old, obsolete code.

People who want to run FSL kernels should be able run FSL's version
of U-Boot.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No journaling file system can recover your data if the disk dies.
                                 - Steve Rago in <D4Cw1p.L9E@plc.com>

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Eric Nelson]

On 03/02/2012 04:25 PM, Wolfgang Denk wrote:
> Dear Eric Nelson,
>
> In message<1330728909-12203-1-git-send-email-eric.nelson@boundarydevices.com>  you wrote:
>> This series of patches is needed to allow main-line U-Boot to be used
>> with Freescale's Linux 2.6.38 non-DT kernel releases.
>>
>> These releases currently require at least the machine type and
>> revision atag entries and are configured to load boot scripts from
>> the ext3 filesystem.
>
> Is this really needed?  I feel we should rather focus on current
> mainline kernel code and support that well instead of adding backward
> compatibility complexity for old, obsolete code.
>

I think new code is still being written for 2.6.38, so it may be obsolete,
but not necessarily old.

> People who want to run FSL kernels should be able run FSL's version
> of U-Boot.
>

They certainly can.

These small patches will be a big help to those of us who need to
support both though, and it's not yet clear how long the kernel
transition will take.

> Best regards,
>
> Wolfgang Denk
>
Regards,


Eric

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Fabio Estevam]

On Fri, Mar 2, 2012 at 8:46 PM, Eric Nelson
<eric.nelson@boundarydevices.com> wrote:

> These small patches will be a big help to those of us who need to
> support both though, and it's not yet clear how long the kernel
> transition will take.

Agree. It would be extremely helpful if we could run FSL kernel with
mainline U-boot.

Thanks,

Fabio Estevam

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Dirk Behme]

Dear Wolfgang,

On 03.03.2012 00:25, Wolfgang Denk wrote:
> Dear Eric Nelson,
>
> In message<1330728909-12203-1-git-send-email-eric.nelson@boundarydevices.com>  you wrote:
>> This series of patches is needed to allow main-line U-Boot to be used
>> with Freescale's Linux 2.6.38 non-DT kernel releases.
>>
>> These releases currently require at least the machine type and
>> revision atag entries and are configured to load boot scripts from
>> the ext3 filesystem.
>
> Is this really needed?  I feel we should rather focus on current
> mainline kernel code and support that well instead of adding backward
> compatibility complexity for old, obsolete code.
>
> People who want to run FSL kernels should be able run FSL's version
> of U-Boot.

Hmm, yes, some days ago, when this discussion started, I was under the 
same impression. But:

After thinking some days about it, I wonder if it would help to make 
the transition path to mainline easier. So maybe it's not mainly a 
technical issue, but more a political one ;)

Having Freescale working on these quite old and unclean U-Boot and 
Kernel versions is a pain. Kernel is an other topic, but with U-Boot, 
thanks to the very good work of everybody, we are in a good position 
to get rid of the old Freescale U-Boot, now. And get everybody to work 
with mainline and create patches against it.

So if it helps to apply some backward compatibility to make it easier 
for everybody, esp. Freescale, to switch to mainline U-Boot, I think 
we should try it.

Let's try to make the first step with U-Boot. Then try to make the 
next, harder, step with the kernel.

Best regards

Dirk

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Wolfgang Denk]

Dear Eric Nelson,

In message <4F515BF1.9030101@boundarydevices.com> you wrote:
>
> I think new code is still being written for 2.6.38, so it may be obsolete,
> but not necessarily old.

It definitely is old - Linux mainline has seen 50240 commits
since v2.6.38, with 3243564 lines added and 2329522 removed.

You probably do not want to consider it obsolete yet...  I do. YMMV.

> These small patches will be a big help to those of us who need to
> support both though, and it's not yet clear how long the kernel
> transition will take.

The problem is that these changes will cause additional maintenance
efforts in the future.  I think we have reached a point in time where
new boards that get added should only add support for DT baed
configurations, and get rid of this MACH_ID crap.  Just look at the
number of recent board breakages and need of fixes just because IDs
get dropped from the mach_id database.

I feel efforts would be better invested if they were directed in
improving current mainline kernel code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"When the only  tool  you  have  is  a  hammer,  you  tend  to  treat
everything as if it were a nail."                    - Abraham Maslow

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Wolfgang Denk]

Dear Dirk,

In message <4F51BBA9.4090608@googlemail.com> you wrote:
> 
> Having Freescale working on these quite old and unclean U-Boot and 
> Kernel versions is a pain. Kernel is an other topic, but with U-Boot, 
> thanks to the very good work of everybody, we are in a good position 
> to get rid of the old Freescale U-Boot, now. And get everybody to work 
> with mainline and create patches against it.

ACK.

> So if it helps to apply some backward compatibility to make it easier 
> for everybody, esp. Freescale, to switch to mainline U-Boot, I think 
> we should try it.

Agreed.  If these patches were only for backward compatibility I would
not complain much.  But they are known to introduce forward incompati-
bilities with all this MACH_ID stuff, and this is what I would like to
avoid.

I think we should make a clear statement that new boards that get
added should only support DT based configurations.  If really needed,
legacy MACH_ID support may be kept out of tree.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Overflow on /dev/null, please empty the bit bucket.

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Stefano Babic]

On 02/03/2012 23:55, Eric Nelson wrote:
>  Freescale 2.6.38 (Non-DT) kernels require the revision atag to
>  enable the VPU.
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---
>  board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
>  include/configs/mx6qsabrelite.h               |    1 +
>  2 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index db1bea9..590030b 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
>  }
>  #endif
>  
> +#ifdef CONFIG_REVISION_TAG

You add CONFIG_REVISION_TAG, then you can get rid of unneeded #ifdef

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE

[Stefano Babic]

On 02/03/2012 23:55, Eric Nelson wrote:
> Allow non-dt kernels to boot
> 
> Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>
> ---
>  board/freescale/mx6qsabrelite/mx6qsabrelite.c |    6 ++++--
>  include/configs/mx6qsabrelite.h               |    1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> index 590030b..2786482 100644
> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> @@ -282,8 +282,10 @@ int board_early_init_f(void)
>  
>  int board_init(void)
>  {
> -       /* address of boot parameters */
> -       gd->bd->bi_boot_params = PHYS_SDRAM + 0x100;
> +	/* board id for linux */
> +	gd->bd->bi_arch_number = MACH_TYPE_MX6Q_SABRELITE;

You do not needed. This is done in common code. Add only
CONFIG_MACH_TYPE in mx6qsabrelite.h

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Stefano Babic]

On 03/03/2012 00:25, Wolfgang Denk wrote:
> Dear Eric Nelson,
> 
> In message <1330728909-12203-1-git-send-email-eric.nelson@boundarydevices.com> you wrote:
>> This series of patches is needed to allow main-line U-Boot to be used
>> with Freescale's Linux 2.6.38 non-DT kernel releases.
>>
>> These releases currently require at least the machine type and
>> revision atag entries and are configured to load boot scripts from
>> the ext3 filesystem.
> 
> Is this really needed?  I feel we should rather focus on current
> mainline kernel code and support that well instead of adding backward
> compatibility complexity for old, obsolete code.
> 
> People who want to run FSL kernels should be able run FSL's version
> of U-Boot.

Generally I agree with you - and it cannot be that U-boot mainline
supports old kernels and not mainline kernel.

However, these are slightly changes and at the end conform with most of
supported ARM boards, if not all. And it can better convince people to
not continue to add features to a really obsolete and dicontinued U-Boot
version as 2009.08 is, instead of working with the community on the
current U-Boot development.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[mailander]

On 03/03/2012 10:38, Wolfgang Denk wrote:
> Dear Dirk,
> 
> In message <4F51BBA9.4090608@googlemail.com> you wrote:
>>
>> Having Freescale working on these quite old and unclean U-Boot and 
>> Kernel versions is a pain. Kernel is an other topic, but with U-Boot, 
>> thanks to the very good work of everybody, we are in a good position 
>> to get rid of the old Freescale U-Boot, now. And get everybody to work 
>> with mainline and create patches against it.
> 
> ACK.
> 
>> So if it helps to apply some backward compatibility to make it easier 
>> for everybody, esp. Freescale, to switch to mainline U-Boot, I think 
>> we should try it.
> 
> Agreed.  If these patches were only for backward compatibility I would
> not complain much.  But they are known to introduce forward incompati-
> bilities with all this MACH_ID stuff, and this is what I would like to
> avoid.
> 
> I think we should make a clear statement that new boards that get
> added should only support DT based configurations.  If really needed,
> legacy MACH_ID support may be kept out of tree.

I think a point here is that boards must not use any macro defined in
mach-types.h, such as machine_is_*.  U-Boot does not need it, and their
usage makes the code strictly dependent from the kernel legacy mach-ids.

If a board defines only CONFIG_MACH_TYPE in its own configuration file,
it should not break if we decide in future to drop it completely.

Best regards,
Stefano Babic

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Dirk Behme]

Dear Wolfgang,

On 03.03.2012 10:38, Wolfgang Denk wrote:
> Dear Dirk,
>
> In message<4F51BBA9.4090608@googlemail.com>  you wrote:
>>
>> Having Freescale working on these quite old and unclean U-Boot and
>> Kernel versions is a pain. Kernel is an other topic, but with U-Boot,
>> thanks to the very good work of everybody, we are in a good position
>> to get rid of the old Freescale U-Boot, now. And get everybody to work
>> with mainline and create patches against it.
>
> ACK.
>
>> So if it helps to apply some backward compatibility to make it easier
>> for everybody, esp. Freescale, to switch to mainline U-Boot, I think
>> we should try it.
>
> Agreed.  If these patches were only for backward compatibility I would
> not complain much.  But they are known to introduce forward incompati-
> bilities with all this MACH_ID stuff, and this is what I would like to
> avoid.

Now I'm just trying to learn something regarding [1]:

Which changes would you accept in the category 'backward compatibility'?

And which changes 'introduce forward incompatibilities', and what are 
these incompatibilities?

Many thanks and best regards

Dirk

[1]

http://lists.denx.de/pipermail/u-boot/2012-March/119207.html

http://lists.denx.de/pipermail/u-boot/2012-March/119208.html

(assuming this will be changed to

--- a/include/configs/mx6qsabrelite.h
+++ b/include/configs/mx6qsabrelite.h
@@ -27,6 +27,7 @@
  #define CONFIG_SYS_MX6_CLK32           32768
  #define CONFIG_DISPLAY_CPUINFO
  #define CONFIG_DISPLAY_BOARDINFO
+#define CONFIG_MACH_TYPE       3769

  #include <asm/arch/imx-regs.h>

)

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Wolfgang Denk]

Dear Dirk Behme,

In message <4F52015A.2080003@googlemail.com> you wrote:
> 
> > Agreed.  If these patches were only for backward compatibility I would
> > not complain much.  But they are known to introduce forward incompati-
> > bilities with all this MACH_ID stuff, and this is what I would like to
> > avoid.
> 
> Now I'm just trying to learn something regarding [1]:
> 
> Which changes would you accept in the category 'backward compatibility'?

There are 3 commits in this series:

[PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG
[PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE
[PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support

I dislike #1 because it uses the completely undocumented
CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments.

The problems I mentioned are with # 2, which now would depend on
MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.

Also, I think we should not need this any more at all, as we now have
DT support in Linux on ARM, too.

I see no issues with # 3.

> And which changes 'introduce forward incompatibilities', and what are 
> these incompatibilities?

See the recent problems that occurred when RMK decided to "clean up"
the machids file.

> (assuming this will be changed to
> 
> --- a/include/configs/mx6qsabrelite.h
> +++ b/include/configs/mx6qsabrelite.h
> @@ -27,6 +27,7 @@
>   #define CONFIG_SYS_MX6_CLK32           32768
>   #define CONFIG_DISPLAY_CPUINFO
>   #define CONFIG_DISPLAY_BOARDINFO
> +#define CONFIG_MACH_TYPE       3769

Such a change would avoid breakages as the ones mentioned above, but
is this nice?  Either we have a mach-types.h that can be used, or we
don't, in which case we should stop using any definitions from it,
especially for new boards where it's not needed due to DT support in
the kernel.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is better to marry than to burn.
                                - Bible ``I Corinthians'' ch. 7, v. 9

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Troy Kisky]

On 3/3/2012 6:30 AM, Wolfgang Denk wrote:
> Dear Dirk Behme,
>
> In message<4F52015A.2080003@googlemail.com>  you wrote:
>>> Agreed.  If these patches were only for backward compatibility I would
>>> not complain much.  But they are known to introduce forward incompati-
>>> bilities with all this MACH_ID stuff, and this is what I would like to
>>> avoid.
>> Now I'm just trying to learn something regarding [1]:
>>
>> Which changes would you accept in the category 'backward compatibility'?
> There are 3 commits in this series:
>
> [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG
> [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE
> [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support
>
> I dislike #1 because it uses the completely undocumented
> CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments.
>
> The problems I mentioned are with # 2, which now would depend on
> MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.
>
> Also, I think we should not need this any more at all, as we now have
> DT support in Linux on ARM, too.
>
> I see no issues with # 3.
>
>> And which changes 'introduce forward incompatibilities', and what are
>> these incompatibilities?
> See the recent problems that occurred when RMK decided to "clean up"
> the machids file.
>
>
Would you rather that I take RMK's cleaned up file, and undelete the 
machines that u-boot
uses?  That would be more simple than adding to the board's config file.
I can delete all of the mach_is_xxx macros in mach-types while I'm at it.


Thanks
Troy

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Wolfgang Denk]

Dear Troy Kisky,

In message <4F52C327.3080309@boundarydevices.com> you wrote:
>
> >> And which changes 'introduce forward incompatibilities', and what are
> >> these incompatibilities?
> > See the recent problems that occurred when RMK decided to "clean up"
> > the machids file.
> >
> Would you rather that I take RMK's cleaned up file, and undelete the 
> machines that u-boot
> uses?  That would be more simple than adding to the board's config file.
> I can delete all of the mach_is_xxx macros in mach-types while I'm at it.

I think we had this discussion before (when RMK's changes hit us), and
it was decided not to do this.  IIRC we decided not to do this.

I have never understood why this mach_types thingy was needed (other
rchitectures worked fine without it, or better), and now we are on the
edge of obsoleting it. So all efforts trying to maintain this file are
futile, and we would have to redo these for any updates of the file.

I feel this is not a good investment of our time.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
:       I've tried (in vi) "g/[a-z]\n[a-z]/s//_/"...but that doesn't
: cut it.  Any ideas?  (I take it that it may be a two-pass sort of solution).
In the first pass, install perl. :-) Larry Wall <6849@jpl-devvax.JPL.NASA.GOV>

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Stefano Babic]

On 03/03/2012 14:30, Wolfgang Denk wrote:
> Dear Dirk Behme,
> 

Hi,

> In message <4F52015A.2080003@googlemail.com> you wrote:
>>
>>> Agreed.  If these patches were only for backward compatibility I would
>>> not complain much.  But they are known to introduce forward incompati-
>>> bilities with all this MACH_ID stuff, and this is what I would like to
>>> avoid.
>>
>> Now I'm just trying to learn something regarding [1]:
>>
>> Which changes would you accept in the category 'backward compatibility'?
> 
> There are 3 commits in this series:
> 
> [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG
> [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE
> [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support
> 
> I dislike #1 because it uses the completely undocumented
> CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments.

A lot of boards are currently set CONFIG_REVISION_TAG. Sure, it would be
nice to document it. To be consistent we should drop this CONFIG_ from
all boards, or add documentation for it.

However, I am asking if this TAG is really needed. I have searched in
2.6.38 Kernel provided by Freescale if the TAG is really evaluated to
set different revisions of the boards, but I have not found anything. Is
it  really needed ? If not, we should drop it.

> 
> The problems I mentioned are with # 2, which now would depend on
> MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.

Right, I agree. And we do not know (maybe it is not this case) if
MACH_TYPE_MX6Q_SABRELITE will be dropped in the future. IMHO we should
not use mach-types at all..

> 
> Also, I think we should not need this any more at all, as we now have
> DT support in Linux on ARM, too.
> 
> I see no issues with # 3.

I will merge #3 into u-boot-imx

> 
>> And which changes 'introduce forward incompatibilities', and what are 
>> these incompatibilities?
> 
> See the recent problems that occurred when RMK decided to "clean up"
> the machids file.
> 
>> (assuming this will be changed to
>>
>> --- a/include/configs/mx6qsabrelite.h
>> +++ b/include/configs/mx6qsabrelite.h
>> @@ -27,6 +27,7 @@
>>   #define CONFIG_SYS_MX6_CLK32           32768
>>   #define CONFIG_DISPLAY_CPUINFO
>>   #define CONFIG_DISPLAY_BOARDINFO
>> +#define CONFIG_MACH_TYPE       3769
> 
> Such a change would avoid breakages as the ones mentioned above, but
> is this nice?  Either we have a mach-types.h that can be used, or we
> don't,

Personally I vote for the second choice. U-boot does not use mach-types,
and it is at the end only a parameter for the kernel.

I think the solution in the patch is better as to try to maintain the
mach-types file: not very nice, but setting its value is not different
as several other setups inside the configuration file that are very
board specifiv. And CONFIG_MACH_TYPE is well documented.

> in which case we should stop using any definitions from it,
> especially for new boards where it's not needed due to DT support in
> the kernel.

Agree completely - mach-types introduces a strong dependency with the
kernel, and we do not need it.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Igor Grinberg]

Hi,

On 03/04/12 13:09, Stefano Babic wrote:
> On 03/03/2012 14:30, Wolfgang Denk wrote:
>> Dear Dirk Behme,
>>
> 
> Hi,
> 
>> In message <4F52015A.2080003@googlemail.com> you wrote:
>>>
>>>> Agreed.  If these patches were only for backward compatibility I would
>>>> not complain much.  But they are known to introduce forward incompati-
>>>> bilities with all this MACH_ID stuff, and this is what I would like to
>>>> avoid.
>>>
>>> Now I'm just trying to learn something regarding [1]:
>>>
>>> Which changes would you accept in the category 'backward compatibility'?
>>
>> There are 3 commits in this series:
>>
>> [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG
>> [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE
>> [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support
>>
>> I dislike #1 because it uses the completely undocumented
>> CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments.
> 
> A lot of boards are currently set CONFIG_REVISION_TAG. Sure, it would be
> nice to document it. To be consistent we should drop this CONFIG_ from
> all boards, or add documentation for it.
> 
> However, I am asking if this TAG is really needed. I have searched in
> 2.6.38 Kernel provided by Freescale if the TAG is really evaluated to
> set different revisions of the boards, but I have not found anything. Is
> it  really needed ? If not, we should drop it.

Linux's system_rev global variable is set to that value.
You can check for it by "cat /proc/cpuinfo | grep Revision".

> 
>>
>> The problems I mentioned are with # 2, which now would depend on
>> MACH_TYPE_MX6Q_SABRELITE, which may or may not exist.
> 
> Right, I agree. And we do not know (maybe it is not this case) if
> MACH_TYPE_MX6Q_SABRELITE will be dropped in the future. IMHO we should
> not use mach-types at all..

+1
making the mach-type local to a board moves the maintenance burden
to a board maintainer which is good because it can be decided on per
board basis.
Also, we should always raise the question if it is still needed
for a particular board and don't let it in, if it is not...

> 
>>
>> Also, I think we should not need this any more at all, as we now have
>> DT support in Linux on ARM, too.
>>
>> I see no issues with # 3.
> 
> I will merge #3 into u-boot-imx
> 
>>
>>> And which changes 'introduce forward incompatibilities', and what are 
>>> these incompatibilities?
>>
>> See the recent problems that occurred when RMK decided to "clean up"
>> the machids file.
>>
>>> (assuming this will be changed to
>>>
>>> --- a/include/configs/mx6qsabrelite.h
>>> +++ b/include/configs/mx6qsabrelite.h
>>> @@ -27,6 +27,7 @@
>>>   #define CONFIG_SYS_MX6_CLK32           32768
>>>   #define CONFIG_DISPLAY_CPUINFO
>>>   #define CONFIG_DISPLAY_BOARDINFO
>>> +#define CONFIG_MACH_TYPE       3769
>>
>> Such a change would avoid breakages as the ones mentioned above, but
>> is this nice?  Either we have a mach-types.h that can be used, or we
>> don't,
> 
> Personally I vote for the second choice. U-boot does not use mach-types,
> and it is at the end only a parameter for the kernel.

+1

> 
> I think the solution in the patch is better as to try to maintain the
> mach-types file: not very nice, but setting its value is not different
> as several other setups inside the configuration file that are very
> board specifiv.

+1

> And CONFIG_MACH_TYPE is well documented.

10x ;-)

> 
>> in which case we should stop using any definitions from it,
>> especially for new boards where it's not needed due to DT support in
>> the kernel.
> 
> Agree completely - mach-types introduces a strong dependency with the
> kernel, and we do not need it.

+1

I think, some more time is needed for the transition to DT.
We should let the boards use non-DT configurations, but move the
maintenance to the board maintainer (i.e. set the CONFIG_MACH_TYPE
in the board config file).
Also, soon there will be no non-DT boards accepted in the mainline kernel
(probably, also, there will be no new machine types accepted in the machine
types registry), so the need for the mach-type will just go away, no?

-- 
Regards,
Igor.

[U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels)

[Eric Nelson]

On 03/04/2012 04:09 AM, Stefano Babic wrote:
> On 03/03/2012 14:30, Wolfgang Denk wrote:
>> Dear Dirk Behme,
>>
>> <snip>
>>
>> There are 3 commits in this series:
>>
>> [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG
>> [PATCH 2/3] i.MX6: mx6qsabrelite: add MACH_TYPE_MX6Q_SABRELITE
>> [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support
>>
>> I dislike #1 because it uses the completely undocumented
>> CONFIG_REVISION_TAG, and I agree with Marek's and Stefano's comments.
>
> A lot of boards are currently set CONFIG_REVISION_TAG. Sure, it would be
> nice to document it. To be consistent we should drop this CONFIG_ from
> all boards, or add documentation for it.
>
> However, I am asking if this TAG is really needed. I have searched in
> 2.6.38 Kernel provided by Freescale if the TAG is really evaluated to
> set different revisions of the boards, but I have not found anything. Is
> it  really needed ? If not, we should drop it.
>

The linkage is really indirect. The ATAG item is still supported in the
main-line kernel for ARM:
	http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704

The breakage I noticed was in the VPU driver, which refused to load
with a zero-value in system_rev. The net result was no video playback
in the Freescale Android ICS release.

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Eric Nelson]

On 03/02/2012 03:59 PM, Marek Vasut wrote:
>>   Freescale 2.6.38 (Non-DT) kernels require the revision atag to
>>   enable the VPU.
>>
>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>> ---
>>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
>>   include/configs/mx6qsabrelite.h               |    1 +
>>   2 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b
>> 100644
>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
>>   }
>>   #endif
>>
>> +#ifdef CONFIG_REVISION_TAG
>> +u32 get_board_rev(void)
>> +{
>> +	return 0x63000 ;
>> +}
>> +#endif
>> +
>>   #ifdef CONFIG_MXC_SPI
>>   iomux_v3_cfg_t ecspi1_pads[] = {
>>   	/* SS1 */
>> diff --git a/include/configs/mx6qsabrelite.h
>> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644
>> --- a/include/configs/mx6qsabrelite.h
>> +++ b/include/configs/mx6qsabrelite.h
>> @@ -33,6 +33,7 @@
>>   #define CONFIG_CMDLINE_TAG
>>   #define CONFIG_SETUP_MEMORY_TAGS
>>   #define CONFIG_INITRD_TAG
>> +#define CONFIG_REVISION_TAG
>
> I think you can avoid this define altogether, you're using it anyway.
>
Hi Marek,

I'm not sure I understand.

I need to define CONFIG_REVISION_TAG in order to get bootm to add
the tag:
	http://git.denx.de/?p=u-boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df7620606ee12b4dc1dd2ee37adee347c;hb=master#l137

I made get_board_rev() conditional so we can get rid of it if (once)
we're only using DT kernels.

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Marek Vasut]

> On 03/02/2012 03:59 PM, Marek Vasut wrote:
> >>   Freescale 2.6.38 (Non-DT) kernels require the revision atag to
> >>   enable the VPU.
> >> 
> >> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >> ---
> >> 
> >>   board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
> >>   include/configs/mx6qsabrelite.h               |    1 +
> >>   2 files changed, 8 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b
> >> 100644
> >> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
> >> 
> >>   }
> >>   #endif
> >> 
> >> +#ifdef CONFIG_REVISION_TAG
> >> +u32 get_board_rev(void)
> >> +{
> >> +	return 0x63000 ;
> >> +}
> >> +#endif
> >> +
> >> 
> >>   #ifdef CONFIG_MXC_SPI
> >>   iomux_v3_cfg_t ecspi1_pads[] = {
> >>   
> >>   	/* SS1 */
> >> 
> >> diff --git a/include/configs/mx6qsabrelite.h
> >> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644
> >> --- a/include/configs/mx6qsabrelite.h
> >> +++ b/include/configs/mx6qsabrelite.h
> >> @@ -33,6 +33,7 @@
> >> 
> >>   #define CONFIG_CMDLINE_TAG
> >>   #define CONFIG_SETUP_MEMORY_TAGS
> >>   #define CONFIG_INITRD_TAG
> >> 
> >> +#define CONFIG_REVISION_TAG
> > 
> > I think you can avoid this define altogether, you're using it anyway.
> 
> Hi Marek,
> 
> I'm not sure I understand.
> 
> I need to define CONFIG_REVISION_TAG in order to get bootm to add
> the tag:
> 	http://git.denx.de/?p=u-
boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df
> 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137
> 
> I made get_board_rev() conditional so we can get rid of it if (once)
> we're only using DT kernels.

You have CONFIG_REVISION_TAG set unconditionally, so you don't need the macro 
around that get_revision() function. I might be wrong.

M

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Eric Nelson]

On 03/04/2012 01:59 PM, Marek Vasut wrote:
>> On 03/02/2012 03:59 PM, Marek Vasut wrote:
>>>>    Freescale 2.6.38 (Non-DT) kernels require the revision atag to
>>>>    enable the VPU.
>>>>
>>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
>>>> ---
>>>>
>>>>    board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
>>>>    include/configs/mx6qsabrelite.h               |    1 +
>>>>    2 files changed, 8 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b
>>>> 100644
>>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
>>>> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
>>>>
>>>>    }
>>>>    #endif
>>>>
>>>> +#ifdef CONFIG_REVISION_TAG
>>>> +u32 get_board_rev(void)
>>>> +{
>>>> +	return 0x63000 ;
>>>> +}
>>>> +#endif
>>>> +
>>>>
>>>>    #ifdef CONFIG_MXC_SPI
>>>>    iomux_v3_cfg_t ecspi1_pads[] = {
>>>>
>>>>    	/* SS1 */
>>>>
>>>> diff --git a/include/configs/mx6qsabrelite.h
>>>> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644
>>>> --- a/include/configs/mx6qsabrelite.h
>>>> +++ b/include/configs/mx6qsabrelite.h
>>>> @@ -33,6 +33,7 @@
>>>>
>>>>    #define CONFIG_CMDLINE_TAG
>>>>    #define CONFIG_SETUP_MEMORY_TAGS
>>>>    #define CONFIG_INITRD_TAG
>>>>
>>>> +#define CONFIG_REVISION_TAG
>>>
>>> I think you can avoid this define altogether, you're using it anyway.
>>
>> Hi Marek,
>>
>> I'm not sure I understand.
>>
>> I need to define CONFIG_REVISION_TAG in order to get bootm to add
>> the tag:
>> 	http://git.denx.de/?p=u-
> boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df
>> 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137
>>
>> I made get_board_rev() conditional so we can get rid of it if (once)
>> we're only using DT kernels.
>
> You have CONFIG_REVISION_TAG set unconditionally, so you don't need the macro
> around that get_revision() function. I might be wrong.
>

You're right. Its just that I did that deliberately so you can save the
(admittedly) small amount of code by editing mx6qsabrelite.h.

Since U-Boot doesn't really have a configuration step, I find that
folks tend to tweak the board configuration file quite a bit based
on their needs.

Though it's a bit of a hack, we even formalized it on our i.MX5x products
to allow customers to keep their git repositories clean:
	http://boundarydevices.com/blogs/customizing-u-boot-on-i-mx

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Marek Vasut]

> On 03/04/2012 01:59 PM, Marek Vasut wrote:
> >> On 03/02/2012 03:59 PM, Marek Vasut wrote:
> >>>>    Freescale 2.6.38 (Non-DT) kernels require the revision atag to
> >>>>    enable the VPU.
> >>>> 
> >>>> Signed-off-by: Eric Nelson<eric.nelson@boundarydevices.com>
> >>>> ---
> >>>> 
> >>>>    board/freescale/mx6qsabrelite/mx6qsabrelite.c |    7 +++++++
> >>>>    include/configs/mx6qsabrelite.h               |    1 +
> >>>>    2 files changed, 8 insertions(+), 0 deletions(-)
> >>>> 
> >>>> diff --git a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >>>> b/board/freescale/mx6qsabrelite/mx6qsabrelite.c index db1bea9..590030b
> >>>> 100644
> >>>> --- a/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >>>> +++ b/board/freescale/mx6qsabrelite/mx6qsabrelite.c
> >>>> @@ -215,6 +215,13 @@ int board_mmc_init(bd_t *bis)
> >>>> 
> >>>>    }
> >>>>    #endif
> >>>> 
> >>>> +#ifdef CONFIG_REVISION_TAG
> >>>> +u32 get_board_rev(void)
> >>>> +{
> >>>> +	return 0x63000 ;
> >>>> +}
> >>>> +#endif
> >>>> +
> >>>> 
> >>>>    #ifdef CONFIG_MXC_SPI
> >>>>    iomux_v3_cfg_t ecspi1_pads[] = {
> >>>>    
> >>>>    	/* SS1 */
> >>>> 
> >>>> diff --git a/include/configs/mx6qsabrelite.h
> >>>> b/include/configs/mx6qsabrelite.h index 93000f0..85f6f7a 100644
> >>>> --- a/include/configs/mx6qsabrelite.h
> >>>> +++ b/include/configs/mx6qsabrelite.h
> >>>> @@ -33,6 +33,7 @@
> >>>> 
> >>>>    #define CONFIG_CMDLINE_TAG
> >>>>    #define CONFIG_SETUP_MEMORY_TAGS
> >>>>    #define CONFIG_INITRD_TAG
> >>>> 
> >>>> +#define CONFIG_REVISION_TAG
> >>> 
> >>> I think you can avoid this define altogether, you're using it anyway.
> >> 
> >> Hi Marek,
> >> 
> >> I'm not sure I understand.
> >> 
> >> I need to define CONFIG_REVISION_TAG in order to get bootm to add
> >> 
> >> the tag:
> >> 	http://git.denx.de/?p=u-
> > 
> > boot.git;a=blob;f=arch/arm/lib/bootm.c;h=afa0093df
> > 
> >> 7620606ee12b4dc1dd2ee37adee347c;hb=master#l137
> >> 
> >> I made get_board_rev() conditional so we can get rid of it if (once)
> >> we're only using DT kernels.
> > 
> > You have CONFIG_REVISION_TAG set unconditionally, so you don't need the
> > macro around that get_revision() function. I might be wrong.
> 
> You're right. Its just that I did that deliberately so you can save the
> (admittedly) small amount of code by editing mx6qsabrelite.h.

Shouldn't that be removed by the compiler anyway?
> 
> Since U-Boot doesn't really have a configuration step, I find that
> folks tend to tweak the board configuration file quite a bit based
> on their needs.

This needs to be fixed, we need kbuild. Who's up to doing it ? ;-D
> 
> Though it's a bit of a hack, we even formalized it on our i.MX5x products
> to allow customers to keep their git repositories clean:
> 	http://boundarydevices.com/blogs/customizing-u-boot-on-i-mx

That's good, thank you :)

Cheers!
M

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Wolfgang Denk]

Dear Eric Nelson,

In message <4F53D8E6.4000408@boundarydevices.com> you wrote:
>
> Since U-Boot doesn't really have a configuration step, I find that
> folks tend to tweak the board configuration file quite a bit based
> on their needs.

Yes, peaople do that a lot, nd it has always been a very good idea to
at least detect such modifications of the code.  The worst thing to
happen is when you receive a bug report for version FOO, and you have
no chance of detecting that somebody actually messed with the
configuration and/or the code.

> Though it's a bit of a hack, we even formalized it on our i.MX5x products
> to allow customers to keep their git repositories clean:
> 	....
[Link deleted to not further distribute this horrible stuff.]

Frankly, this "keep their git repositories clean" is a really, really
bad idea.  We pay special care to add the git commit ID to the U-Boot
version string so you can easily detect if somebody messed with the
code, and you come up and invent methods of circumventing that.

This is totally counterproductive.

If you use this, that's your business.  But please do me the favour and
do not recommend this to others.


Thanks.


Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Alliance: In international politics, the union  of  two  thieves  who
have  their hands so deeply inserted in each other's pocket that they
cannot separately plunder a third.                   - Ambrose Bierce

[U-Boot] [PATCH 1/3] i.MX6: mx6qsabrelite: add CONFIG_REVISION_TAG

[Eric Nelson]

On 03/04/2012 03:06 PM, Wolfgang Denk wrote:
> Dear Eric Nelson,
>
> In message<4F53D8E6.4000408@boundarydevices.com>  you wrote:
>>
>> Since U-Boot doesn't really have a configuration step, I find that
>> folks tend to tweak the board configuration file quite a bit based
>> on their needs.
>
> Yes, peaople do that a lot, nd it has always been a very good idea to
> at least detect such modifications of the code.  The worst thing to
> happen is when you receive a bug report for version FOO, and you have
> no chance of detecting that somebody actually messed with the
> configuration and/or the code.
>
>> Though it's a bit of a hack, we even formalized it on our i.MX5x products
>> to allow customers to keep their git repositories clean:
>> 	....
> [Link deleted to not further distribute this horrible stuff.]
>
> Frankly, this "keep their git repositories clean" is a really, really
> bad idea.  We pay special care to add the git commit ID to the U-Boot
> version string so you can easily detect if somebody messed with the
> code, and you come up and invent methods of circumventing that.
>
> This is totally counterproductive.
>
> If you use this, that's your business.  But please do me the favour and
> do not recommend this to others.
>

We don't. Sorry if I implied otherwise.

We recommend that customers keep their own repositories with changes
from our "standard" releases, but we have a number of customers whose
complete set of differences amounts to default environment variables
and/or choices of serial consoles for which we don't want to carry and
publish an entire configuration.

This hack was borne out of frustration when getting a 'revision-dirty'
reference back from a customer because they've changed something
simple (usually environment settings).

[U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels)

[Stefano Babic]

On 04/03/2012 20:45, Eric Nelson wrote:

> The linkage is really indirect. The ATAG item is still supported in the
> main-line kernel for ARM:
>     http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704
> 
> 
> The breakage I noticed was in the VPU driver, which refused to load
> with a zero-value in system_rev. The net result was no video playback
> in the Freescale Android ICS release.

However, I have not found a statement in FSL's kernel where system_rev
in VPU driver is checked, directly or indirectly - neither in VPU driver
nor in the board initialization code, nor in another MXC driver.
It seems to me a side-effect in FSL's kernel, and for some not yet known
reasons the problem disappears with this tag - or can reappear later,
because we do not know the cause.

And if I am not wrong, there are two macros in FSL's kernel checking the
system_rev (arch/arm/plat-mxc/include/mach/mxc.h):

#define imx_cpu_ver()		(system_rev & 0xFF)
#define board_is_rev(rev)	(((system_rev & 0x0F00) == rev) ? 1 : 0)

But you defines board_rev as:
> +u32 get_board_rev(void)
> +{
> +	return 0x63000 ;

Both macro still return 0x00...there is no change if the ATAG is not
set. Sure that the problem you report is really bound to system_rev ? I
am quite OT here, we are investigating an issue in FSL kernel on the
U-boot ML.

Anyway, the ATAG is supported and as I already said quite common for
U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
require the revision atag to enable the VPU." should be extended
explaining the real cause (if known) or changed dropping VPU because
there is no clear relationship between the ATAG and the issue.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH 3/3] i.MX6: mx6qsabrelite: add ext2 support

[Stefano Babic]

On 02/03/2012 23:55, Eric Nelson wrote:
> Current Ubuntu releases from Freescale contain a boot script in ext3 filesystem.
> 
> Signed-off-by: Eric Nelson <eric.nelson@boundarydevices.com>
> ---

Applied to u-boot-imx, thanks.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels

[Albert ARIBAUD]

Hi Wolfgang,

Le 04/03/2012 09:39, Wolfgang Denk a ?crit :
> Dear Troy Kisky,
>
> In message<4F52C327.3080309@boundarydevices.com>  you wrote:
>>
>>>> And which changes 'introduce forward incompatibilities', and what are
>>>> these incompatibilities?
>>> See the recent problems that occurred when RMK decided to "clean up"
>>> the machids file.
>>>
>> Would you rather that I take RMK's cleaned up file, and undelete the
>> machines that u-boot
>> uses?  That would be more simple than adding to the board's config file.
>> I can delete all of the mach_is_xxx macros in mach-types while I'm at it.
>
> I think we had this discussion before (when RMK's changes hit us), and
> it was decided not to do this.  IIRC we decided not to do this.

YRC. :)

The retained strategy is to not fiddle with the Linux mach-type.h and to 
complement (and lampshade) any discrepancy between Linux and U-Boot 
support in the board config files.

> I have never understood why this mach_types thingy was needed (other
> rchitectures worked fine without it, or better), and now we are on the
> edge of obsoleting it. So all efforts trying to maintain this file are
> futile, and we would have to redo these for any updates of the file.
>
> I feel this is not a good investment of our time.

Hopefully FDT will make mach-types obsolete, except for the rare boards 
which will want to keep support for it, at which point we'll decide to 
either maintain or own, reduced and mostly legacy, mach-type file, or to 
move mach-type declarations in the board's config files.

> Best regards,
>
> Wolfgang Denk

Amicalement,
-- 
Albert.

[U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels)

[Eric Nelson]

Hi Stefano,

Thanks for the feedback and the prod.

On 03/07/2012 01:43 AM, Stefano Babic wrote:
> On 04/03/2012 20:45, Eric Nelson wrote:
>
>> The linkage is really indirect. The ATAG item is still supported in the
>> main-line kernel for ARM:
>>      http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=arch/arm/kernel/setup.c;h=a255c39612ca3cfa10bddb7c7728216efeeb04d5;hb=HEAD#l704
>>
>> The breakage I noticed was in the VPU driver, which refused to load
>> with a zero-value in system_rev. The net result was no video playback
>> in the Freescale Android ICS release.
>
> However, I have not found a statement in FSL's kernel where system_rev
> in VPU driver is checked, directly or indirectly - neither in VPU driver
> nor in the board initialization code, nor in another MXC driver.
> It seems to me a side-effect in FSL's kernel, and for some not yet known
> reasons the problem disappears with this tag - or can reappear later,
> because we do not know the cause.
>

I did the same search but was apparently not as persistent as you were.
The symptom is simple: Video won't play back on Android (R13.1 ICS)
without the system revision but play nicely with it.

> And if I am not wrong, there are two macros in FSL's kernel checking the
> system_rev (arch/arm/plat-mxc/include/mach/mxc.h):
>
> #define imx_cpu_ver()		(system_rev&  0xFF)
> #define board_is_rev(rev)	(((system_rev&  0x0F00) == rev) ? 1 : 0)
>
> But you defines board_rev as:
>> +u32 get_board_rev(void)
>> +{
>> +	return 0x63000 ;
>
> Both macro still return 0x00...there is no change if the ATAG is not
> set. Sure that the problem you report is really bound to system_rev ? I
> am quite OT here, we are investigating an issue in FSL kernel on the
> U-boot ML.
>

I think I just found the culprit, and it's in userspace, not in the
kernel. In package imx-lib-11.11.01, file vpu/vpu.c, there's this
routine that sets a global system_rev based on /proc/cpuinfo:

static int get_system_rev(void)
{
	FILE *fp;
	char buf[1024];
	int nread;
	char *tmp, *rev;
	int ret = -1;

	fp = fopen("/proc/cpuinfo", "r");
	if (fp == NULL) {
		perror("/proc/cpuinfo\n");
		return ret;
	}

	nread = fread(buf, 1, sizeof(buf), fp);
	fclose(fp);
	if ((nread == 0) || (nread == sizeof(buf))) {
		fclose(fp);
		return ret;
	}

	buf[nread] = '\0';

	tmp = strstr(buf, "Revision");
	if (tmp != NULL) {
		rev = index(tmp, ':');
		if (rev != NULL) {
			rev++;
			system_rev = strtoul(rev, NULL, 16);
			ret = 0;
		}
	}

	return ret;
}

The global is then exported via macros:
	vpu/vpu_io.c:unsigned int system_rev;
	vpu/vpu_io.c:static int get_system_rev(void)
	vpu/vpu_io.c:			system_rev = strtoul(rev, NULL, 16);
	vpu/vpu_io.c:	ret = get_system_rev();
	vpu/vpu_lib.h:extern unsigned int system_rev;
	vpu/vpu_lib.h:#define mxc_cpu()               (system_rev >> 12)
	vpu/vpu_lib.h:#define mxc_cpu_rev()           (system_rev & 0xFF)

and used to find the firmware file:
	vpu/vpu_util.c:	sprintf(temp_str, "vpu_fw_imx%2x.bin", mxc_cpu());

...all to support the userspace I/O for the VPU.

We are way off topic here, but I certainly hope we can address this in the
future and get a real driver written for the VPU.

> Anyway, the ATAG is supported and as I already said quite common for
> U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
> require the revision atag to enable the VPU." should be extended
> explaining the real cause (if known) or changed dropping VPU because
> there is no clear relationship between the ATAG and the issue.
>

How about something more generic like this?
	"Freescale Linux distributions depend on system_rev".

> Best regards,
> Stefano Babic
>

[U-Boot] CONFIG_REVISION (was i.MX6: mx6qsabrelite: allow use with Freescale 2.6.38 kernels)

[Stefano Babic]

On 07/03/2012 16:53, Eric Nelson wrote:
> Hi Stefano,
> 

Hi Eric,

> 
> I did the same search but was apparently not as persistent as you were.
> The symptom is simple: Video won't play back on Android (R13.1 ICS)
> without the system revision but play nicely with it.

Ok, we know the symptoms...

> I think I just found the culprit, and it's in userspace, not in the
> kernel. In package imx-lib-11.11.01, file vpu/vpu.c, there's this
> routine that sets a global system_rev based on /proc/cpuinfo:
> 
> static int get_system_rev(void)
> {
>     FILE *fp;
>     char buf[1024];
>     int nread;
>     char *tmp, *rev;
>     int ret = -1;
> 
>     fp = fopen("/proc/cpuinfo", "r");
>     if (fp == NULL) {
>         perror("/proc/cpuinfo\n");
>         return ret;
>     }
> 
>     nread = fread(buf, 1, sizeof(buf), fp);
>     fclose(fp);
>     if ((nread == 0) || (nread == sizeof(buf))) {
>         fclose(fp);
>         return ret;
>     }
> 
>     buf[nread] = '\0';
> 
>     tmp = strstr(buf, "Revision");
>     if (tmp != NULL) {
>         rev = index(tmp, ':');
>         if (rev != NULL) {
>             rev++;
>             system_rev = strtoul(rev, NULL, 16);
>             ret = 0;
>         }
>     }
> 
>     return ret;
> }
> 
> The global is then exported via macros:
>     vpu/vpu_io.c:unsigned int system_rev;
>     vpu/vpu_io.c:static int get_system_rev(void)
>     vpu/vpu_io.c:            system_rev = strtoul(rev, NULL, 16);
>     vpu/vpu_io.c:    ret = get_system_rev();
>     vpu/vpu_lib.h:extern unsigned int system_rev;
>     vpu/vpu_lib.h:#define mxc_cpu()               (system_rev >> 12)
>     vpu/vpu_lib.h:#define mxc_cpu_rev()           (system_rev & 0xFF)
> 
> and used to find the firmware file:
>     vpu/vpu_util.c:    sprintf(temp_str, "vpu_fw_imx%2x.bin", mxc_cpu());
> 
> ...all to support the userspace I/O for the VPU.

Understood - this is really crappy, because it makes so absurd
dependencies that is very easy to break - and when it happens, nobody
knows why, as we find now. Really this is a problem neither in u-boot
nor in kernel...

> 
> We are way off topic here,

Well, we have now the cause...

> but I certainly hope we can address this in the
> future and get a real driver written for the VPU.

..in the mainline kernel...

> 
>> Anyway, the ATAG is supported and as I already said quite common for
>> U-Boot boards. The commit message " Freescale 2.6.38 (Non-DT) kernels
>> require the revision atag to enable the VPU." should be extended
>> explaining the real cause (if known) or changed dropping VPU because
>> there is no clear relationship between the ATAG and the issue.
>>
> 
> How about something more generic like this?
>     "Freescale Linux distributions depend on system_rev".

Because you deeply investigated and found the reason, I propose you add
a full description indicating that the imx lib libraries depend on the
system_rev in kernel to transfer the correct firmware. So we know it is
neither a problem in u-boot nor in kernel, but we as u-bootlers are fair
with some bad implemented libraries....

Normally I would say that the fix should be done where the bug is - we
are introducing a work-around for a problem in user space. But as I
stated previously, the revision tag is used on a lot of ARM boards, and
there is no reason to reject it only on this board.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================