Skip to contentSkip to conversation

imx-common: cpu: add fdt_file environment variable

PATCH New Troy Kisky 5 participants 13 messages Mar 29, 2014 – Apr 1, 2014

Subsystems (2)
Arm (32-bit) Arm i.MX and Layerscape
lore
Share & tools
lore

Troy Kisky

author
#0 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>
---
 arch/arm/imx-common/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/lib/board.c      |  7 +++++++
 2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +44 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 	ipuv3_fb_shutdown();
 }
 #endif
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+	switch (imxtype) {
+	case MXC_CPU_MX6Q:
+	case MXC_CPU_MX6D:
+		return "imx6q";		/* Quad/Dual-core version of the mx6 */
+	case MXC_CPU_MX6DL:
+	case MXC_CPU_MX6SOLO:
+		return "imx6dl";	/* Dual Lite/Solo version of the mx6 */
+	case MXC_CPU_MX6SL:
+		return "imx6sl";	/* Solo-Lite version of the mx6 */
+	case MXC_CPU_MX51:
+		return "imx51";
+	case MXC_CPU_MX53:
+		return "imx53";
+	}
+	return "??";
+}
+
+int cpu_late_init(void)
+{
+	char buf[128];
+	const char *board;
+	u32 imxtype = (get_cpu_rev() >> 12) & 0xff;
+
+	if (getenv("fdt_file"))
+		return 0;
+	board = getenv("board");
+	if (!board) {
+		board = CONFIG_SYS_BOARD;
+		if ((board[0] == 'm') && (board[1] == 'x')) {
+			if (board[2] == '6') {
+				board += 3;
+			} else if (board[2] == '5') {
+				if ((board[3] == '1') || (board[3] == '3'))
+					board += 4;
+			}
+		}
+	}
+	sprintf(buf, "%s-%s.dtb", get_dtb_prefix(imxtype), board);
+	setenv("fdt_file", buf);
+	return 0;
+}
arch/arm/lib/board.c +7 -0
index e9a7708..61cee98 100644
@@ -496,6 +496,11 @@ static void display_fdt_model(const void *blob)
 }
 #endif
 
+int __weak cpu_late_init(void)
+{
+	return 0;
+}
+
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now

@@ -649,6 +654,8 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	board_late_init();
 #endif
 
+	cpu_late_init();
+
 #ifdef CONFIG_BITBANGMII
 	bb_miiphy_init();
 #endif
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 6 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

Nice! I do think it is in the right direction. I will give it a try soon...

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
#2 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 11 lines · Otavio Salvador

On Sat, Mar 29, 2014 at 7:34 PM, Troy Kisky

Quoted 6 lines · <[email protected]> wrote:

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

Nice! I do think it is in the right direction. I will give it a try soon...

Full NAK on this patch, I completely agree with Eric that this approach is totally wrong. This adds stuff which should be pulled from DT into common code, this is just NAK.

Best regards,
Marek Vasut

author
#3 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 11 lines · Marek Vasut
Quoted 10 lines · Otavio Salvador

On Sat, Mar 29, 2014 at 7:34 PM, Troy Kisky

Quoted 6 lines · <[email protected]> wrote:

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>
Quoted 9 lines
Quoted 1 line

Nice! I do think it is in the right direction. I will give it a try soon...

Full NAK on this patch, I completely agree with Eric that this approach is totally wrong. This adds stuff which should be pulled from DT into common code, this is just NAK.

Best regards,
Marek Vasut

I am not sure what you are suggesting. Don't you have an chicken and egg problem?

Troy

#4 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

In reply to Troy Kisky

Hi Troy,

Quoted 29 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>
---
  arch/arm/imx-common/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
  arch/arm/lib/board.c      |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +10 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
  	ipuv3_fb_shutdown();
  }
  #endif
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+	switch (imxtype) {
+	case MXC_CPU_MX6Q:
+	case MXC_CPU_MX6D:
+		return "imx6q";		/* Quad/Dual-core version of the mx6 */
+	case MXC_CPU_MX6DL:
+	case MXC_CPU_MX6SOLO:
+		return "imx6dl";	/* Dual Lite/Solo version of the mx6 */

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 10 lines
arch/arm/imx-common/cpu.c +10 -0
+	case MXC_CPU_MX6SL:
+		return "imx6sl";	/* Solo-Lite version of the mx6 */
+	case MXC_CPU_MX51:
+		return "imx51";
+	case MXC_CPU_MX53:
+		return "imx53";
+	}
+	return "??";
+}
+

I really dislike this implementation of naming policy in code.

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 213

