All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
@ 2017-12-07 17:06 Evgeniy Didin
  2017-12-13  7:34 ` Thomas Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Evgeniy Didin @ 2017-12-07 17:06 UTC (permalink / raw)
  To: buildroot

Synopsys DesignWare HSDK (which stands for ARC HS
Development Kit) is the latest and greatest development
platform that sports quad-core ARC HS38 in real silicon.

Most noticeable features of the board are:
 * Quad-core ARC HS38 CPU running at 1GHz
 * 4Gb of DDR
 * Built-in Vivante GPU (well supported via open source
   Etnaviv drivers)
 * Built-in Wi-Fi/Bluetooth module (RedPine RS-9113)

And as usual we have:
 * [micro] SD-card slot
 * 2 USB 2.0 ports
 * 1Gbit Ethernet port
 * Built-in Digilent JTAG probe
 * Serial port accessible via micro-USB port

Writing sdcard.img on SDcard creates two partitions:
 * FAT32 with uImage and uboot.env
 * EXT4 with root filesystem

We modify kernel config because in default hsdk kernel config
CONFIG_INITRAMFS_SOURCE parameter is set and when we build
rootfs separately (BR2_TARGET_ROOTFS_INITRAMFS is not set)
error appears. Also we set up CONFIG_ARC_UBOOT_SUPPORT which
enables usage of uboot variables in the boot process.

Signed-off-by: Evgeniy Didin <didin@synopsys.com>
Cc: Alexey Brodkin <abrodkin@synopsys.com>
Cc: arc-buildroot at synopsys.com
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---
 board/synopsys/hsdk/genimage-hsdk.cfg | 26 +++++++++++++++++++++++++
 board/synopsys/hsdk/linux.fragment    |  2 ++
 board/synopsys/hsdk/post-image.sh     | 18 ++++++++++++++++++
 board/synopsys/hsdk/uboot.env.txt     |  9 +++++++++
 configs/snps_hsdk_defconfig           | 36 +++++++++++++++++++++++++++++++++++
 5 files changed, 91 insertions(+)
 create mode 100644 board/synopsys/hsdk/genimage-hsdk.cfg
 create mode 100644 board/synopsys/hsdk/linux.fragment
 create mode 100755 board/synopsys/hsdk/post-image.sh
 create mode 100644 board/synopsys/hsdk/uboot.env.txt
 create mode 100644 configs/snps_hsdk_defconfig

diff --git a/board/synopsys/hsdk/genimage-hsdk.cfg b/board/synopsys/hsdk/genimage-hsdk.cfg
new file mode 100644
index 0000000000..8928f704a2
--- /dev/null
+++ b/board/synopsys/hsdk/genimage-hsdk.cfg
@@ -0,0 +1,26 @@
+image boot.vfat {
+  vfat {
+    files = {
+      "uboot.env",
+      "uImage"
+    }
+  }
+  size = 100M
+}
+
+image sdcard.img {
+  hdimage {
+  }
+
+  partition boot {
+    partition-type = 0xC
+    bootable = "true"
+    image = "boot.vfat"
+  }
+
+  partition rootfs {
+    partition-type = 0x83
+    image = "rootfs.ext4"
+    size = 1G
+  }
+}
diff --git a/board/synopsys/hsdk/linux.fragment b/board/synopsys/hsdk/linux.fragment
new file mode 100644
index 0000000000..53b9305e09
--- /dev/null
+++ b/board/synopsys/hsdk/linux.fragment
@@ -0,0 +1,2 @@
+CONFIG_INITRAMFS_SOURCE=
+CONFIG_ARC_UBOOT_SUPPORT=y
diff --git a/board/synopsys/hsdk/post-image.sh b/board/synopsys/hsdk/post-image.sh
new file mode 100755
index 0000000000..5871a410a5
--- /dev/null
+++ b/board/synopsys/hsdk/post-image.sh
@@ -0,0 +1,18 @@
+#!/bin/bash
+
+BOARD_DIR="$(dirname $0)"
+BOARD_NAME="$(basename ${BOARD_DIR})"
+GENIMAGE_CFG="${BOARD_DIR}/genimage-${BOARD_NAME}.cfg"
+GENIMAGE_TMP="${BUILD_DIR}/genimage.tmp"
+rm -rf "${GENIMAGE_TMP}"
+
+mkenvimage -s 0x4000 -o output/images/uboot.env ${BOARD_DIR}/uboot.env.txt
+
+genimage                           \
+        --rootpath "${TARGET_DIR}"     \
+        --tmppath "${GENIMAGE_TMP}"    \
+        --inputpath "${BINARIES_DIR}"  \
+        --outputpath "${BINARIES_DIR}" \
+        --config "${GENIMAGE_CFG}"
+tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img
+exit $?
diff --git a/board/synopsys/hsdk/uboot.env.txt b/board/synopsys/hsdk/uboot.env.txt
new file mode 100644
index 0000000000..6bcfb56a77
--- /dev/null
+++ b/board/synopsys/hsdk/uboot.env.txt
@@ -0,0 +1,9 @@
+baudrate=115200
+bootargs=console=ttyS3,115200n8 root=/dev/mmcblk0p2 rootwait
+bootcmd=fatload mmc 0:1; bootm
+bootdelay=2
+bootfile=uImage
+loadaddr=0x82000000
+stderr=serial0 at f0005000
+stdin=serial0 at f0005000
+stdout=serial0 at f0005000
diff --git a/configs/snps_hsdk_defconfig b/configs/snps_hsdk_defconfig
new file mode 100644
index 0000000000..8c67b0ec0d
--- /dev/null
+++ b/configs/snps_hsdk_defconfig
@@ -0,0 +1,36 @@
+#Architecture
+BR2_arcle=y
+BR2_archs38=y
+
+#Linux headers
+BR2_KERNEL_HEADERS_4_14=y
+
+#System
+BR2_TARGET_GENERIC_HOSTNAME="hsdk"
+BR2_TARGET_GENERIC_ISSUE="Welcome to the HSDK Platform"
+BR2_SYSTEM_DHCP="eth0"
+
+#Kernel
+BR2_LINUX_KERNEL=y
+BR2_LINUX_KERNEL_CUSTOM_VERSION=y
+BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.14.4"
+BR2_LINUX_KERNEL_DEFCONFIG="hsdk"
+BR2_LINUX_KERNEL_CONFIG_FRAGMENT_FILES="board/synopsys/hsdk/linux.fragment"
+
+#Filesystem / image
+BR2_TARGET_ROOTFS_EXT2=y
+BR2_TARGET_ROOTFS_EXT2_4=y
+# BR2_TARGET_ROOTFS_TAR is not set
+BR2_PACKAGE_HOST_DOSFSTOOLS=y
+BR2_PACKAGE_HOST_GENIMAGE=y
+BR2_PACKAGE_HOST_MTOOLS=y
+BR2_ROOTFS_POST_IMAGE_SCRIPT="board/synopsys/hsdk/post-image.sh"
+
+#Bootloader
+BR2_TARGET_UBOOT=y
+BR2_TARGET_UBOOT_BUILD_SYSTEM_KCONFIG=y
+BR2_TARGET_UBOOT_CUSTOM_VERSION=y
+BR2_TARGET_UBOOT_CUSTOM_VERSION_VALUE="2017.11"
+BR2_TARGET_UBOOT_BOARD_DEFCONFIG="hsdk"
+BR2_TARGET_UBOOT_NEEDS_DTC=y
+BR2_TARGET_UBOOT_FORMAT_ELF=y
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-07 17:06 [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK) Evgeniy Didin
@ 2017-12-13  7:34 ` Thomas Petazzoni
  2017-12-13 18:04   ` Alexey Brodkin
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2017-12-13  7:34 UTC (permalink / raw)
  To: buildroot

Hello,

On Thu,  7 Dec 2017 20:06:28 +0300, Evgeniy Didin wrote:
> Synopsys DesignWare HSDK (which stands for ARC HS
> Development Kit) is the latest and greatest development
> platform that sports quad-core ARC HS38 in real silicon.
> 
> Most noticeable features of the board are:
>  * Quad-core ARC HS38 CPU running at 1GHz
>  * 4Gb of DDR
>  * Built-in Vivante GPU (well supported via open source
>    Etnaviv drivers)
>  * Built-in Wi-Fi/Bluetooth module (RedPine RS-9113)

Nice. Do you send free boards ? :-)

>  board/synopsys/hsdk/genimage-hsdk.cfg | 26 +++++++++++++++++++++++++

genimage.cfg is enough as a name I'd say.


> +mkenvimage -s 0x4000 -o output/images/uboot.env ${BOARD_DIR}/uboot.env.txt

This can be done using BR2_TARGET_UBOOT_ENVIMAGE.

> +
> +genimage                           \
> +        --rootpath "${TARGET_DIR}"     \
> +        --tmppath "${GENIMAGE_TMP}"    \
> +        --inputpath "${BINARIES_DIR}"  \
> +        --outputpath "${BINARIES_DIR}" \
> +        --config "${GENIMAGE_CFG}"

This should instead use the generic support/scripts/genimage.sh

> +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img

And this is not needed.

So basically, you can remove your post-image script, and point to
support/scripts/genimage.sh directly.

Finally, add an entry in the DEVELOPERS file, and update
the .gitlab-ci.yml file by running "make .gitlab-ci.yml" and committing
the result.

Other than that, looks good to me!

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-13  7:34 ` Thomas Petazzoni
@ 2017-12-13 18:04   ` Alexey Brodkin
  2017-12-14  6:08     ` Thomas Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Alexey Brodkin @ 2017-12-13 18:04 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

