diff mbox

[U-Boot,1/3] config: Update envs for trats and trats2 - new entries for new partitions

Message ID 1389682946-29694-2-git-send-email-l.majewski@samsung.com
State Accepted
Delegated to: Tom Rini
Headers show

Commit Message

Łukasz Majewski Jan. 14, 2014, 7:02 a.m. UTC
This patch adds extra dfu_alt_info entries to support storing the whole BOOT
, DATA and UMS partitions.
This allows upgrade of uImage and device tree blob (dtb) files at once.

Now it is also possible to store ext4 rootfs prepared with well established
linux tools (like mkfs.ext4).

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

---
Changes for v2:
- New partions added
Change-Id: I97aaa95e2745736de417bd07dbe2433dfff0f02c
---
 include/configs/trats.h  |    5 ++++-
 include/configs/trats2.h |    5 ++++-
 2 files changed, 8 insertions(+), 2 deletions(-)

Comments

Minkyu Kang Jan. 14, 2014, 9:57 a.m. UTC | #1
On 14/01/14 16:02, Lukasz Majewski wrote:
> This patch adds extra dfu_alt_info entries to support storing the whole BOOT
> , DATA and UMS partitions.
> This allows upgrade of uImage and device tree blob (dtb) files at once.
> 
> Now it is also possible to store ext4 rootfs prepared with well established
> linux tools (like mkfs.ext4).
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> 
> ---
> Changes for v2:
> - New partions added
> Change-Id: I97aaa95e2745736de417bd07dbe2433dfff0f02c
> ---
>  include/configs/trats.h  |    5 ++++-
>  include/configs/trats2.h |    5 ++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 

Acked-by: Minkyu Kang <mk7.kang@samsung.com>

Thanks,
Minkyu Kang.
Gerhard Sittig Jan. 15, 2014, 4:12 p.m. UTC | #2
On Tue, Jan 14, 2014 at 08:02 +0100, Lukasz Majewski wrote:
> 
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index 6cd15c2..0bdfe86 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -143,7 +143,10 @@
>  	"u-boot mmc 80 400;" \
>  	"uImage ext4 0 2;" \
>  	"exynos4210-trats.dtb ext4 0 2;" \
> -	""PARTS_ROOT" part 0 5\0"
> +	""PARTS_BOOT" part 0 2;" \
> +	""PARTS_ROOT" part 0 5;" \
> +	""PARTS_DATA" part 0 6;" \
> +	""PARTS_UMS" part 0 7\0"

Have recent compilers become stricter about string concatenation?
ISTR that a space is recommended (which helps human readers as
well).


virtually yours
Gerhard Sittig
Lukasz Majewski Jan. 15, 2014, 8:32 p.m. UTC | #3
Hi Gerard,

> On Tue, Jan 14, 2014 at 08:02 +0100, Lukasz Majewski wrote:
> > 
> > diff --git a/include/configs/trats.h b/include/configs/trats.h
> > index 6cd15c2..0bdfe86 100644
> > --- a/include/configs/trats.h
> > +++ b/include/configs/trats.h
> > @@ -143,7 +143,10 @@
> >  	"u-boot mmc 80 400;" \
> >  	"uImage ext4 0 2;" \
> >  	"exynos4210-trats.dtb ext4 0 2;" \
> > -	""PARTS_ROOT" part 0 5\0"
> > +	""PARTS_BOOT" part 0 2;" \
> > +	""PARTS_ROOT" part 0 5;" \
> > +	""PARTS_DATA" part 0 6;" \
> > +	""PARTS_UMS" part 0 7\0"
> 
> Have recent compilers become stricter about string concatenation?

I'm using ELDK 5.3 toolchain and this code compiles without warnings.
Also I've tested it with Pengutronix's OSELAS 4.8.2 (OSEALS 2013.12).

> ISTR that a space is recommended (which helps human readers as
> well).

Could you be more specific about the missing space?

Also, since we close the release in a few days, I'd like to ask for
pulling this code as it is.

Those are envs related to DFU/THOR, so probably will be changed in a
near future.

> 
> 
> virtually yours
> Gerhard Sittig

Best regards,
Lukasz Majewski
Gerhard Sittig Jan. 16, 2014, 4:10 p.m. UTC | #4
On Wed, Jan 15, 2014 at 21:32 +0100, Lukasz Majewski wrote:
> 
> Hi Gerard,
> 
> > On Tue, Jan 14, 2014 at 08:02 +0100, Lukasz Majewski wrote:
> > > 
> > > diff --git a/include/configs/trats.h b/include/configs/trats.h
> > > index 6cd15c2..0bdfe86 100644
> > > --- a/include/configs/trats.h
> > > +++ b/include/configs/trats.h
> > > @@ -143,7 +143,10 @@
> > >  	"u-boot mmc 80 400;" \
> > >  	"uImage ext4 0 2;" \
> > >  	"exynos4210-trats.dtb ext4 0 2;" \
> > > -	""PARTS_ROOT" part 0 5\0"
> > > +	""PARTS_BOOT" part 0 2;" \
> > > +	""PARTS_ROOT" part 0 5;" \
> > > +	""PARTS_DATA" part 0 6;" \
> > > +	""PARTS_UMS" part 0 7\0"
> > 
> > Have recent compilers become stricter about string concatenation?
> 
> I'm using ELDK 5.3 toolchain and this code compiles without warnings.
> Also I've tested it with Pengutronix's OSELAS 4.8.2 (OSEALS 2013.12).

