Patchwork [U-Boot,RESEND,2/3] mx28evk: Add 'nandboot' environment command

login
register
mail settings
Submitter Otavio Salvador
Date Oct. 21, 2013, 11:24 p.m.
Message ID <1382397845-5313-3-git-send-email-otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/285295/
State Changes Requested
Delegated to: Stefano Babic
Headers show

Comments

Otavio Salvador - Oct. 21, 2013, 11:24 p.m.
This reads the kernel, ftd and boot into ubifs filesystem. While on
that, the SD firmware filename definition has been moved next to the
other SD related commands.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 include/configs/mx28evk.h | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
Marek Vasut - Oct. 27, 2013, 4:16 p.m.
Dear Otavio Salvador,

> This reads the kernel, ftd and boot into ubifs filesystem. While on
> that, the SD firmware filename definition has been moved next to the
> other SD related commands.
> 
> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>

Any changes to built-in environment are now NAKd I believe.

Best regards,
Marek Vasut
Otavio Salvador - Oct. 27, 2013, 7:51 p.m.
On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> This reads the kernel, ftd and boot into ubifs filesystem. While on
>> that, the SD firmware filename definition has been moved next to the
>> other SD related commands.
>>
>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>
> Any changes to built-in environment are now NAKd I believe.

I don't see a reason to not merged it for now; I will work in porting
the environments for the external file but this could go now, anyway.
Marek Vasut - Oct. 27, 2013, 8:47 p.m.
Dear Otavio Salvador,

> On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> This reads the kernel, ftd and boot into ubifs filesystem. While on
> >> that, the SD firmware filename definition has been moved next to the
> >> other SD related commands.
> >> 
> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> > 
> > Any changes to built-in environment are now NAKd I believe.
> 
> I don't see a reason to not merged it for now; I will work in porting
> the environments for the external file but this could go now, anyway.

Read [1], in particular what Stefano said.

http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/169042

Thus this patch shall be rejected as well, sorry.

Best regards,
Marek Vasut
Otavio Salvador - Oct. 28, 2013, 11:19 a.m.
Hello,

I am not sure I agree ...

On Sun, Oct 27, 2013 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
>> > Dear Otavio Salvador,
>> >
>> >> This reads the kernel, ftd and boot into ubifs filesystem. While on
>> >> that, the SD firmware filename definition has been moved next to the
>> >> other SD related commands.
>> >>
>> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>> >
>> > Any changes to built-in environment are now NAKd I believe.
>>
>> I don't see a reason to not merged it for now; I will work in porting
>> the environments for the external file but this could go now, anyway.
>
> Read [1], in particular what Stefano said.
>
> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/169042
>
> Thus this patch shall be rejected as well, sorry.

I agree with the goal but until we have a way to insert the /default/
environment into the binary of U-Boot for use (not adding an extra
environment image) this should be accepted.
Marek Vasut - Oct. 28, 2013, 11:31 a.m.
Dear Otavio Salvador,

> Hello,
> 
> I am not sure I agree ...
> 
> On Sun, Oct 27, 2013 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Otavio Salvador,
> >> > 
> >> >> This reads the kernel, ftd and boot into ubifs filesystem. While on
> >> >> that, the SD firmware filename definition has been moved next to the
> >> >> other SD related commands.
> >> >> 
> >> >> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
> >> > 
> >> > Any changes to built-in environment are now NAKd I believe.
> >> 
> >> I don't see a reason to not merged it for now; I will work in porting
> >> the environments for the external file but this could go now, anyway.
> > 
> > Read [1], in particular what Stefano said.
> > 
> > http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/169042
> > 
> > Thus this patch shall be rejected as well, sorry.
> 
> I agree with the goal but until we have a way to insert the /default/
> environment into the binary of U-Boot for use (not adding an extra
> environment image) this should be accepted.

I disagree with the decision to reject the envs as well, I just present previous 
discussion here. I will pull out of this discussion and leave the rest up to 
Stefano, since this is his call afterall.