On Wed, 2017-12-13 at 08:34 +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Thu,  7 Dec 2017 20:06:28 +0300, Evgeniy Didin wrote:
> > Synopsys DesignWare HSDK (which stands for ARC HS
> > Development Kit) is the latest and greatest development
> > platform that sports quad-core ARC HS38 in real silicon.
> > 
> > Most noticeable features of the board are:
> >  * Quad-core ARC HS38 CPU running at 1GHz
> >  * 4Gb of DDR
> >  * Built-in Vivante GPU (well supported via open source
> >    Etnaviv drivers)
> >  * Built-in Wi-Fi/Bluetooth module (RedPine RS-9113)
> 
> Nice. Do you send free boards ? :-)

If you guys are interested I'll try to make sure you'll get one.
Seriously. But it won't happen before later next year anyways :)

> >  board/synopsys/hsdk/genimage-hsdk.cfg | 26 +++++++++++++++++++++++++
> 
> genimage.cfg is enough as a name I'd say.

Ok.

> > +mkenvimage -s 0x4000 -o output/images/uboot.env ${BOARD_DIR}/uboot.env.txt
> 
> This can be done using BR2_TARGET_UBOOT_ENVIMAGE.

There's one caveat though.
On that board we use SD-card as a storage for uImage, rootfs and among other things
U-Boot environment. But compared to some ARM boards we keep it on the first FAT partiotion,
not just put as a raw blob on SD-card before filesystem starts.