Quoted 11 lines
arch/arm/imx-common/cpu.c +11 -0
+int cpu_late_init(void)
+{
+	char buf[128];
+	const char *board;
+	u32 imxtype = (get_cpu_rev() >> 12) & 0xff;
+
+	if (getenv("fdt_file"))
+		return 0;
+	board = getenv("board");
+	if (!board) {
+		board = CONFIG_SYS_BOARD;

And this part seems to impose a board-naming requirement to implement the dtb-naming requirement:

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 226

Quoted 13 lines
arch/arm/imx-common/cpu.c +13 -0
+		if ((board[0] == 'm') && (board[1] == 'x')) {
+			if (board[2] == '6') {
+				board += 3;
+			} else if (board[2] == '5') {
+				if ((board[3] == '1') || (board[3] == '3'))
+					board += 4;
+			}
+		}
+	}
+	sprintf(buf, "%s-%s.dtb", get_dtb_prefix(imxtype), board);
+	setenv("fdt_file", buf);
+	return 0;
+}

I sent patches last year to provide default "cpu" and "board" environment variables that could be used by boot scripts to implement this part.

	http://lists.denx.de/pipermail/u-boot/2013-November/thread.html#167129

That seems more generally useful.

Otherwise, I'd rather just see an environment-variable convention.

Regards,

Eric

author
#5 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 36 lines · Eric Nelson

Hi Troy,

Quoted 33 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>
---
  arch/arm/imx-common/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
  arch/arm/lib/board.c      |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +14 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
      ipuv3_fb_shutdown();
  }
  #endif
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6 */
+    case MXC_CPU_MX6DL:
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

Troy

#6 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 51 lines · Troy Kisky
Quoted 41 lines · Eric Nelson

Hi Troy,

Quoted 38 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

  arch/arm/imx-common/cpu.c | 44
  ++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c    
   |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +14 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 
      ipuv3_fb_shutdown();
 
  }
  #endif
 
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6 */
+    case MXC_CPU_MX6DL:
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

Quoted 5 lines

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

You can make the CPU code set the CPU type into some variable. Afterwards, some scripts in the HUSH can assemble the DTB name based on those variables. This would be much more generic and re-usable than this ...

Best regards,
Marek Vasut

#7 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 58 lines · Marek Vasut
Quoted 51 lines · Troy Kisky
Quoted 41 lines · Eric Nelson

Hi Troy,

Quoted 38 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

  arch/arm/imx-common/cpu.c | 44
  ++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c
   |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +14 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 
      ipuv3_fb_shutdown();
 
  }
  #endif
 
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6 */
+    case MXC_CPU_MX6DL:
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

Quoted 5 lines

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

Quoted 3 lines

You can make the CPU code set the CPU type into some variable. Afterwards, some scripts in the HUSH can assemble the DTB name based on those variables. This would be much more generic and re-usable than this ...

The problem is this script will be mostly the same for most boards.

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
#8 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 61 lines · Otavio Salvador
Quoted 60 lines · Marek Vasut
Quoted 58 lines · Troy Kisky
Quoted 43 lines · Eric Nelson

Hi Troy,

Quoted 40 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

  arch/arm/imx-common/cpu.c | 44

++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c

   |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +7 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 
      ipuv3_fb_shutdown();
 
  }
  #endif
 
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6

*/ + case MXC_CPU_MX6DL:

arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

Quoted 6 lines
Quoted 4 lines

You can make the CPU code set the CPU type into some variable. Afterwards, some scripts in the HUSH can assemble the DTB name based on those variables. This would be much more generic and re-usable than this
...

The problem is this script will be mostly the same for most boards.

This does by no means justify poluting code with non-reusable convoluted stuff. A script which is part of the environment can be tweaked on a running system as seen fit at any later date, updating bootloader on a running system is often not an option.

Furthermore, having partial information which can be used for decisionmaking is much better than having a lengthy string which needs to be parsed first. Especially with the limited parsing options we have in some configurations of U- Boot.

Best regards,
Marek Vasut

author
#9 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 74 lines · Marek Vasut
Quoted 61 lines · Otavio Salvador
Quoted 60 lines · Marek Vasut
Quoted 58 lines · Troy Kisky
Quoted 43 lines · Eric Nelson

Hi Troy,

Quoted 40 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

  arch/arm/imx-common/cpu.c | 44

++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c

   |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +7 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 
      ipuv3_fb_shutdown();
 
  }
  #endif
 
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6

*/ + case MXC_CPU_MX6DL:

arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c void arch_preboot_os(void) · line 202

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

Quoted 6 lines
Quoted 4 lines

You can make the CPU code set the CPU type into some variable. Afterwards, some scripts in the HUSH can assemble the DTB name based on those variables. This would be much more generic and re-usable than this
...