Best regards,
Marek Vasut
Stefano Babic - Oct. 28, 2013, 11:32 a.m.
Hi Otavio,

On 28/10/2013 12:19, Otavio Salvador wrote:
> Hello,
> 
> I am not sure I agree ...
> 
> On Sun, Oct 27, 2013 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
>> Dear Otavio Salvador,
>>
>>> On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
>>>> Dear Otavio Salvador,
>>>>
>>>>> This reads the kernel, ftd and boot into ubifs filesystem. While on
>>>>> that, the SD firmware filename definition has been moved next to the
>>>>> other SD related commands.
>>>>>
>>>>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>>>>
>>>> Any changes to built-in environment are now NAKd I believe.
>>>
>>> I don't see a reason to not merged it for now; I will work in porting
>>> the environments for the external file but this could go now, anyway.
>>
>> Read [1], in particular what Stefano said.
>>
>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/169042
>>
>> Thus this patch shall be rejected as well, sorry.
> 
> I agree with the goal but until we have a way to insert the /default/
> environment into the binary of U-Boot for use (not adding an extra
> environment image) this should be accepted.

Indeed. However, why cannot we do the right thing soon ?

It is very interesting if Simon's patches will flow into mainline soon -
as feeling, I see some comments by Wolfgang, but no evident signal that
they will be stopped. Octavio, I will wait for your patches for a while
and if Simon's patches will be accepted soon, I will kindly ask you to
reformat your patches, moving the default environment into an .env file.

Best regards,
Stefano Babic
Otavio Salvador - Oct. 28, 2013, 12:03 p.m.
On Mon, Oct 28, 2013 at 9:32 AM, Stefano Babic <sbabic@denx.de> wrote:
> Hi Otavio,
>
> On 28/10/2013 12:19, Otavio Salvador wrote:
>> Hello,
>>
>> I am not sure I agree ...
>>
>> On Sun, Oct 27, 2013 at 6:47 PM, Marek Vasut <marex@denx.de> wrote:
>>> Dear Otavio Salvador,
>>>
>>>> On Sun, Oct 27, 2013 at 2:16 PM, Marek Vasut <marex@denx.de> wrote:
>>>>> Dear Otavio Salvador,
>>>>>
>>>>>> This reads the kernel, ftd and boot into ubifs filesystem. While on
>>>>>> that, the SD firmware filename definition has been moved next to the
>>>>>> other SD related commands.
>>>>>>
>>>>>> Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
>>>>>
>>>>> Any changes to built-in environment are now NAKd I believe.
>>>>
>>>> I don't see a reason to not merged it for now; I will work in porting
>>>> the environments for the external file but this could go now, anyway.
>>>
>>> Read [1], in particular what Stefano said.
>>>
>>> http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/169042
>>>
>>> Thus this patch shall be rejected as well, sorry.
>>
>> I agree with the goal but until we have a way to insert the /default/
>> environment into the binary of U-Boot for use (not adding an extra
>> environment image) this should be accepted.
>
> Indeed. However, why cannot we do the right thing soon ?

There're no pending patches to address the root problem yet (allowing
changing /internal and default/ environment).

> It is very interesting if Simon's patches will flow into mainline soon -
> as feeling, I see some comments by Wolfgang, but no evident signal that
> they will be stopped. Octavio, I will wait for your patches for a while
> and if Simon's patches will be accepted soon, I will kindly ask you to
> reformat your patches, moving the default environment into an .env file.

I will rework the environments as .env files but I don't want to hold
this change until this is accepted.

My patch is fine and could go in now as is. Once Simon's patches get
in I will post another series reworking the environment to convert it
to the .env file format but please don't block on this.
Wolfgang Denk - Oct. 28, 2013, 1:21 p.m.
Dear Otavio,

In message <CAP9ODKpN+PMX1HP+7bEMP7u-Hc44oNc6PEpAn+nbzbQm8cDfrg@mail.gmail.com> you wrote:
>
> > Indeed. However, why cannot we do the right thing soon ?
> 
> There're no pending patches to address the root problem yet (allowing
> changing /internal and default/ environment).