This is perfectly possible with up-to-date U-Boot - everything is already thee, just
say that environment will be on FAT partition like that:
----------------------------->8----------------------
CONFIG_ENV_IS_IN_FAT=y                                                                                                                            
CONFIG_ENV_FAT_INTERFACE="mmc"                                                                                                                    
CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
----------------------------->8----------------------

And by default U-Boot will look for a file with name "uboot.env", see
http://git.denx.de/?p=u-boot.git;a=blob;f=env/Kconfig#l394

But in Buildroot for some reason "uboot-env.bin" is generated.

So what we may do here:
 1) We may switch to "uboot-env.bin" in our U-Boot.
 2) We may keep using post build script... or
 3) We may switch to more common U-Boot-wise "uboot.env" with:
----------------------------->8----------------------
diff --git a/board/altera/socrates_cyclone5/genimage.cfg b/board/altera/socrates_cyclone5/genimage.cfg
index 63fef8ba4764..04d30490cb4d 100644
--- a/board/altera/socrates_cyclone5/genimage.cfg
+++ b/board/altera/socrates_cyclone5/genimage.cfg
@@ -35,7 +35,7 @@ image sdcard.img {
 
        partition uboot-env {
                in-partition-table = "no"
-               image = "uboot-env.bin"
+               image = "uboot.env"
                offset = 17408 # 512 * 34 -> just after gpt
        }
 
diff --git a/board/altera/socrates_cyclone5/readme.txt b/board/altera/socrates_cyclone5/readme.txt
index 4e78feef11a5..2e34c3d31f85 100644
--- a/board/altera/socrates_cyclone5/readme.txt
+++ b/board/altera/socrates_cyclone5/readme.txt
@@ -30,7 +30,7 @@ Following files will be generated in output/images
 ??? u-boot-spl.bin.crc
 ??? u-boot.bin
 ??? u-boot.img
-??? uboot-env.bin
+??? uboot.env
 ??? uboot.img
 ??? zImage
 
diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
index 2ae38e95d5af..d6fd3672aee0 100644
--- a/boot/uboot/Config.in
+++ b/boot/uboot/Config.in
@@ -378,7 +378,7 @@ menuconfig BR2_TARGET_UBOOT_ENVIMAGE
          Generate a valid binary environment image from a text file
          describing the key=value pairs of the environment.
 
-         The environment image will be called uboot-env.bin.
+         The environment image will be called uboot.env.
 
 if BR2_TARGET_UBOOT_ENVIMAGE
 
diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
index 14a8cc949a33..aaecf4d466cf 100644
--- a/boot/uboot/uboot.mk
+++ b/boot/uboot/uboot.mk
@@ -246,7 +246,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
                        $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
                        $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
                        $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
-                       -o $(BINARIES_DIR)/uboot-env.bin -)
+                       -o $(BINARIES_DIR)/uboot.env -)
        $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
                $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
                        -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