The problem is this script will be mostly the same for most boards.

This does by no means justify poluting code with non-reusable convoluted stuff. A script which is part of the environment can be tweaked on a running system as seen fit at any later date, updating bootloader on a running system is often not an option.

Quoted 5 lines

Furthermore, having partial information which can be used for decisionmaking is much better than having a lengthy string which needs to be parsed first. Especially with the limited parsing options we have in some configurations of U- Boot.

I can agree to that, if I understand you correctly. So are you fine with having a board and cpu variable? If so, what should the cpu variable contain?

I propose cpu defaults to "IMX%s", get_imx_type(imxtype)

so cpu=IMX6Q, IMX6D, IMX6DL, IMX6SOLO, IMX6SL, IMX51, IMX53

and board defaults to plain CONFIG_SYS_BOARD. So, mx6sabresd would need to set board=sabresd.

in mx6sabresd.h
setenv board=sabresd

and in some common file
setenv dtbpIMX6Q setenv dtbprefix imx6q
setenv dtbpIMX6D setenv dtbprefix imx6d
setenv dtbpIMX6DL setenv dtbprefix imx6dl
setenv dtbpIMX6SOLO setenv dtbprefix imx6dl
setenv dtbpIMX6SL setenv dtbprefix imx6sl
setenv find_dtb_file 'run dtbp${cpu} ; setenv fdt_file $dtbprefix-$board'

run find_dtb_file
echo $fdt_file

--------
Or if you know a better way please speak up.

Thanks
Troy

#10 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 75 lines · Troy Kisky
Quoted 74 lines · Marek Vasut
Quoted 61 lines · Otavio Salvador
Quoted 60 lines · Marek Vasut
Quoted 58 lines · Troy Kisky
Quoted 43 lines · Eric Nelson

Hi Troy,

Quoted 40 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

  arch/arm/imx-common/cpu.c | 44

++++++++++++++++++++++++++++++++++++++++++++ arch/arm/lib/board.c

   |  7 +++++++
  2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +7 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 
      ipuv3_fb_shutdown();
 
  }
  #endif
 
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+    switch (imxtype) {
+    case MXC_CPU_MX6Q:
+    case MXC_CPU_MX6D:
+        return "imx6q";        /* Quad/Dual-core version of the mx6

*/ + case MXC_CPU_MX6DL:

arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX6SOLO:
+        return "imx6dl";    /* Dual Lite/Solo version of the mx6 */
+    case MXC_CPU_MX6SL:
+        return "imx6sl";    /* Solo-Lite version of the mx6 */
+    case MXC_CPU_MX51:
+        return "imx51";
Quoted 8 lines

arch/arm/imx-common/cpu.c

Quoted 6 lines
arch/arm/imx-common/cpu.c +6 -0
+    case MXC_CPU_MX53:
+        return "imx53";
+    }
+    return "??";
+}
+

I really dislike this implementation of naming policy in code.

It is not truly a policy. It is a convenience which can be ignored if so desired. Though I do agree that cpu and board environment variables would also be useful. Still, a cpu variable would still require some scripting to combine the quad/dual, duallite/solo. So, your way is not as convenient for dtb file names.

Quoted 6 lines
Quoted 4 lines

You can make the CPU code set the CPU type into some variable. Afterwards, some scripts in the HUSH can assemble the DTB name based on those variables. This would be much more generic and re-usable than this ...

The problem is this script will be mostly the same for most boards.

This does by no means justify poluting code with non-reusable convoluted stuff. A script which is part of the environment can be tweaked on a running system as seen fit at any later date, updating bootloader on a running system is often not an option.

Quoted 8 lines
Quoted 4 lines

Furthermore, having partial information which can be used for decisionmaking is much better than having a lengthy string which needs to be parsed first. Especially with the limited parsing options we have in some configurations of U- Boot.

I can agree to that, if I understand you correctly. So are you fine with having a board and cpu variable? If so, what should the cpu variable contain?

Looks reasonable.

Quoted 1 line

I propose cpu defaults to "IMX%s", get_imx_type(imxtype)

This should be introduced and work for _all_ CPUs and must not be imx specific. Otherwise, for i.MX, you can pick whatever is suitable, IMX%s is OK.

Quoted 4 lines

so cpu=IMX6Q, IMX6D, IMX6DL, IMX6SOLO, IMX6SL, IMX51, IMX53

and board defaults to plain CONFIG_SYS_BOARD. So, mx6sabresd would need to set board=sabresd.

Again, should work across all boards.

Quoted 5 lines

in mx6sabresd.h
setenv board=sabresd

and in some common file
setenv dtbpIMX6Q setenv dtbprefix imx6q