Simon's patch set "env: Add support for environment files" does
exactly that.

> I will rework the environments as .env files but I don't want to hold
> this change until this is accepted.

Please be patient, like we all are.  It's better to wait a bit now,
then adding work now, and adding more work later to clean up the mess
again.

> My patch is fine and could go in now as is. Once Simon's patches get
> in I will post another series reworking the environment to convert it
> to the .env file format but please don't block on this.

I agree with Stefano that we should not add more to the already
existing amount of env settings but instead wait for a cleaner way.
Please be patient - you lose nothing here.

Best regards,

Wolfgang Denk
Otavio Salvador - Oct. 28, 2013, 1:29 p.m.
On Mon, Oct 28, 2013 at 11:21 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Otavio,
>
> In message <CAP9ODKpN+PMX1HP+7bEMP7u-Hc44oNc6PEpAn+nbzbQm8cDfrg@mail.gmail.com> you wrote:
>>
>> > Indeed. However, why cannot we do the right thing soon ?
>>
>> There're no pending patches to address the root problem yet (allowing
>> changing /internal and default/ environment).
>
> Simon's patch set "env: Add support for environment files" does
> exactly that.

Doing it without changing source code, directly into the binary?

From what I read it allows import / export but this is at runtime, I
need it /before/ booting the board. So currently I need to patch the
environment files for it.

>> I will rework the environments as .env files but I don't want to hold
>> this change until this is accepted.
>
> Please be patient, like we all are.  It's better to wait a bit now,
> then adding work now, and adding more work later to clean up the mess
> again.

The work has already been done.

>> My patch is fine and could go in now as is. Once Simon's patches get
>> in I will post another series reworking the environment to convert it
>> to the .env file format but please don't block on this.
>
> I agree with Stefano that we should not add more to the already
> existing amount of env settings but instead wait for a cleaner way.
> Please be patient - you lose nothing here.

I really see no point in holding it as the work has been done already;
it is Stefano call but I disagree with postpone it as work is done and
being in use.

I will comment on Simon's patch to clarify my understanding there.
Wolfgang Denk - Oct. 28, 2013, 1:41 p.m.
Dear Otavio,

In message <CAP9ODKq0Ac6azBOav7R54RncB9=mhtMZ+hydjY0UiYUSCS1O0A@mail.gmail.com> you wrote:
>
> > Simon's patch set "env: Add support for environment files" does
> > exactly that.
> 
> Doing it without changing source code, directly into the binary?

Yes.

> From what I read it allows import / export but this is at runtime, I
> need it /before/ booting the board. So currently I need to patch the
> environment files for it.

Please re-read it again.

> > Please be patient, like we all are.  It's better to wait a bit now,
> > then adding work now, and adding more work later to clean up the mess
> > again.
> 
> The work has already been done.

Yes, but that's actually your own problem; it seems you ignored the
previous discussion.

Best regards,

Wolfgang Denk
Stefano Babic - Oct. 28, 2013, 1:53 p.m.
Hi Otavio,

On 28/10/2013 14:29, Otavio Salvador wrote:
> On Mon, Oct 28, 2013 at 11:21 AM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Otavio,
>>
>> In message <CAP9ODKpN+PMX1HP+7bEMP7u-Hc44oNc6PEpAn+nbzbQm8cDfrg@mail.gmail.com> you wrote:
>>>
>>>> Indeed. However, why cannot we do the right thing soon ?
>>>
>>> There're no pending patches to address the root problem yet (allowing
>>> changing /internal and default/ environment).
>>
>> Simon's patch set "env: Add support for environment files" does
>> exactly that.
> 
> Doing it without changing source code, directly into the binary?
> 
> From what I read it allows import / export but this is at runtime, I
> need it /before/ booting the board. So currently I need to patch the
> environment files for it.