----------------------------->8----------------------

What do you think?

> > +
> > +genimage                           \
> > +        --rootpath "${TARGET_DIR}"     \
> > +        --tmppath "${GENIMAGE_TMP}"    \
> > +        --inputpath "${BINARIES_DIR}"  \
> > +        --outputpath "${BINARIES_DIR}" \
> > +        --config "${GENIMAGE_CFG}"
> 
> This should instead use the generic support/scripts/genimage.sh
> 
> > +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img
> 
> And this is not needed.

Well IMHO compression of the image makes sense because that way we may generate images
with larger FAT-partition without bumping size of the output image.

See we always allocate a bit more space for FAT partition (or it is applicable to any not-last
partiotion - not sure about that) compared to what we need just to make sure it's enough space for our
stuff. Moreover I'd like to have some more space so I may for example put 2 uImages side by side
and switch between them later on and do it all without re-formatting of the SD-card.

And given how large SD-cards these days are I'm fine with allocating even 100Mb for first FAT partiotion.
Now if we don't compress sdcard.img it will be of size 100+ Mb. But if we gzip it then we only have size
of real payload, i.e. ~6-7 Mb while keeping a lot of flexibility for image user.

Not to mention a case of sharing built images.

That said I'd rather prefer to either compress images by default (does it really hurt?)
or at least have it as an option.

> So basically, you can remove your post-image script, and point to
> support/scripts/genimage.sh directly.
> 
> Finally, add an entry in the DEVELOPERS file,

Is something like that OK?
------------------------->8----------------------------
diff --git a/DEVELOPERS b/DEVELOPERS
index 79e7202a8fcf..66b7a9e4d133 100644
--- a/DEVELOPERS
+++ b/DEVELOPERS
@@ -129,6 +129,7 @@ F:  package/wf111/
 
 N:     ARC Maintainers <arc-buildroot@synopsys.com>
 F:     arch/Config.in.arc
+F:     board/synopsys/
 
 N:     Ariel D'Alessandro <ariel@vanguardiasur.com.ar>
 F:     package/axfsutils/
------------------------->8----------------------------

> and update
> the .gitlab-ci.yml file by running "make .gitlab-ci.yml" and committing
> the result.

Sure, we'll do that.