This must be part of a per-board environment if and only if the board can contain multiple CPU configurations.
[...]

#11 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

In reply to Troy Kisky

Hi Troy,

Quoted 7 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

I have a general problem with this implementation. I am ok if, as you proposed some times ago, there is a general rule for the "default" dtb file in the CONFIG_EXTRA_ENV.

However, you are binding in code naming conventions. In U-boot, it must be allowed to set the environment as the user wants, and this must be not overwritten by such an internal code.

I mean: a board user, if he wants, should be allowed to do something as

setenv fdt_file my_preferred_dtb_name.dtb

and this must work when the file is loaded from storage - this is not possible if the rule chosen from user is overwritten by code.

This makes the environment useless and generates headaches for a lot of users. They will ask themselves why the wrong file is taken when they tried in any way to set it differently...

Quoted 74 lines
 arch/arm/imx-common/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 arch/arm/lib/board.c      |  7 +++++++
 2 files changed, 51 insertions(+)
arch/arm/imx-common/cpu.c +44 -0
index a77c4de..5d48011 100644
@@ -180,3 +180,47 @@ void arch_preboot_os(void)
 	ipuv3_fb_shutdown();
 }
 #endif
+
+const char *get_dtb_prefix(u32 imxtype)
+{
+	switch (imxtype) {
+	case MXC_CPU_MX6Q:
+	case MXC_CPU_MX6D:
+		return "imx6q";		/* Quad/Dual-core version of the mx6 */
+	case MXC_CPU_MX6DL:
+	case MXC_CPU_MX6SOLO:
+		return "imx6dl";	/* Dual Lite/Solo version of the mx6 */
+	case MXC_CPU_MX6SL:
+		return "imx6sl";	/* Solo-Lite version of the mx6 */
+	case MXC_CPU_MX51:
+		return "imx51";
+	case MXC_CPU_MX53:
+		return "imx53";
+	}
+	return "??";
+}
+
+int cpu_late_init(void)
+{
+	char buf[128];
+	const char *board;
+	u32 imxtype = (get_cpu_rev() >> 12) & 0xff;
+
+	if (getenv("fdt_file"))
+		return 0;
+	board = getenv("board");
+	if (!board) {
+		board = CONFIG_SYS_BOARD;
+		if ((board[0] == 'm') && (board[1] == 'x')) {
+			if (board[2] == '6') {
+				board += 3;
+			} else if (board[2] == '5') {
+				if ((board[3] == '1') || (board[3] == '3'))
+					board += 4;
+			}
+		}
+	}
+	sprintf(buf, "%s-%s.dtb", get_dtb_prefix(imxtype), board);
+	setenv("fdt_file", buf);
+	return 0;
+}
arch/arm/lib/board.c +5 -0
index e9a7708..61cee98 100644
@@ -496,6 +496,11 @@ static void display_fdt_model(const void *blob)
 }
 #endif
 
+int __weak cpu_late_init(void)
+{
+	return 0;
+}
+
 /************************************************************************
  *
  * This is the next part if the initialization sequence: we are now

arch/arm/lib/board.c void board_init_r(gd_t *id, ulong dest_addr) · line 661

Quoted 8 lines
arch/arm/lib/board.c +2 -0
@@ -649,6 +654,8 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	board_late_init();
 #endif
 
+	cpu_late_init();
+
 #ifdef CONFIG_BITBANGMII
 	bb_miiphy_init();
 #endif

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
=====================================================================
author
#12 lore raw

PATCH 1/1 imx-common: cpu: add fdt_file environment variable

Quoted 11 lines · Stefano Babic

Hi Troy,

Quoted 7 lines · Troy Kisky

This removes one block in the move toward 1 u-boot for both a mx6q (quad) and mx6dl (duallite) processor.

Now fdt_file hardcoded value can be removed.

Signed-off-by Troy Kisky <[email protected]>

---

Quoted 7 lines

I have a general problem with this implementation. I am ok if, as you proposed some times ago, there is a general rule for the "default" dtb file in the CONFIG_EXTRA_ENV.

However, you are binding in code naming conventions. In U-boot, it must be allowed to set the environment as the user wants, and this must be not overwritten by such an internal code.

In the patch, the code returns without any changes if fdt_file is already defined. So, I don't know what you are referring to here.

Quoted 7 lines

I mean: a board user, if he wants, should be allowed to do something as

setenv fdt_file my_preferred_dtb_name.dtb

and this must work when the file is loaded from storage - this is not possible if the rule chosen from user is overwritten by code.

I agree, but it does work.

Quoted 4 lines

This makes the environment useless and generates headaches for a lot of users. They will ask themselves why the wrong file is taken when they tried in any way to set it differently...

Still no disagreement.