[U-Boot] [v2 1/1] wandboard: fix dtb file names.
With the previous suggestion from Wolfgang Denk, this patch removes the findfdt function from wandboard.h and instead replaces it with some simple logic in wandboard.c.
The new function "set_fdtfile" is called in board_late_init. This function simply finds the length of the dtb file name, allocates enough space for that string, and sets the fdtfile name to that string.
This results in slightly shorter / faster script code as well.
Dear Adam Duskett,
In message BLU436-SMTP46BCB8C63151CD3166FBD1B9360@phx.gbl you wrote:
With the release of kernel 4.1.15 for the imx6 line of processors, wandboard now uses imx6q-wandboard-revc1.dtb and imx6dl-wandboard-revc1.dtb. This patch fixes the naming convention to work with kernel 4.1.15
Signed-off-by: Adam Duskett adamduskett@outlook.com
include/configs/wandboard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index 99f5c0c..d41b600 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -135,9 +135,9 @@ "setenv bootargs ${bootargs} ${fbmem}\0" \ "findfdt="\ "if test $board_name = C1 && test $board_rev = MX6Q ; then " \
- "setenv fdtfile imx6q-wandboard.dtb; fi; " \
- "setenv fdtfile imx6q-wandboard-revc1.dtb; fi; " \
"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
- "setenv fdtfile imx6dl-wandboard.dtb; fi; " \
- "setenv fdtfile imx6dl-wandboard-revc1.dtb; fi; " \
"if test $board_name = B1 && test $board_rev = MX6Q ; then " \ "setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \ "if test $board_name = B1 && test $board_rev = MX6DL ; then " \
Instead of adding to an ever growing list of names, would it not make sense to solve this programmatically, like by concatenating "i" + tolower($board_rev) + "-wandboard-rev" + tolower($board_name) + ".dtb"?
I guess this would also result in shorter / faster script code?
Best regards,
Wolfgang Denk
Signed-off-by: Adam Duskett Adamduskett@outlook.com --- Changes:
v1 - v2: - Removed findfdt from h file and moved it to init code.
board/wandboard/wandboard.c | 20 ++++++++++++++++++++ include/configs/wandboard.h | 12 ------------ 2 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/board/wandboard/wandboard.c b/board/wandboard/wandboard.c index 4ce74cd..a1037ed 100644 --- a/board/wandboard/wandboard.c +++ b/board/wandboard/wandboard.c @@ -29,6 +29,8 @@ #include <phy.h> #include <input.h> #include <i2c.h> +#include <linux/ctype.h> +#include <malloc.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -377,6 +379,22 @@ static bool is_revc1(void) return false; }
+static void set_fdtfile(void) +{ + char *fdtfile; + int i; + int length = strlen(getenv("board_rev")) + strlen("-wandboard-rev") + + strlen(getenv("board_name")) + strlen(".dtb") + 1; + fdtfile = malloc(length); + sprintf(fdtfile, "i%s-wandboard-rev%s.dtb", getenv("board_rev"), getenv("board_name")); + for (i = 0; i < length; i++){ + fdtfile[i] = tolower(fdtfile[i]); + } + + setenv("fdtfile", fdtfile); + free(fdtfile); +} + int board_late_init(void) { #ifdef CONFIG_CMD_BMODE @@ -393,6 +411,8 @@ int board_late_init(void) setenv("board_name", "C1"); else setenv("board_name", "B1"); + + set_fdtfile(); #endif return 0; } diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index 99f5c0c..778c3f5 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -133,17 +133,6 @@ "echo '- no FWBADAPT-7WVGA-LCD-F07A-0102 display';" \ "fi; " \ "setenv bootargs ${bootargs} ${fbmem}\0" \ - "findfdt="\ - "if test $board_name = C1 && test $board_rev = MX6Q ; then " \ - "setenv fdtfile imx6q-wandboard.dtb; fi; " \ - "if test $board_name = C1 && test $board_rev = MX6DL ; then " \ - "setenv fdtfile imx6dl-wandboard.dtb; fi; " \ - "if test $board_name = B1 && test $board_rev = MX6Q ; then " \ - "setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \ - "if test $board_name = B1 && test $board_rev = MX6DL ; then " \ - "setenv fdtfile imx6dl-wandboard-revb1.dtb; fi; " \ - "if test $fdtfile = undefined; then " \ - "echo WARNING: Could not determine dtb to use; fi; \0" \ "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "pxefile_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \ "ramdisk_addr_r=0x13000000\0" \ @@ -159,7 +148,6 @@ func(DHCP, dhcp, na)
#define CONFIG_BOOTCOMMAND \ - "run findfdt; " \ "run distro_bootcmd"
#include <config_distro_bootcmd.h>
On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett Adamduskett@outlook.com wrote:
With the previous suggestion from Wolfgang Denk, this patch removes the findfdt function from wandboard.h and instead replaces it with some simple logic in wandboard.c.
The new function "set_fdtfile" is called in board_late_init. This function simply finds the length of the dtb file name, allocates enough space for that string, and sets the fdtfile name to that string.
This results in slightly shorter / faster script code as well.
Dear Adam Duskett,
In message BLU436-SMTP46BCB8C63151CD3166FBD1B9360@phx.gbl you wrote:
With the release of kernel 4.1.15 for the imx6 line of processors, wandboard now uses imx6q-wandboard-revc1.dtb and imx6dl-wandboard-revc1.dtb. This patch fixes the naming convention to work with kernel 4.1.15
Signed-off-by: Adam Duskett adamduskett@outlook.com
include/configs/wandboard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index 99f5c0c..d41b600 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -135,9 +135,9 @@ "setenv bootargs ${bootargs} ${fbmem}\0" \ "findfdt="\ "if test $board_name = C1 && test $board_rev = MX6Q ; then " \
"setenv fdtfile imx6q-wandboard.dtb; fi; " \
"setenv fdtfile imx6q-wandboard-revc1.dtb; fi; " \"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
"setenv fdtfile imx6dl-wandboard-revc1.dtb; fi; " \"if test $board_name = B1 && test $board_rev = MX6Q ; then " \ "setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \ "if test $board_name = B1 && test $board_rev = MX6DL ; then " \
Instead of adding to an ever growing list of names, would it not make sense to solve this programmatically, like by concatenating "i" + tolower($board_rev) + "-wandboard-rev" + tolower($board_name) + ".dtb"?
I guess this would also result in shorter / faster script code?
Best regards,
Wolfgang Denk
Signed-off-by: Adam Duskett Adamduskett@outlook.com
Changes:
v1 - v2:
- Removed findfdt from h file and moved it to init code.
board/wandboard/wandboard.c | 20 ++++++++++++++++++++ include/configs/wandboard.h | 12 ------------ 2 files changed, 20 insertions(+), 12 deletions(-)
This is not really making things simpler as shown by the diff stat.
I prefer to keep the current code as is.
From: festevam@gmail.com Date: Tue, 19 Jul 2016 14:32:17 -0300 Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. To: Adamduskett@outlook.com CC: u-boot@lists.denx.de; sbabic@denx.de
On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett Adamduskett@outlook.com wrote:
With the previous suggestion from Wolfgang Denk, this patch removes the findfdt function from wandboard.h and instead replaces it with some simple logic in wandboard.c.
The new function "set_fdtfile" is called in board_late_init. This function simply finds the length of the dtb file name, allocates enough space for that string, and sets the fdtfile name to that string.
This results in slightly shorter / faster script code as well.
Dear Adam Duskett,
In message BLU436-SMTP46BCB8C63151CD3166FBD1B9360@phx.gbl you wrote:
With the release of kernel 4.1.15 for the imx6 line of processors, wandboard now uses imx6q-wandboard-revc1.dtb and imx6dl-wandboard-revc1.dtb. This patch fixes the naming convention to work with kernel 4.1.15
Signed-off-by: Adam Duskett adamduskett@outlook.com
include/configs/wandboard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index 99f5c0c..d41b600 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -135,9 +135,9 @@ "setenv bootargs ${bootargs} ${fbmem}\0" \ "findfdt="\ "if test $board_name = C1 && test $board_rev = MX6Q ; then " \
"setenv fdtfile imx6q-wandboard.dtb; fi; " \
"setenv fdtfile imx6q-wandboard-revc1.dtb; fi; " \"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
"setenv fdtfile imx6dl-wandboard-revc1.dtb; fi; " \"if test $board_name = B1 && test $board_rev = MX6Q ; then " \ "setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \ "if test $board_name = B1 && test $board_rev = MX6DL ; then " \
Instead of adding to an ever growing list of names, would it not make sense to solve this programmatically, like by concatenating "i" + tolower($board_rev) + "-wandboard-rev" + tolower($board_name) + ".dtb"?
I guess this would also result in shorter / faster script code?
Best regards,
Wolfgang Denk
Signed-off-by: Adam Duskett Adamduskett@outlook.com
Changes:
v1 - v2:
- Removed findfdt from h file and moved it to init code.
board/wandboard/wandboard.c | 20 ++++++++++++++++++++ include/configs/wandboard.h | 12 ------------ 2 files changed, 20 insertions(+), 12 deletions(-)
This is not really making things simpler as shown by the diff stat.It changes it to be programatic instead of static.
I prefer to keep the current code as is.
If it is left as is, then the previous version of the patch needs to be appliedbecause the new dtb file names in kernel 4.1.15 are different.
On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett AdamDuskett@outlook.com wrote:
I prefer to keep the current code as is.
If it is left as is, then the previous version of the patch needs to be applied because the new dtb file names in kernel 4.1.15 are different.
Then please change the name of the 4.1.15 vendor kernel dts to match the one from mainline.
From: festevam@gmail.com> Date: Tue, 19 Jul 2016 14:46:38 -0300 Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. To: AdamDuskett@outlook.com CC: u-boot@lists.denx.de; sbabic@denx.de
On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett AdamDuskett@outlook.com wrote:
I prefer to keep the current code as is.
If it is left as is, then the previous version of the patch needs to be applied because the new dtb file names in kernel 4.1.15 are different.
Then please change the name of the 4.1.15 vendor kernel dts to match the one from mainline.
I did, and as per the commit message; Wolfgang Denk suggested making it into a logic based function? What would you like me to do? I can just resubmit the old patchwhich changed the vendor names, or would you like me to follow Wolfgang's directions? Adam
Hi Adam,
On 19/07/2016 19:51, Adam Duskett wrote:
From: festevam@gmail.com Date: Tue, 19 Jul 2016 14:46:38 -0300 Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. To: AdamDuskett@outlook.com CC: u-boot@lists.denx.de; sbabic@denx.de
On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett
AdamDuskett@outlook.com wrote:
I prefer to keep the current code as is.
If it is left as is, then the previous version of the patch needs to be applied because the new dtb file names in kernel 4.1.15 are different.
Then please change the name of the 4.1.15 vendor kernel dts to match the one from mainline.
I did, and as per the commit message; Wolfgang Denk suggested making it into a logic based function? What would you like me to do? I can just resubmit the old patch which changed the vendor names, or would you like me to follow Wolfgang's directions?
There is something wrong in the approach. Please think that this is the "default" environment fopr the wandboard, and this does not mean that you have *always* to use the same environment. It is not thinkable that anytime the kernel name (or something like this) is changed, u-boot *code* must be changed.
More in the case of "community" boards like a wandboard, it must be ensured that everybody can still work with it, without breaking for small things as the DT's name the behaviour.
wandboar uses distro environment: why do not put a "boot.scr" in your /boot directory on SD, as wandboard boots only from SD, and you overwrite all values you want ?
Best regards, Stefano Babic
On Tue, Jul 19, 2016 at 02:32:17PM -0300, Fabio Estevam wrote:
On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett Adamduskett@outlook.com wrote:
With the previous suggestion from Wolfgang Denk, this patch removes the findfdt function from wandboard.h and instead replaces it with some simple logic in wandboard.c.
The new function "set_fdtfile" is called in board_late_init. This function simply finds the length of the dtb file name, allocates enough space for that string, and sets the fdtfile name to that string.
This results in slightly shorter / faster script code as well.
Dear Adam Duskett,
In message BLU436-SMTP46BCB8C63151CD3166FBD1B9360@phx.gbl you wrote:
With the release of kernel 4.1.15 for the imx6 line of processors, wandboard now uses imx6q-wandboard-revc1.dtb and imx6dl-wandboard-revc1.dtb. This patch fixes the naming convention to work with kernel 4.1.15
Signed-off-by: Adam Duskett adamduskett@outlook.com
include/configs/wandboard.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/configs/wandboard.h b/include/configs/wandboard.h index 99f5c0c..d41b600 100644 --- a/include/configs/wandboard.h +++ b/include/configs/wandboard.h @@ -135,9 +135,9 @@ "setenv bootargs ${bootargs} ${fbmem}\0" \ "findfdt="\ "if test $board_name = C1 && test $board_rev = MX6Q ; then " \
"setenv fdtfile imx6q-wandboard.dtb; fi; " \
"setenv fdtfile imx6q-wandboard-revc1.dtb; fi; " \"if test $board_name = C1 && test $board_rev = MX6DL ; then " \
"setenv fdtfile imx6dl-wandboard.dtb; fi; " \
"setenv fdtfile imx6dl-wandboard-revc1.dtb; fi; " \"if test $board_name = B1 && test $board_rev = MX6Q ; then " \ "setenv fdtfile imx6q-wandboard-revb1.dtb; fi; " \ "if test $board_name = B1 && test $board_rev = MX6DL ; then " \
Instead of adding to an ever growing list of names, would it not make sense to solve this programmatically, like by concatenating "i" + tolower($board_rev) + "-wandboard-rev" + tolower($board_name) + ".dtb"?
I guess this would also result in shorter / faster script code?
Best regards,
Wolfgang Denk
Signed-off-by: Adam Duskett Adamduskett@outlook.com
Changes:
v1 - v2:
- Removed findfdt from h file and moved it to init code.
board/wandboard/wandboard.c | 20 ++++++++++++++++++++ include/configs/wandboard.h | 12 ------------ 2 files changed, 20 insertions(+), 12 deletions(-)
This is not really making things simpler as shown by the diff stat.
But this does show that we need to step back and think about how to more easily do what this is trying to do. If $soc was set as in Stefan's patch for example, we would just need board_name to be being set in lowercase rather than upper case.
On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini trini@konsulko.com wrote:
But this does show that we need to step back and think about how to more easily do what this is trying to do. If $soc was set as in Stefan's patch for example, we would just need board_name to be being set in lowercase rather than upper case.
This patch says in the subject: "fix dtb file names" and it gives no indication of what is really wrong with the current code. What is it fixing?
If I understand this correctly the goal of this patch is to make it work with a 4.1.15 kernel vendor dts naming scheme.
My suggestion is to change the kernel vendor dts name to the same used in mainline and then it will just work with the current U-boot code.
From: festevam@gmail.com Date: Tue, 19 Jul 2016 14:51:11 -0300 Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. To: trini@konsulko.com CC: Adamduskett@outlook.com; u-boot@lists.denx.de
On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini trini@konsulko.com wrote:
But this does show that we need to step back and think about how to more easily do what this is trying to do. If $soc was set as in Stefan's patch for example, we would just need board_name to be being set in lowercase rather than upper case.
This patch says in the subject: "fix dtb file names" and it gives no indication of what is really wrong with the current code. What is it fixing? The original patch that I submitted just changed the dts files to the new namingscheme found in 4.1.15. Once I submitted that, (as you can see in the commit message provided) Woflgang suggested changing it to some basic logic. If I understand this correctly the goal of this patch is to make it work with a 4.1.15 kernel vendor dts naming scheme.
My suggestion is to change the kernel vendor dts name to the same used in mainline and then it will just work with the current U-boot code.
That was the first version of the patch I submitted. Again; Wolfgang suggested changingit to a logic based function. Would you like me to just change the names statically? Or would you like me to set the string dynamically?
On Tue, Jul 19, 2016 at 2:58 PM, Adam Duskett AdamDuskett@outlook.com wrote:
The original patch that I submitted just changed the dts files to the new naming scheme found in 4.1.15. Once I submitted that,
And the naming scheme in 4.1.15 vendor kernel is the problem. Make it match the dtb names of the mainline kernel and this problem will be solved.
Let me try to clarify: you are trying to make a U-boot change so that it can read a dtb file called: imx6q-wandboard-revc1.dtb.
If you look at the dtb's in the mainline kernel we have (let's take imx6q for example):
imx6q-wandboard.dtb ---> This should be used with revC1 boards imx6q-wandboard-revb1.dtb ---> This should be used with revB1 boards
Just fix 4.1.15 to produce the same file names and then you don't need to modify U-boot.
On Tue, Jul 19, 2016 at 02:51:11PM -0300, Fabio Estevam wrote:
On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini trini@konsulko.com wrote:
But this does show that we need to step back and think about how to more easily do what this is trying to do. If $soc was set as in Stefan's patch for example, we would just need board_name to be being set in lowercase rather than upper case.
This patch says in the subject: "fix dtb file names" and it gives no indication of what is really wrong with the current code. What is it fixing?
If I understand this correctly the goal of this patch is to make it work with a 4.1.15 kernel vendor dts naming scheme.
Also true, and I agree, we (mainline U-Boot) shouldn't be trying to match non-mainline DT names.
My suggestion is to change the kernel vendor dts name to the same used in mainline and then it will just work with the current U-boot code.
Yes. But it also shows that, ug, this could be better.
Dear Adam,
In message BLU436-SMTP222E98D640B2559335122F3B9370@phx.gbl you wrote:
- int length = strlen(getenv("board_rev")) + strlen("-wandboard-rev") +
strlen(getenv("board_name")) + strlen(".dtb") + 1;- fdtfile = malloc(length);
Don't you think you should add some error checking here? Each of the getenv() calls may fail, as well as the malloc() - and then?
As is: NAK!!
Best regards,
Wolfgang Denk
participants (6)
-
Adam Duskett -
Adam Duskett
-
Fabio Estevam -
Stefano Babic -
Tom Rini -
Wolfgang Denk