-Alexey

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-13 18:04   ` Alexey Brodkin
@ 2017-12-14  6:08     ` Thomas Petazzoni
  2017-12-15  9:15       ` Alexey Brodkin
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2017-12-14  6:08 UTC (permalink / raw)
  To: buildroot

Hello,

On Wed, 13 Dec 2017 18:04:04 +0000, Alexey Brodkin wrote:

> > Nice. Do you send free boards ? :-)  
> 
> If you guys are interested I'll try to make sure you'll get one.
> Seriously. But it won't happen before later next year anyways :)

OK, let's see when the boards are available. I'm not sure I would spend
enough time playing with the board to justify having one.

> There's one caveat though.
> On that board we use SD-card as a storage for uImage, rootfs and among other things
> U-Boot environment. But compared to some ARM boards we keep it on the first FAT partiotion,
> not just put as a raw blob on SD-card before filesystem starts.
> 
> This is perfectly possible with up-to-date U-Boot - everything is already thee, just
> say that environment will be on FAT partition like that:
> ----------------------------->8----------------------  
> CONFIG_ENV_IS_IN_FAT=y                                                                                                                            
> CONFIG_ENV_FAT_INTERFACE="mmc"                                                                                                                    
> CONFIG_ENV_FAT_DEVICE_AND_PART="0:1"
> ----------------------------->8----------------------  
> 
> And by default U-Boot will look for a file with name "uboot.env", see
> http://git.denx.de/?p=u-boot.git;a=blob;f=env/Kconfig#l394
> 
> But in Buildroot for some reason "uboot-env.bin" is generated.

"some reason" = back then, there was no support in U-Boot for reading
the environment from a file, or at least we didn't know about this.

> So what we may do here:
>  1) We may switch to "uboot-env.bin" in our U-Boot.
>  2) We may keep using post build script... or
>  3) We may switch to more common U-Boot-wise "uboot.env" with:
> ----------------------------->8----------------------  
> diff --git a/board/altera/socrates_cyclone5/genimage.cfg b/board/altera/socrates_cyclone5/genimage.cfg
> index 63fef8ba4764..04d30490cb4d 100644
> --- a/board/altera/socrates_cyclone5/genimage.cfg
> +++ b/board/altera/socrates_cyclone5/genimage.cfg
> @@ -35,7 +35,7 @@ image sdcard.img {
>  
>         partition uboot-env {
>                 in-partition-table = "no"
> -               image = "uboot-env.bin"
> +               image = "uboot.env"
>                 offset = 17408 # 512 * 34 -> just after gpt
>         }
>  
> diff --git a/board/altera/socrates_cyclone5/readme.txt b/board/altera/socrates_cyclone5/readme.txt
> index 4e78feef11a5..2e34c3d31f85 100644
> --- a/board/altera/socrates_cyclone5/readme.txt
> +++ b/board/altera/socrates_cyclone5/readme.txt
> @@ -30,7 +30,7 @@ Following files will be generated in output/images
>  ??? u-boot-spl.bin.crc
>  ??? u-boot.bin
>  ??? u-boot.img
> -??? uboot-env.bin
> +??? uboot.env
>  ??? uboot.img
>  ??? zImage
>  
> diff --git a/boot/uboot/Config.in b/boot/uboot/Config.in
> index 2ae38e95d5af..d6fd3672aee0 100644
> --- a/boot/uboot/Config.in
> +++ b/boot/uboot/Config.in
> @@ -378,7 +378,7 @@ menuconfig BR2_TARGET_UBOOT_ENVIMAGE
>           Generate a valid binary environment image from a text file
>           describing the key=value pairs of the environment.
>  
> -         The environment image will be called uboot-env.bin.
> +         The environment image will be called uboot.env.
>  
>  if BR2_TARGET_UBOOT_ENVIMAGE
>  
> diff --git a/boot/uboot/uboot.mk b/boot/uboot/uboot.mk
> index 14a8cc949a33..aaecf4d466cf 100644
> --- a/boot/uboot/uboot.mk
> +++ b/boot/uboot/uboot.mk
> @@ -246,7 +246,7 @@ define UBOOT_INSTALL_IMAGES_CMDS
>                         $(HOST_DIR)/bin/mkenvimage -s $(BR2_TARGET_UBOOT_ENVIMAGE_SIZE) \
>                         $(if $(BR2_TARGET_UBOOT_ENVIMAGE_REDUNDANT),-r) \
>                         $(if $(filter BIG,$(BR2_ENDIAN)),-b) \
> -                       -o $(BINARIES_DIR)/uboot-env.bin -)
> +                       -o $(BINARIES_DIR)/uboot.env -)
>         $(if $(BR2_TARGET_UBOOT_BOOT_SCRIPT),
>                 $(HOST_DIR)/bin/mkimage -C none -A $(MKIMAGE_ARCH) -T script \
>                         -d $(call qstrip,$(BR2_TARGET_UBOOT_BOOT_SCRIPT_SOURCE)) \
> ----------------------------->8----------------------  
> 
> What do you think?

I think option (3) is the best. But it has a drawback: it breaks
backward compatibility for existing users that rely on the file being
named uboot-env.bin. So instead, you could add a new U-Boot option that
allows to customize the name of the generated image. By default it
would be uboot-env.bin to keep backward compatibility, but it could be
set to a different value in your defconfig.

Or we decide to break the backward compatibility :)
 
> > This should instead use the generic support/scripts/genimage.sh
> >   
> > > +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img  
> > 
> > And this is not needed.  
> 
> Well IMHO compression of the image makes sense because that way we may generate images
> with larger FAT-partition without bumping size of the output image.
> 
> See we always allocate a bit more space for FAT partition (or it is applicable to any not-last
> partiotion - not sure about that) compared to what we need just to make sure it's enough space for our
> stuff. Moreover I'd like to have some more space so I may for example put 2 uImages side by side
> and switch between them later on and do it all without re-formatting of the SD-card.
> 
> And given how large SD-cards these days are I'm fine with allocating even 100Mb for first FAT partiotion.
> Now if we don't compress sdcard.img it will be of size 100+ Mb. But if we gzip it then we only have size
> of real payload, i.e. ~6-7 Mb while keeping a lot of flexibility for image user.
> 
> Not to mention a case of sharing built images.
> 
> That said I'd rather prefer to either compress images by default (does it really hurt?)
> or at least have it as an option.

I'm not sure we want to solve this in your specific post-image script,
it's a more generic problem. Though I agree we don't have a good way of
solving it right now.

Also, creating a tarball for just one file is a bit weird, you could
just gzip sdcard.img to produce sdcard.img.gz.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-14  6:08     ` Thomas Petazzoni
@ 2017-12-15  9:15       ` Alexey Brodkin
  2017-12-18 11:06         ` Thomas Petazzoni
  0 siblings, 1 reply; 7+ messages in thread
