Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
Hi Adam, On 19/07/2016 19:51, Adam Duskett wrote: >> From: feste...@gmail.com >> Date: Tue, 19 Jul 2016 14:46:38 -0300 >> Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. >> To: adamdusk...@outlook.com >> CC: u-boot@lists.denx.de; sba...@denx.de >> >> On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett > 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 -- = DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 2:58 PM, Adam Duskett 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 02:51:11PM -0300, Fabio Estevam wrote: > On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini 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. -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
> From: feste...@gmail.com > Date: Tue, 19 Jul 2016 14:51:11 -0300 > Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. > To: tr...@konsulko.com > CC: adamdusk...@outlook.com; u-boot@lists.denx.de > > On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini 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? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
> From: feste...@gmail.com> Date: Tue, 19 Jul 2016 14:46:38 -0300 > Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names. > To: adamdusk...@outlook.com > CC: u-boot@lists.denx.de; sba...@denx.de > > On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 02:32:17PM -0300, Fabio Estevam wrote:
> On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett 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 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
> >> ---
> >> 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
> > ---
> > 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.
--
Tom
signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 2:43 PM, Tom Rini 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
Dear Adam,
In message 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
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Which witch wished which wicked wish?
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 2:40 PM, Adam Duskett 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. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
> From: feste...@gmail.com
> Date: Tue, 19 Jul 2016 14:32:17 -0300
> Subject: Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
> To: adamdusk...@outlook.com
> CC: u-boot@lists.denx.de; sba...@denx.de
>
> On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett 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 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
> >> ---
> >> 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
> > ---
> > 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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [v2 1/1] wandboard: fix dtb file names.
On Tue, Jul 19, 2016 at 2:23 PM, Adam Duskett 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 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
>> ---
>> 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
> ---
> 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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[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 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
> ---
> 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
---
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
#include
#include
+#include
+#include
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=0x1300\0" \
@@ -159,7 +148,6 @@
func(DHCP, dhcp, na)
#define CONFIG_BOOTCOMMAND \
- "run findfdt; " \
"run distro_bootcmd"
#include
--
2.7.4
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