With "recent" I meant "so recent that they are not yet in
widespread use".  Can't even remember whether it was a specific
version of a compiler, or a pending C language standard that was
said to become stricter, my memory is failing on me.  Is there
someone around with better memorization capabilities? :)

> > ISTR that a space is recommended (which helps human readers as
> > well).
> 
> Could you be more specific about the missing space?

What I meant is quite simple.  Put whitespace between the strings
that you want to have concatenated.  Instead of

  ""PARTS_BOOT" part 0 5\0"

write something "more portable" (and more readable?) like

  "" PARTS_BOOT " part 0 5\0"

which should not be too much of an issue.  It's not that the
semantics of string concatenation would change, it's just that
syntax is expected to become stricter, and code should not
(ab)use the gray areas that one currently may get away with.

> Also, since we close the release in a few days, I'd like to ask for
> pulling this code as it is.

No problem there, I was just trying to raise or keep awareness.
Just queue another editor session and follow up with a cleanup
later.  Even better if you will touch the code anyway.


virtually yours
Gerhard Sittig
Wolfgang Denk Jan. 16, 2014, 10:12 p.m. UTC | #5
Dear Gerhard Sittig,

In message <20140116161026.GX20094@book.gsilab.sittig.org> you wrote:
>
> write something "more portable" (and more readable?) like
> 
>   "" PARTS_BOOT " part 0 5\0"

What would the "" be needed for anyway?

Best regards,

Wolfgang Denk
Tom Rini Jan. 17, 2014, 1:05 p.m. UTC | #6
On Tue, Jan 14, 2014 at 08:02:24AM +0100, Łukasz Majewski wrote:

> This patch adds extra dfu_alt_info entries to support storing the whole BOOT
> , DATA and UMS partitions.
> This allows upgrade of uImage and device tree blob (dtb) files at once.
> 
> Now it is also possible to store ext4 rootfs prepared with well established
> linux tools (like mkfs.ext4).
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Acked-by: Minkyu Kang <mk7.kang@samsung.com>

Applied to u-boot/master, thanks!
Gerhard Sittig Jan. 17, 2014, 2:11 p.m. UTC | #7
On Thu, Jan 16, 2014 at 23:12 +0100, Wolfgang Denk wrote:
> 
> Dear Gerhard Sittig,
> 
> In message <20140116161026.GX20094@book.gsilab.sittig.org> you wrote:
> >
> > write something "more portable" (and more readable?) like
> > 
> >   "" PARTS_BOOT " part 0 5\0"
> 
> What would the "" be needed for anyway?

It wouldn't.  It just was there before, and could remain there
since it "does nothing" (concatenates an empty string, like
adding zero, or multiplying by one -- sometimes it's nice for a
uniform layout, sometimes it's more confusing than helping).
Removing this neutral element is an option, yet not strictly
necessary.  I was more concerned about potential build breakage
than (function wise neutral) style issues.


virtually yours
Gerhard Sittig
diff mbox

Patch

diff --git a/include/configs/trats.h b/include/configs/trats.h
index 6cd15c2..0bdfe86 100644
--- a/include/configs/trats.h
+++ b/include/configs/trats.h
@@ -143,7 +143,10 @@ 
 	"u-boot mmc 80 400;" \
 	"uImage ext4 0 2;" \
 	"exynos4210-trats.dtb ext4 0 2;" \
-	""PARTS_ROOT" part 0 5\0"
+	""PARTS_BOOT" part 0 2;" \
+	""PARTS_ROOT" part 0 5;" \
+	""PARTS_DATA" part 0 6;" \
+	""PARTS_UMS" part 0 7\0"
 
 #define CONFIG_ENV_OVERWRITE
 #define CONFIG_SYS_CONSOLE_INFO_QUIET
diff --git a/include/configs/trats2.h b/include/configs/trats2.h
index c9ce828..f335280 100644
--- a/include/configs/trats2.h
+++ b/include/configs/trats2.h
@@ -173,7 +173,10 @@ 
 	"u-boot mmc 80 800;" \
 	"uImage ext4 0 2;" \
 	"exynos4412-trats2.dtb ext4 0 2;" \
-	""PARTS_ROOT" part 0 5\0"
+	""PARTS_BOOT" part 0 2;" \
+	""PARTS_ROOT" part 0 5;" \
+	""PARTS_DATA" part 0 6;" \
+	""PARTS_UMS" part 0 7\0"
 
 #define CONFIG_EXTRA_ENV_SETTINGS \
 	"bootk=" \