From: Alexey Brodkin @ 2017-12-15  9:15 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

On Thu, 2017-12-14 at 07:08 +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 13 Dec 2017 18:04:04 +0000, Alexey Brodkin wrote:

[snip]

> > But in Buildroot for some reason "uboot-env.bin" is generated.
> 
> "some reason" = back then, there was no support in U-Boot for reading
> the environment from a file, or at least we didn't know about this.
> 
> > So what we may do here:
> >  1) We may switch to "uboot-env.bin" in our U-Boot.
> >  2) We may keep using post build script... or
> >  3) We may switch to more common U-Boot-wise "uboot.env" with:

[snip]

> I think option (3) is the best. But it has a drawback: it breaks
> backward compatibility for existing users that rely on the file being
> named uboot-env.bin. So instead, you could add a new U-Boot option that
> allows to customize the name of the generated image. By default it
> would be uboot-env.bin to keep backward compatibility, but it could be
> set to a different value in your defconfig.
> 
> Or we decide to break the backward compatibility :)

Well here again we're bumping to a situation that "something might be used
by BR users and we don't know what and how" :)

I mean in upstream Buildroot (as of today, i.e. without HSDK support) 
"uboot-env.bin" is only used for dumping binary data into SD-card image,
which essentially means file name makes no sense as long as genimage.cfg mentions
it correctly and as it might be seen from my patch proposal it is already handled
(proabably I should have left the patch above in place so you don't need to
look-up my previous email).

So our only concern might be people use "uboot-env.bin" on their own.

> > > This should instead use the generic support/scripts/genimage.sh
> > >   
> > > > +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img  
> > > 
> > > And this is not needed.  
> > 
> > Well IMHO compression of the image makes sense because that way we may generate images
> > with larger FAT-partition without bumping size of the output image.
> > 
> > See we always allocate a bit more space for FAT partition (or it is applicable to any not-last
> > partiotion - not sure about that) compared to what we need just to make sure it's enough space for our
> > stuff. Moreover I'd like to have some more space so I may for example put 2 uImages side by side
> > and switch between them later on and do it all without re-formatting of the SD-card.
> > 
> > And given how large SD-cards these days are I'm fine with allocating even 100Mb for first FAT partiotion.
> > Now if we don't compress sdcard.img it will be of size 100+ Mb. But if we gzip it then we only have size
> > of real payload, i.e. ~6-7 Mb while keeping a lot of flexibility for image user.
> > 
> > Not to mention a case of sharing built images.
> > 
> > That said I'd rather prefer to either compress images by default (does it really hurt?)
> > or at least have it as an option.
> 
> I'm not sure we want to solve this in your specific post-image script,
> it's a more generic problem. Though I agree we don't have a good way of
> solving it right now.
> 
> Also, creating a tarball for just one file is a bit weird, you could
> just gzip sdcard.img to produce sdcard.img.gz.