The way I see with Simon's patch is to define a .env file for each use
case we can have. Because the environment is outside the configuration
file, we could select the right environment when we produce the binary.

Let's say we have two environments, one defined from you and the other
as "debian" environment. We could select the desired environment at
build time by the make command or adding an entry into boards.cfg,
exactly as we do now to select the imximage file with IMX_CONFIG (same
sources, different imximage.cfg).

I understand that even with Simon's patches we will not have a way to
have separate u-boot nad environment binaries, and then merge them
together. However, we have a way to select which environment type
(distro, minimal, ..) at build time.

> 
>>> I will rework the environments as .env files but I don't want to hold
>>> this change until this is accepted.
>>
>> Please be patient, like we all are.  It's better to wait a bit now,
>> then adding work now, and adding more work later to clean up the mess
>> again.
> 
> The work has already been done.
> 
>>> My patch is fine and could go in now as is. Once Simon's patches get
>>> in I will post another series reworking the environment to convert it
>>> to the .env file format but please don't block on this.
>>
>> I agree with Stefano that we should not add more to the already
>> existing amount of env settings but instead wait for a cleaner way.
>> Please be patient - you lose nothing here.
> 
> I really see no point in holding it as the work has been done already;
> it is Stefano call but I disagree with postpone it as work is done and
> being in use.

Another example is maybe not in your "wandboard: add Future Eletronics
7" WVGA LCD extension board". In that patch, you add a lot of stuff
inside CONFIG_EXTRA_ENV, that is perfectly suitable for your needs, but
it is maybe not suitable for someone else who has not a display or maybe
another LCD. But again, it fits then perfectly if we define different
.env files, and it is possible to select one of them at build time.

It is then not exactly as we started, that is having two different
images (u-boot and environemnt) and finding a way to merge them
together, but it is quite close.

> 
> I will comment on Simon's patch to clarify my understanding there.
> 

Fine, I will follow the discussion.

Best regards,
Stefano Babic

Patch

diff --git a/include/configs/mx28evk.h b/include/configs/mx28evk.h
index 27b1a41..e40df09 100644
--- a/include/configs/mx28evk.h
+++ b/include/configs/mx28evk.h
@@ -163,7 +163,6 @@ 
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	"update_nand_full_filename=u-boot.nand\0" \
 	"update_nand_firmware_filename=u-boot.sb\0"	\
-	"update_sd_firmware_filename=u-boot.sd\0" \
 	"update_nand_firmware_maxsz=0x100000\0"	\
 	"update_nand_stride=0x40\0"	/* MX28 datasheet ch. 12.12 */ \
 	"update_nand_count=0x4\0"	/* MX28 datasheet ch. 12.12 */ \
@@ -191,6 +190,23 @@ 
 		"nand write ${loadaddr} ${fcb_sz} ${filesize} ; " \
 		"nand write ${loadaddr} ${fw_off} ${filesize} ; " \
 		"fi\0" \
+	"nandargs=setenv bootargs console=${console_mainline},${baudrate} " \
+		"rootfstype=ubifs ubi.mtd=6 root=ubi0_0 ${mtdparts}\0" \
+	"nandboot="		/* Boot from NAND */ \
+		"mtdparts default; " \
+		"run nandargs; " \
+		"nand read ${loadaddr} kernel 0x00400000; " \
+		"if test ${boot_fdt} = yes; then " \
+			"nand read ${fdt_addr} fdt 0x00080000; " \
+			"bootm ${loadaddr} - ${fdt_addr}; " \
+		"else " \
+			"if test ${boot_fdt} = no; then " \
+				"bootm; " \
+			"else " \
+				"echo \"ERROR: Set boot_fdt to yes or no.\"; " \
+			"fi; " \
+		"fi\0" \
+	"update_sd_firmware_filename=u-boot.sd\0" \
 	"update_sd_firmware="		/* Update the SD firmware partition */ \
 		"if mmc rescan ; then "	\
 		"if tftp ${update_sd_firmware_filename} ; then " \