That's exactly what we do: we compress only resulting "sdcard.img" into "sdcard.img.tar.gz".
Please pardon myself if I expressed something not very clear :)

-Alexey

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-15  9:15       ` Alexey Brodkin
@ 2017-12-18 11:06         ` Thomas Petazzoni
  2017-12-18 13:05           ` Alexey Brodkin
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni @ 2017-12-18 11:06 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 15 Dec 2017 09:15:43 +0000, Alexey Brodkin wrote:

> > I think option (3) is the best. But it has a drawback: it breaks
> > backward compatibility for existing users that rely on the file being
> > named uboot-env.bin. So instead, you could add a new U-Boot option that
> > allows to customize the name of the generated image. By default it
> > would be uboot-env.bin to keep backward compatibility, but it could be
> > set to a different value in your defconfig.
> > 
> > Or we decide to break the backward compatibility :)  
> 
> Well here again we're bumping to a situation that "something might be used
> by BR users and we don't know what and how" :)
> 
> I mean in upstream Buildroot (as of today, i.e. without HSDK support) 
> "uboot-env.bin" is only used for dumping binary data into SD-card image,
> which essentially means file name makes no sense as long as genimage.cfg mentions
> it correctly and as it might be seen from my patch proposal it is already handled
> (proabably I should have left the patch above in place so you don't need to
> look-up my previous email).
> 
> So our only concern might be people use "uboot-env.bin" on their own.

I think we should simply make the output file name configurable, which
solves your use case, and preserves backward compatibility. This should
all be explained in the commit log.

> > > > This should instead use the generic support/scripts/genimage.sh
> > > >     
> > > > > +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img    
> > > > 
> > > > And this is not needed.    
> > > 
> > > Well IMHO compression of the image makes sense because that way we may generate images
> > > with larger FAT-partition without bumping size of the output image.
> > > 
> > > See we always allocate a bit more space for FAT partition (or it is applicable to any not-last
> > > partiotion - not sure about that) compared to what we need just to make sure it's enough space for our
> > > stuff. Moreover I'd like to have some more space so I may for example put 2 uImages side by side
> > > and switch between them later on and do it all without re-formatting of the SD-card.
> > > 
> > > And given how large SD-cards these days are I'm fine with allocating even 100Mb for first FAT partiotion.
> > > Now if we don't compress sdcard.img it will be of size 100+ Mb. But if we gzip it then we only have size
> > > of real payload, i.e. ~6-7 Mb while keeping a lot of flexibility for image user.
> > > 
> > > Not to mention a case of sharing built images.
> > > 
> > > That said I'd rather prefer to either compress images by default (does it really hurt?)
> > > or at least have it as an option.  
> > 
> > I'm not sure we want to solve this in your specific post-image script,
> > it's a more generic problem. Though I agree we don't have a good way of
> > solving it right now.
> > 
> > Also, creating a tarball for just one file is a bit weird, you could
> > just gzip sdcard.img to produce sdcard.img.gz.  
> 
> That's exactly what we do: we compress only resulting "sdcard.img" into "sdcard.img.tar.gz".

No, the code creates a tarball:

tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img

Which is useless when compressing a single file. Just compress the file
with gzip, no need to create a tarball.

But again, I recommend against that, as none of our defconfigs do this
currently. If you want to solve this problem, solve it separately,
otherwise your defconfig will wait for ages before being merged.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK)
  2017-12-18 11:06         ` Thomas Petazzoni
@ 2017-12-18 13:05           ` Alexey Brodkin
  0 siblings, 0 replies; 7+ messages in thread
From: Alexey Brodkin @ 2017-12-18 13:05 UTC (permalink / raw)
  To: buildroot

Hi Thomas,

On Mon, 2017-12-18 at 12:06 +0100, Thomas Petazzoni wrote:
> Hello,
> 
> On Fri, 15 Dec 2017 09:15:43 +0000, Alexey Brodkin wrote:
> 
> > > I think option (3) is the best. But it has a drawback: it breaks
> > > backward compatibility for existing users that rely on the file being
> > > named uboot-env.bin. So instead, you could add a new U-Boot option that
> > > allows to customize the name of the generated image. By default it
> > > would be uboot-env.bin to keep backward compatibility, but it could be
> > > set to a different value in your defconfig.
> > > 
> > > Or we decide to break the backward compatibility :)  
> > 
> > Well here again we're bumping to a situation that "something might be used
> > by BR users and we don't know what and how" :)
> > 
> > I mean in upstream Buildroot (as of today, i.e. without HSDK support) 
> > "uboot-env.bin" is only used for dumping binary data into SD-card image,
> > which essentially means file name makes no sense as long as genimage.cfg mentions
> > it correctly and as it might be seen from my patch proposal it is already handled
> > (proabably I should have left the patch above in place so you don't need to
> > look-up my previous email).
> > 
> > So our only concern might be people use "uboot-env.bin" on their own.
> 
> I think we should simply make the output file name configurable, which
> solves your use case, and preserves backward compatibility. This should
> all be explained in the commit log.

Right, we're already working on the patch to that effect.

> > > > > This should instead use the generic support/scripts/genimage.sh
> > > > >     
> > > > > > +tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img    
> > > > > 
> > > > > And this is not needed.    
> > > > 
> > > > Well IMHO compression of the image makes sense because that way we may generate images
> > > > with larger FAT-partition without bumping size of the output image.
> > > > 
> > > > See we always allocate a bit more space for FAT partition (or it is applicable to any not-last
> > > > partiotion - not sure about that) compared to what we need just to make sure it's enough space for our
> > > > stuff. Moreover I'd like to have some more space so I may for example put 2 uImages side by side
> > > > and switch between them later on and do it all without re-formatting of the SD-card.
> > > > 
> > > > And given how large SD-cards these days are I'm fine with allocating even 100Mb for first FAT partiotion.
> > > > Now if we don't compress sdcard.img it will be of size 100+ Mb. But if we gzip it then we only have size
> > > > of real payload, i.e. ~6-7 Mb while keeping a lot of flexibility for image user.
> > > > 
> > > > Not to mention a case of sharing built images.
> > > > 
> > > > That said I'd rather prefer to either compress images by default (does it really hurt?)
> > > > or at least have it as an option.  
> > > 
> > > I'm not sure we want to solve this in your specific post-image script,
> > > it's a more generic problem. Though I agree we don't have a good way of
> > > solving it right now.
> > > 
> > > Also, creating a tarball for just one file is a bit weird, you could
> > > just gzip sdcard.img to produce sdcard.img.gz.  
> > 
> > That's exactly what we do: we compress only resulting "sdcard.img" into "sdcard.img.tar.gz".
> 
> No, the code creates a tarball:
> 
> tar czf output/images/sdcard.img.tar.gz --directory="./output/images"  sdcard.img
> 
> Which is useless when compressing a single file. Just compress the file
> with gzip, no need to create a tarball.

I spotted that as well in Eugeniy's patch :)
Essentially there's no need for a tarball, plaingzip will be used.

> But again, I recommend against that, as none of our defconfigs do this
> currently.

As usual probably because people never had to share built images with others or
keep many of them. In comparison OpenWrt guys do it for all images just because
they build images for tons of boards and keep those images on FTPs.

> If you want to solve this problem, solve it separately,
> otherwise your defconfig will wait for ages before being merged.

Sure that makes perfect sense. But we'll keep working on that functionality so that
we'll be able to use it once corresponding patches are merged.

-Alexey

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-12-18 13:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-07 17:06 [Buildroot] [PATCH] board: add support for ARC HS Development Kit (HSDK) Evgeniy Didin
2017-12-13  7:34 ` Thomas Petazzoni
2017-12-13 18:04   ` Alexey Brodkin
2017-12-14  6:08     ` Thomas Petazzoni
2017-12-15  9:15       ` Alexey Brodkin
2017-12-18 11:06         ` Thomas Petazzoni
2017-12-18 13:05           ` Alexey Brodkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.