Re: [PATCH] habv4: add the possibility to changing the signing area from Kconfig

2019-12-10 Thread Maik Otto
Hi Sascha,

in my opinion it is better to have it configurable, because ther are
different use cases and security requirements.
i found the problem by creating  a sd-card \emmc image with wic.  The
mbr, the partition table and bootloader became be signed at barebox
build and wic changes
the partition table at the end of the build process. Then the sd card
image could not boot , because the signature was wrong. yeah secure boot
works :-)
the highest protection you have, when mbr and partition table is signed
with the bootloader, but it is not always necessary.

there was implemented skip-mbr, dcdofs and full, but full was by default
in the code. at the moment i think , it is a good and easy choice.

Best regards

Maik

Am 10.12.2019 um 16:21 schrieb Sascha Hauer:
> On Tue, Dec 10, 2019 at 04:06:27PM +0100, Maik Otto wrote:
>> the whole barebox with mbr and partition table will be signed by default
>> add the possibility in the Kconfig to change from full signing to skip-mbr
>> and from-dcdofs
> Not signing the MBR seems the right thing to do. Do we need it
> configurable at all or would it be better to just always skip the first
> KiB?
>
> Sascha
>


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 2/4] ARM: zynq: zedboard: allow lowlevel init to be called as second stage

2019-12-10 Thread Lucas Stach
If the code is already executing in DRAM, the PS7 init must not be
executed, as it initializes the DRAM controller.

As the OCM can be configured to an address which aliases with the DRAM
address space we can't reliably infer if we are running from OCM or
DRAM from the execution address. So instead of using the address, look
at the OCM mapping, as the BootROM leaves a quite unique mapping behind
with 192KB OCM mapped at the low address and 64KB mapped to the high
address.

Signed-off-by: Lucas Stach 
---
 arch/arm/boards/avnet-zedboard/lowlevel.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boards/avnet-zedboard/lowlevel.c 
b/arch/arm/boards/avnet-zedboard/lowlevel.c
index b50c36b28869..9b90ef112b46 100644
--- a/arch/arm/boards/avnet-zedboard/lowlevel.c
+++ b/arch/arm/boards/avnet-zedboard/lowlevel.c
@@ -31,6 +31,16 @@ extern char __dtb_zynq_zed_start[];
 
 static void avnet_zedboard_ps7_init(void)
 {
+   /*
+* Read OCM mapping configuration, if only the upper 64 KByte are
+* mapped to the high address, it's very likely that we just got control
+* from the BootROM. If the mapping is changed something other than the
+* BootROM was running before us. Skip PS7 init to avoid cutting the
+* branch we are sitting on in that case.
+*/
+   if ((readl(0xf8000910) & 0xf) != 0x8)
+   return;
+
/* open sesame */
writel(0xDF0D, ZYNQ_SLCR_UNLOCK);
 
-- 
2.23.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 1/4] ARM: zynq: zedboard: split out PS7 init

2019-12-10 Thread Lucas Stach
Move the PS7 inititalization into its own function. This helps
readability and logically splits the FPGA toolchain generated
setup from the reset of the board init.

Also execute the PS7 setup after the lowlevel CPU init, as this
is the regular order used in the Barebox codebase.

Signed-off-by: Lucas Stach 
---
 arch/arm/boards/avnet-zedboard/lowlevel.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boards/avnet-zedboard/lowlevel.c 
b/arch/arm/boards/avnet-zedboard/lowlevel.c
index 93e4da96ecd4..b50c36b28869 100644
--- a/arch/arm/boards/avnet-zedboard/lowlevel.c
+++ b/arch/arm/boards/avnet-zedboard/lowlevel.c
@@ -29,11 +29,8 @@
 
 extern char __dtb_zynq_zed_start[];
 
-ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2)
+static void avnet_zedboard_ps7_init(void)
 {
-
-   void *fdt = __dtb_zynq_zed_start + get_runtime_offset();
-
/* open sesame */
writel(0xDF0D, ZYNQ_SLCR_UNLOCK);
 
@@ -260,8 +257,16 @@ ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2)
 
/* lock up. secure, secure */
writel(0x767B, ZYNQ_SLCR_LOCK);
+}
+
+ENTRY_FUNCTION(start_avnet_zedboard, r0, r1, r2)
+{
+
+   void *fdt = __dtb_zynq_zed_start + get_runtime_offset();
 
arm_cpu_lowlevel_init();
 
+   avnet_zedboard_ps7_init();
+
barebox_arm_entry(0, SZ_512M, fdt);
 }
-- 
2.23.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 3/4] filetype: add Zynq image type

2019-12-10 Thread Lucas Stach
Use the 2 invariant words (width detection and image identification)
from the Zynq image header to detect the filetype.

Signed-off-by: Lucas Stach 
---
 common/filetype.c  | 4 
 include/filetype.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/common/filetype.c b/common/filetype.c
index 39fea45edff5..fd6e8e3d1c35 100644
--- a/common/filetype.c
+++ b/common/filetype.c
@@ -80,6 +80,7 @@ static const struct filetype_str filetype_str[] = {
[filetype_ubootvar] = { "U-Boot environmemnt variable data",
"ubootvar" },
[filetype_stm32_image_v1] = { "STM32 image (v1)", "stm32-image-v1" },
+   [filetype_zynq_image] = { "Zynq image", "zynq-image" },
 };
 
 const char *file_type_to_string(enum filetype f)
@@ -392,6 +393,9 @@ enum filetype file_detect_type(const void *_buf, size_t 
bufsize)
if (is_imx_flash_header_v2(_buf))
return filetype_imx_image_v2;
 
+   if (buf[8] == 0xAA995566 && buf[9] == 0x584C4E58)
+   return filetype_zynq_image;
+
return filetype_unknown;
 }
 
diff --git a/include/filetype.h b/include/filetype.h
index 90a03de58105..db95fdaa0a5f 100644
--- a/include/filetype.h
+++ b/include/filetype.h
@@ -49,6 +49,7 @@ enum filetype {
filetype_layerscape_qspi_image,
filetype_ubootvar,
filetype_stm32_image_v1,
+   filetype_zynq_image,
filetype_max,
 };
 
-- 
2.23.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 4/4] ARM: zynq: add Zynq image bootm handler

2019-12-10 Thread Lucas Stach
This adds a bootm handler for the Zynq image, to allow second
stage booting of a unchanged Zynq boot image.

Signed-off-by: Lucas Stach 
---
 arch/arm/mach-zynq/Makefile|  2 +-
 arch/arm/mach-zynq/bootm-zynqimg.c | 49 ++
 2 files changed, 50 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/mach-zynq/bootm-zynqimg.c

diff --git a/arch/arm/mach-zynq/Makefile b/arch/arm/mach-zynq/Makefile
index 3252247d176d..c88ab4666f0f 100644
--- a/arch/arm/mach-zynq/Makefile
+++ b/arch/arm/mach-zynq/Makefile
@@ -1 +1 @@
-obj-y += zynq.o
+obj-y += zynq.o bootm-zynqimg.o
diff --git a/arch/arm/mach-zynq/bootm-zynqimg.c 
b/arch/arm/mach-zynq/bootm-zynqimg.c
new file mode 100644
index ..e903ab667905
--- /dev/null
+++ b/arch/arm/mach-zynq/bootm-zynqimg.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include 
+#include 
+#include 
+#include 
+
+static int do_bootm_zynqimage(struct image_data *data)
+{
+   resource_size_t start, end;
+   void (*barebox)(void);
+   u32 *header;
+   int ret;
+
+   ret = memory_bank_first_find_space(, );
+   if (ret)
+   return ret;
+
+   ret = bootm_load_os(data, start);
+   if (ret)
+   return ret;
+
+   header = (u32*)start;
+   barebox = (void*)start + header[12];
+
+   if (data->verbose)
+   printf("Loaded barebox image to 0x%08lx\n",
+  (unsigned long)barebox);
+
+   shutdown_barebox();
+
+   barebox();
+
+   return -EIO;
+}
+
+static struct image_handler zynq_image_handler = {
+   .name = "Zynq image",
+   .bootm = do_bootm_zynqimage,
+   .filetype = filetype_zynq_image,
+};
+
+static int zynq_register_image_handler(void)
+{
+   register_image_handler(_image_handler);
+
+   return 0;
+}
+late_initcall(zynq_register_image_handler);
-- 
2.23.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH] of: demote most debug output to loglevel 8 (vdebug)

2019-12-10 Thread Roland Hieber
Loglevel 7 (debug) is often useful to get barebox running on new
hardware; however, the heavy output of "OF:" messages makes most
boards take several minutes to boot to a prompt, printing things which
are probably interesting when debugging OF code itself, but not very
helpful otherwise. Degrade most of the message to loglevel 8 (vdebug).

Signed-off-by: Roland Hieber 

---
For illustration, here is the output before this patch with level 7 for
a single OF node:

of_platform_device_create: register device 44e0.target-mod...@0.of, 
io=0x44e0
register_device: 44e0.target-mod...@0.of
   create child: 
/ocp/interconnect@44c0/segment@20/target-module@3000
   create child: 
/ocp/interconnect@44c0/segment@20/target-module@5000
   create child: 
/ocp/interconnect@44c0/segment@20/target-module@7000
OF: ** translation for device 
/ocp/interconnect@44c0/segment@20/target-module@7000 **
OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20
OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0
OF: walking ranges...
OF: default map, cp=0, s=2000, da=7000
OF: default map, cp=2000, s=1000, da=7000
OF: default map, cp=3000, s=1000, da=7000
OF: default map, cp=4000, s=1000, da=7000
OF: default map, cp=5000, s=1000, da=7000
OF: default map, cp=6000, s=1000, da=7000
OF: default map, cp=7000, s=1000, da=7000
OF: with offset: 0
OF: parent bus is default (na=1, ns=1) on /ocp
OF: walking ranges...
OF: default map, cp=0, s=10, da=207000
OF: default map, cp=10, s=10, da=207000
OF: default map, cp=20, s=10, da=207000
OF: with offset: 7000
OF: parent bus is default (na=1, ns=1) on
OF: empty ranges; 1:1 translation
OF: with offset: 44e07000
OF: reached root node
OF: ** translation for device 
/ocp/interconnect@44c0/segment@20/target-module@7000 **
OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20
OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0
OF: walking ranges...
OF: default map, cp=0, s=2000, da=7010
OF: default map, cp=2000, s=1000, da=7010
OF: default map, cp=3000, s=1000, da=7010
OF: default map, cp=4000, s=1000, da=7010
OF: default map, cp=5000, s=1000, da=7010
OF: default map, cp=6000, s=1000, da=7010
OF: default map, cp=7000, s=1000, da=7010
OF: with offset: 10
OF: parent bus is default (na=1, ns=1) on /ocp
OF: walking ranges...
OF: default map, cp=0, s=10, da=207010
OF: default map, cp=10, s=10, da=207010
OF: default map, cp=20, s=10, da=207010
OF: with offset: 7010
OF: parent bus is default (na=1, ns=1) on
OF: empty ranges; 1:1 translation
OF: with offset: 44e07010
OF: reached root node
OF: ** translation for device 
/ocp/interconnect@44c0/segment@20/target-module@7000 **
OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20
OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0
OF: walking ranges...
OF: default map, cp=0, s=2000, da=7114
OF: default map, cp=2000, s=1000, da=7114
OF: default map, cp=3000, s=1000, da=7114
OF: default map, cp=4000, s=1000, da=7114
OF: default map, cp=5000, s=1000, da=7114
OF: default map, cp=6000, s=1000, da=7114
OF: default map, cp=7000, s=1000, da=7114
OF: with offset: 114
OF: parent bus is default (na=1, ns=1) on /ocp
OF: walking ranges...
OF: default map, cp=0, s=10, da=207114
OF: default map, cp=10, s=10, da=207114
OF: default map, cp=20, s=10, da=207114
OF: with offset: 7114
OF: parent bus is default (na=1, ns=1) on
OF: empty ranges; 1:1 translation
OF: with offset: 44e07114
OF: reached root node
OF: ** translation for device 
/ocp/interconnect@44c0/segment@20/target-module@7000 **
OF: bus is default (na=1, ns=1) on /ocp/interconnect@44c0/segment@20
OF: parent bus is default (na=1, ns=1) on /ocp/interconnect@44c0
OF: walking ranges...
OF: default map, cp=0, s=2000, da=7000
OF: default map, cp=2000, s=1000, da=7000
OF: default map, cp=3000, s=1000, da=7000
OF: default map, cp=4000, s=1000, da=7000
OF: default map, cp=5000, s=1000, da=7000
OF: default map, cp=6000, s=1000, da=7000
OF: default map, cp=7000, s=1000, da=7000
OF: with offset: 0
OF: parent bus is default (na=1, ns=1) on /ocp
OF: walking ranges...
OF: default map, cp=0, s=10, da=207000
OF: default map, cp=10, s=10, da=207000
OF: default map, cp=20, s=10, da=207000
OF: with offset: 7000
OF: parent bus is default (na=1, ns=1) on
OF: empty ranges; 1:1 translation
OF: with offset: 44e07000
OF: reached root node
OF: ** translation for device 

Re: [BUG] imx6qdl: degraded eMMC write performance

2019-12-10 Thread Hubert Feurstein
Am Di., 10. Dez. 2019 um 17:18 Uhr schrieb Sascha Hauer
:
[...]
> > > We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full
> > > chunk size (16KiB). Does this give you back some lost performance?
> > No, changing RW_BUF_SIZE did not help.
>
> And I also see why :-/
>
> block_op_write() works around block sizes (512bytes), not around chunk
> sizes. This means we always read before we write. This could probably be
> optimized somehow, but this would only speed up the write case you
> described in your initial mail. It seems what you are more interested in
> is the read performance.
Well, I'm interested in both. Of course the read-performance is important
for booting. But the write-performance is important for production. It simply
matters if you have to wait for another 200 seconds for a device to
get programmed.

Regards
Hubert

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH] commands: digest: don't be to restrictive with the length of verification files

2019-12-10 Thread Hubert Feurstein
Otherwise it is not possible to verify against a file created by shaXsum
from linux. Usually there is the filename appended after the hash.

Signed-off-by: Hubert Feurstein 
---
 commands/digest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commands/digest.c b/commands/digest.c
index 99b27dcc2..21a091717 100644
--- a/commands/digest.c
+++ b/commands/digest.c
@@ -164,7 +164,7 @@ static int do_digest(int argc, char *argv[])
 
if (sig) {
digestlen = digest_length(d);
-   if (siglen == 2 * digestlen) {
+   if (siglen >= 2 * digestlen) {
if (!tmp_sig)
tmp_sig = xmalloc(digestlen);
 
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 1/4] led: unify led trigger tables

2019-12-10 Thread Hubert Feurstein
Currently we have two slightly different led-trigger tables. One in the
core module, and the other one in the led-triggers module. The one in the
core module, which is used to parse the device-tree triggers, is lacking
net-rx and net-tx.

Signed-off-by: Hubert Feurstein 
---
 drivers/led/core.c | 46 +++---
 drivers/led/led-triggers.c | 25 -
 2 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/drivers/led/core.c b/drivers/led/core.c
index 431966d06..fec7324e7 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -256,22 +256,35 @@ void led_unregister(struct led *led)
list_del(>list);
 }
 
-struct led_trg {
-   const char *str;
-   enum led_trigger trg;
+static char *trigger_names[] = {
+   [LED_TRIGGER_PANIC] = "panic",
+   [LED_TRIGGER_HEARTBEAT] = "heartbeat",
+   [LED_TRIGGER_NET_RX] = "net-rx",
+   [LED_TRIGGER_NET_TX] = "net-tx",
+   [LED_TRIGGER_NET_TXRX] = "net",
+   [LED_TRIGGER_DEFAULT_ON] = "default-on",
 };
 
-static struct led_trg triggers[] = {
-   { .str = "heartbeat", LED_TRIGGER_HEARTBEAT, },
-   { .str = "panic", LED_TRIGGER_PANIC, },
-   { .str = "net", LED_TRIGGER_NET_TXRX, },
-   { .str = "default-on", LED_TRIGGER_DEFAULT_ON, },
-};
+const char *trigger_name(enum led_trigger trigger)
+{
+   return trigger_names[trigger];
+}
+
+enum led_trigger trigger_by_name(const char *name)
+{
+   int i;
+
+   for (i = 0; i < LED_TRIGGER_MAX; i++)
+   if (!strcmp(name, trigger_names[i]))
+   return i;
+
+   return LED_TRIGGER_MAX;
+}
 
 void led_of_parse_trigger(struct led *led, struct device_node *np)
 {
+   enum led_trigger trg;
const char *trigger;
-   int i;
 
trigger = of_get_property(np, "linux,default-trigger", NULL);
if (!trigger)
@@ -280,13 +293,10 @@ void led_of_parse_trigger(struct led *led, struct 
device_node *np)
if (!trigger)
return;
 
-   for (i = 0; i < ARRAY_SIZE(triggers); i++) {
-   struct led_trg *trg = [i];
-   if (!strcmp(trg->str, trigger)) {
-   /* disable LED before installing trigger */
-   led_set(led, 0);
-   led_set_trigger(trg->trg, led);
-   return;
-   }
+   trg = trigger_by_name(trigger);
+   if (trg != LED_TRIGGER_MAX) {
+   /* disable LED before installing trigger */
+   led_set(led, 0);
+   led_set_trigger(trg, led);
}
 }
diff --git a/drivers/led/led-triggers.c b/drivers/led/led-triggers.c
index 76a1481e1..216c8639b 100644
--- a/drivers/led/led-triggers.c
+++ b/drivers/led/led-triggers.c
@@ -143,31 +143,6 @@ int led_set_trigger(enum led_trigger trigger, struct led 
*led)
return 0;
 }
 
-static char *trigger_names[] = {
-   [LED_TRIGGER_PANIC] = "panic",
-   [LED_TRIGGER_HEARTBEAT] = "heartbeat",
-   [LED_TRIGGER_NET_RX] = "net-rx",
-   [LED_TRIGGER_NET_TX] = "net-tx",
-   [LED_TRIGGER_NET_TXRX] = "net",
-   [LED_TRIGGER_DEFAULT_ON] = "default-on",
-};
-
-const char *trigger_name(enum led_trigger trigger)
-{
-   return trigger_names[trigger];
-}
-
-enum led_trigger trigger_by_name(const char *name)
-{
-   int i;
-
-   for (i = 0; i < LED_TRIGGER_MAX; i++)
-   if (!strcmp(name, trigger_names[i]))
-   return i;
-
-   return LED_TRIGGER_MAX;
-}
-
 /**
  * led_triggers_show_info - Show information about all registered
  * triggers
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 4/4] led: parse panic-indicator from device-tree

2019-12-10 Thread Hubert Feurstein
Signed-off-by: Hubert Feurstein 
---
 Documentation/devicetree/bindings/leds/common.rst |  3 +++
 drivers/led/core.c| 11 ---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/common.rst 
b/Documentation/devicetree/bindings/leds/common.rst
index 5a592d67d..911a55f4f 100644
--- a/Documentation/devicetree/bindings/leds/common.rst
+++ b/Documentation/devicetree/bindings/leds/common.rst
@@ -12,3 +12,6 @@ Common leds properties
 
 * ``label``: The label for this LED. If omitted, the label is taken
   from the node name (excluding the unit address).
+
+* ``panic-indicator`` - This property specifies that the LED should be used as 
a
+ panic indicator.
diff --git a/drivers/led/core.c b/drivers/led/core.c
index 4bf19abcc..e727148a2 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -286,11 +286,16 @@ enum led_trigger trigger_by_name(const char *name)
 
 void led_of_parse_trigger(struct led *led, struct device_node *np)
 {
-   enum led_trigger trg;
+   enum led_trigger trg = LED_TRIGGER_MAX;
const char *trigger;
 
-   trigger = of_get_property(np, "linux,default-trigger", NULL);
-   trg = trigger_by_name(trigger);
+   if (of_property_read_bool(np, "panic-indicator"))
+   trg = LED_TRIGGER_PANIC;
+
+   if (trg == LED_TRIGGER_MAX) {
+   trigger = of_get_property(np, "linux,default-trigger", NULL);
+   trg = trigger_by_name(trigger);
+   }
 
if (trg == LED_TRIGGER_MAX) {
trigger = of_get_property(np, "barebox,default-trigger", NULL);
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 2/4] led: add documentation for net-tx and net-rx triggers

2019-12-10 Thread Hubert Feurstein
Signed-off-by: Hubert Feurstein 
---
 Documentation/devicetree/bindings/leds/common.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/leds/common.rst 
b/Documentation/devicetree/bindings/leds/common.rst
index 1f5ae42ca..5a592d67d 100644
--- a/Documentation/devicetree/bindings/leds/common.rst
+++ b/Documentation/devicetree/bindings/leds/common.rst
@@ -6,7 +6,9 @@ Common leds properties
 
 * ``heartbeat`` - LED flashes at a constant rate
 * ``panic`` - LED turns on when barebox panics
-* ``net`` - LED indicates network activity
+* ``net`` - LED indicates network activity (tx and rx)
+* ``net-rx`` - LED indicates network activity (rx only)
+* ``net-tx`` - LED indicates network activity (tx only)
 
 * ``label``: The label for this LED. If omitted, the label is taken
   from the node name (excluding the unit address).
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 3/4] led: check for 'barebox, default-trigger' when 'linux, default-trigger' is not found

2019-12-10 Thread Hubert Feurstein
When the linux,default-trigger is not found by barebox, then also check if
there might be a barebox,default-trigger.

Signed-off-by: Hubert Feurstein 
---
 drivers/led/core.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/led/core.c b/drivers/led/core.c
index fec7324e7..4bf19abcc 100644
--- a/drivers/led/core.c
+++ b/drivers/led/core.c
@@ -274,6 +274,9 @@ enum led_trigger trigger_by_name(const char *name)
 {
int i;
 
+   if (!name)
+   return LED_TRIGGER_MAX;
+
for (i = 0; i < LED_TRIGGER_MAX; i++)
if (!strcmp(name, trigger_names[i]))
return i;
@@ -287,13 +290,13 @@ void led_of_parse_trigger(struct led *led, struct 
device_node *np)
const char *trigger;
 
trigger = of_get_property(np, "linux,default-trigger", NULL);
-   if (!trigger)
-   trigger = of_get_property(np, "barebox,default-trigger", NULL);
+   trg = trigger_by_name(trigger);
 
-   if (!trigger)
-   return;
+   if (trg == LED_TRIGGER_MAX) {
+   trigger = of_get_property(np, "barebox,default-trigger", NULL);
+   trg = trigger_by_name(trigger);
+   }
 
-   trg = trigger_by_name(trigger);
if (trg != LED_TRIGGER_MAX) {
/* disable LED before installing trigger */
led_set(led, 0);
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [BUG] imx6qdl: degraded eMMC write performance

2019-12-10 Thread Sascha Hauer
On Tue, Dec 10, 2019 at 04:43:54PM +0100, Hubert Feurstein wrote:
> Hi Sascha,
> 
> Am Di., 10. Dez. 2019 um 16:10 Uhr schrieb Sascha Hauer
> :
> [...]
> > Well the commit message says it:
> > | commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43
> > | Author: Sascha Hauer 
> > | Date:   Thu Mar 29 13:49:45 2018 +0200
> > |
> > | block: Adjust cache sizes
> > |
> > | Use four times more cache entries and divide the memory for each entry
> > | by four. This lowers the linear read throughput somewhat but increases
> > | the access speed for filesystems.
> Yes, I know. I've read the commit message. But this patch even costs me ~200ms
> in boottime (loading kernel and dts from ext4 root partition).
> 
> [...]
> > We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full
> > chunk size (16KiB). Does this give you back some lost performance?
> No, changing RW_BUF_SIZE did not help.

And I also see why :-/

block_op_write() works around block sizes (512bytes), not around chunk
sizes. This means we always read before we write. This could probably be
optimized somehow, but this would only speed up the write case you
described in your initial mail. It seems what you are more interested in
is the read performance.

We could make the number of chunks and the chunk size configurable
during runtime. This is less than ideal, but at least we could get a
better performance with manageable effort.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [BUG] imx6qdl: degraded eMMC write performance

2019-12-10 Thread Hubert Feurstein
Hi Sascha,

Am Di., 10. Dez. 2019 um 16:10 Uhr schrieb Sascha Hauer
:
[...]
> Well the commit message says it:
> | commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43
> | Author: Sascha Hauer 
> | Date:   Thu Mar 29 13:49:45 2018 +0200
> |
> | block: Adjust cache sizes
> |
> | Use four times more cache entries and divide the memory for each entry
> | by four. This lowers the linear read throughput somewhat but increases
> | the access speed for filesystems.
Yes, I know. I've read the commit message. But this patch even costs me ~200ms
in boottime (loading kernel and dts from ext4 root partition).

[...]
> We could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full
> chunk size (16KiB). Does this give you back some lost performance?
No, changing RW_BUF_SIZE did not help.

Regards
Hubert

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH] habv4: add the possibility to changing the signing area from Kconfig

2019-12-10 Thread Sascha Hauer
On Tue, Dec 10, 2019 at 04:06:27PM +0100, Maik Otto wrote:
> the whole barebox with mbr and partition table will be signed by default
> add the possibility in the Kconfig to change from full signing to skip-mbr
> and from-dcdofs

Not signing the MBR seems the right thing to do. Do we need it
configurable at all or would it be better to just always skip the first
KiB?

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [BUG] imx6qdl: degraded eMMC write performance

2019-12-10 Thread Sascha Hauer
Hi Hubert,

On Tue, Dec 10, 2019 at 03:44:52PM +0100, Hubert Feurstein wrote:
> Hi,
> 
> I've updated barebox for our custom platform from v2015.06.0 to
> v2019.12.0. With the new version I have noticed a much worse write
> performance onto the onboard eMMC.
> 
> With v2015.06.0 the indicated progress of the copy command is very
> smooth. Calling "cp -v /dev/zero /dev/mmc3.root" takes about 80
> seconds for 256MB. But with v2019.12.0 the progress is very bumpy and
> the copy takes about 280 seconds.
> 
> I've tracked this down to this commit which destroys the performance:
> "block: Adjust cache sizes" (b6fef20c1215c6ef0004f6af4a9c4b77af51dc43)

Well the commit message says it:

| commit b6fef20c1215c6ef0004f6af4a9c4b77af51dc43
| Author: Sascha Hauer 
| Date:   Thu Mar 29 13:49:45 2018 +0200
| 
| block: Adjust cache sizes
| 
| Use four times more cache entries and divide the memory for each entry
| by four. This lowers the linear read throughput somewhat but increases
| the access speed for filesystems.

The barebox block layer is quite simple. It always handles data in chunks of
a hardcoded size. For a high throughput these should be large as this increases
the buffer size we can write in the MMC controller in one go. Increasing the
size is bad for filesystems though, as for each small block a fs wants
to read at minimum a whole chunk must be read, or for write,
read-modified-written.

I remember I once tried to make the caching more intelligent. It seems I didn't
even get far enough to store a branch in my working copy :(

I just saw that in block_op_write() we must first read any partially
written chunk, then modify it and write it back. Only full chunks can be
written without reading them first. This means we should gain
performance when we enter block_op_write() with only full chunks. We
could adjust RW_BUF_SIZE (used by copy_file as buffer size) to a full
chunk size (16KiB). Does this give you back some lost performance?

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH 1/1] mci: imx-esdhc: fix termination of statement

2019-12-10 Thread Hubert Feurstein
Use semicolon instead of comma to terminate statement.

Signed-off-by: Hubert Feurstein 
---
 drivers/mci/imx-esdhc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mci/imx-esdhc.c b/drivers/mci/imx-esdhc.c
index db3450a26..4f9fe2996 100644
--- a/drivers/mci/imx-esdhc.c
+++ b/drivers/mci/imx-esdhc.c
@@ -697,7 +697,7 @@ static int fsl_esdhc_probe(struct device_d *dev)
host->mci.card_present = esdhc_card_present;
host->mci.hw_dev = dev;
 
-   dev->detect = fsl_esdhc_detect,
+   dev->detect = fsl_esdhc_detect;
 
rate = clk_get_rate(host->clk);
host->mci.f_min = rate >> 12;
-- 
2.24.0


___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[BUG] imx6qdl: degraded eMMC write performance

2019-12-10 Thread Hubert Feurstein
Hi,

I've updated barebox for our custom platform from v2015.06.0 to
v2019.12.0. With the new version I have noticed a much worse write
performance onto the onboard eMMC.

With v2015.06.0 the indicated progress of the copy command is very
smooth. Calling "cp -v /dev/zero /dev/mmc3.root" takes about 80
seconds for 256MB. But with v2019.12.0 the progress is very bumpy and
the copy takes about 280 seconds.

I've tracked this down to this commit which destroys the performance:
"block: Adjust cache sizes" (b6fef20c1215c6ef0004f6af4a9c4b77af51dc43)

Regards
Hubert

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path

2019-12-10 Thread Stefan Riedmüller

Hi Sascha,

On 10.12.19 11:45, Sascha Hauer wrote:

On Tue, Dec 10, 2019 at 09:15:52AM +0100, Stefan Riedmüller wrote:

Hi Sascha,

On 10.12.19 09:05, Sascha Hauer wrote:

On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote:

Hi Sascha,

On 09.12.19 16:37, Sascha Hauer wrote:

On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote:

Change environment device-path from using a separate label to
referencing a device plus partname.


Why? The way it was is fine.


Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not
aware of?


It feels more natural to directly point to the partition, that's why I
prefer that way. So if you don't have a good reason I suggest to convert
it the other way round for consistency


I will recheck if there was a specific reason for us to use the partname
approach. But you can drop this patch for now.


I just tried. The series doesn't apply on current master. Could you
rebase it?


Yes, of course.

Stefan



Sascha



___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


[PATCH v2] firmware: zynqmp-fpga: drop example bin format header

2019-12-10 Thread Michael Tretter
Avoid the example bitstream header to validate the bitstream that should
be loaded into the FPGA. The header is mostly 0x with a few
special values at a certain offsets and can be better described with the
offsets and their magic values.

As a drive by, this fixes/removes a broken check in the header
validation. The != operator has a higher precedence than ?: and this
check should have had parenthesis around the ?: expression:

bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ?
  bin_format[i] : __swab32(bin_format[i])

Signed-off-by: Michael Tretter 
Reviewed-by: Thomas Haemmerle 
---
Changes in v2:
 - Fix subject zynmp-fpga -> zynqmp-fpga

 drivers/firmware/zynqmp-fpga.c | 121 +
 1 file changed, 62 insertions(+), 59 deletions(-)

diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
index 1728e2a954..e02667355f 100644
--- a/drivers/firmware/zynqmp-fpga.c
+++ b/drivers/firmware/zynqmp-fpga.c
@@ -24,11 +24,32 @@
 #define ZYNQMP_PM_VERSION_1_1_FEATURES (ZYNQMP_PM_FEATURE_BYTE_ORDER_IRREL | \
 ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
 
+/*
+ * Xilinx KU040 Bitstream Composition:
+ *
+ * Bitstream can be provided with an optinal header (`struct bs_header`).
+ * The true bitstream starts with the binary-header composed of 21 words:
+ *
+ *  0: 0x (Dummy pad word)
+ * ...
+ * 15: 0x (Dummy pad word)
+ * 16: 0x00BB (Bus width auto detect word 1)
+ * 17: 0x11220044 (Bus width auto detect word 2)
+ * 18: 0x (Dummy pad word)
+ * 19: 0x (Dummy pad word)
+ * 20: 0xAA995566 (Sync word)
+ *
+ * See Xilinx UG570 (v1.11) September 30 2019, Chapter 9 "Configuration
+ * Details - Bitstream Composition" for further details.
+ */
 #define DUMMY_WORD 0x
-#define BUS_WIDTH_WORD_1   0x00BB
-#define BUS_WIDTH_WORD_2   0x11220044
+#define BUS_WIDTH_AUTO_DETECT1_OFFSET  16
+#define BUS_WIDTH_AUTO_DETECT1 0x00BB
+#define BUS_WIDTH_AUTO_DETECT2_OFFSET  17
+#define BUS_WIDTH_AUTO_DETECT2 0x11220044
+#define SYNC_WORD_OFFSET   20
 #define SYNC_WORD  0xAA995566
-#define SYNC_WORD_OFFS 20
+#define BIN_HEADER_LENGTH  21
 
 enum xilinx_byte_order {
XILINX_BYTE_ORDER_BIT,
@@ -58,48 +79,6 @@ struct bs_header_entry {
char data[0];
 } __attribute__ ((packed));
 
-/*
- * Xilinx KU040 Bitstream Composition:
- * Bitstream can be provided with an optinal header (`struct bs_header`).
- * The true bitstream starts with the binary-header composed of 21 words:
- *
- *  1: 0x (Dummy pad word)
- * ...
- * 16: 0x (Dummy pad word)
- * 17: 0x00BB (Bus width auto detect word 1)
- * 18: 0x11220044 (Bus width auto detect word 2)
- * 19: 0x (Dummy pad word)
- * 20: 0x (Dummy pad word)
- * 21: 0xAA995566 (Sync word)
- *
- * Please refer to  Xilinx UG570 (v1.11) September 30 2019,
- * Chapter 9 Configuration Details - Bitstream Composition
- * for further details!
- */
-static const u32 bin_format[] = {
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   BUS_WIDTH_WORD_1,
-   BUS_WIDTH_WORD_2,
-   DUMMY_WORD,
-   DUMMY_WORD,
-   SYNC_WORD,
-};
-
 static void copy_words_swapped(u32 *dst, const u32 *src, size_t size)
 {
int i;
@@ -111,36 +90,59 @@ static void copy_words_swapped(u32 *dst, const u32 *src, 
size_t size)
 static int get_byte_order(const u32 *bin_header, size_t size,
  enum xilinx_byte_order *byte_order)
 {
-   if (size < sizeof(bin_format))
+   if (size < BIN_HEADER_LENGTH * sizeof(*bin_header))
return -EINVAL;
 
-   if (bin_header[SYNC_WORD_OFFS] == SYNC_WORD) {
+   if (bin_header[SYNC_WORD_OFFSET] == SYNC_WORD) {
*byte_order = XILINX_BYTE_ORDER_BIT;
return 0;
}
 
-   if (bin_header[SYNC_WORD_OFFS] == __swab32(SYNC_WORD)) {
-   *byte_order =  XILINX_BYTE_ORDER_BIN;
+   if (bin_header[SYNC_WORD_OFFSET] == __swab32(SYNC_WORD)) {
+   *byte_order = XILINX_BYTE_ORDER_BIN;
return 0;
}
 
return -EINVAL;
 }
 
-static int is_bin_header_valid(const u32 *bin_header, size_t size,
-  enum xilinx_byte_order byte_order)
+static bool is_bin_header_valid(const u32 *bin_header, size_t size,
+   enum xilinx_byte_order byte_order)
 {
-   int i;
+   size_t i;
 
-   if (size < ARRAY_SIZE(bin_format))
-   return 0;
+   if (size < BIN_HEADER_LENGTH * sizeof(*bin_header))
+   

Re: [PATCH] firmware: zynmp-fpga: drop example bin format header

2019-12-10 Thread Thomas Hämmerle
typo in subject: zynmp-fpga -> zynqmp-fpga

On 09.12.19 14:59, Michael Tretter wrote:
> Avoid the example bitstream header to validate the bitstream that should
> be loaded into the FPGA. The header is mostly 0x with a few
> special values at a certain offsets and can be better described with the
> offsets and their magic values.
> 
> As a drive by, this fixes/removes a broken check in the header
> validation. The != operator has a higher precedence than ?: and this
> check should have had parenthesis around the ?: expression:
> 
>   bin_header[i] != (byte_order == XILINX_BYTE_ORDER_BIT) ?
>bin_format[i] : __swab32(bin_format[i])
> 
> Signed-off-by: Michael Tretter 

Reviewed-by: Thomas Haemmerle 

> ---
>   drivers/firmware/zynqmp-fpga.c | 121 +
>   1 file changed, 62 insertions(+), 59 deletions(-)
> 
> diff --git a/drivers/firmware/zynqmp-fpga.c b/drivers/firmware/zynqmp-fpga.c
> index 1728e2a954..e02667355f 100644
> --- a/drivers/firmware/zynqmp-fpga.c
> +++ b/drivers/firmware/zynqmp-fpga.c
> @@ -24,11 +24,32 @@
>   #define ZYNQMP_PM_VERSION_1_1_FEATURES  
> (ZYNQMP_PM_FEATURE_BYTE_ORDER_IRREL | \
>ZYNQMP_PM_FEATURE_SIZE_NOT_NEEDED)
>   
> +/*
> + * Xilinx KU040 Bitstream Composition:
> + *
> + * Bitstream can be provided with an optinal header (`struct bs_header`).
> + * The true bitstream starts with the binary-header composed of 21 words:
> + *
> + *  0: 0x (Dummy pad word)
> + * ...
> + * 15: 0x (Dummy pad word)
> + * 16: 0x00BB (Bus width auto detect word 1)
> + * 17: 0x11220044 (Bus width auto detect word 2)
> + * 18: 0x (Dummy pad word)
> + * 19: 0x (Dummy pad word)
> + * 20: 0xAA995566 (Sync word)
> + *
> + * See Xilinx UG570 (v1.11) September 30 2019, Chapter 9 "Configuration
> + * Details - Bitstream Composition" for further details.
> + */
>   #define DUMMY_WORD  0x
> -#define BUS_WIDTH_WORD_1 0x00BB
> -#define BUS_WIDTH_WORD_2 0x11220044
> +#define BUS_WIDTH_AUTO_DETECT1_OFFSET16
> +#define BUS_WIDTH_AUTO_DETECT1   0x00BB
> +#define BUS_WIDTH_AUTO_DETECT2_OFFSET17
> +#define BUS_WIDTH_AUTO_DETECT2   0x11220044
> +#define SYNC_WORD_OFFSET 20
>   #define SYNC_WORD   0xAA995566
> -#define SYNC_WORD_OFFS   20
> +#define BIN_HEADER_LENGTH21
>   
>   enum xilinx_byte_order {
>   XILINX_BYTE_ORDER_BIT,
> @@ -58,48 +79,6 @@ struct bs_header_entry {
>   char data[0];
>   } __attribute__ ((packed));
>   
> -/*
> - * Xilinx KU040 Bitstream Composition:
> - * Bitstream can be provided with an optinal header (`struct bs_header`).
> - * The true bitstream starts with the binary-header composed of 21 words:
> - *
> - *  1: 0x (Dummy pad word)
> - * ...
> - * 16: 0x (Dummy pad word)
> - * 17: 0x00BB (Bus width auto detect word 1)
> - * 18: 0x11220044 (Bus width auto detect word 2)
> - * 19: 0x (Dummy pad word)
> - * 20: 0x (Dummy pad word)
> - * 21: 0xAA995566 (Sync word)
> - *
> - * Please refer to  Xilinx UG570 (v1.11) September 30 2019,
> - * Chapter 9 Configuration Details - Bitstream Composition
> - * for further details!
> - */
> -static const u32 bin_format[] = {
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - BUS_WIDTH_WORD_1,
> - BUS_WIDTH_WORD_2,
> - DUMMY_WORD,
> - DUMMY_WORD,
> - SYNC_WORD,
> -};
> -
>   static void copy_words_swapped(u32 *dst, const u32 *src, size_t size)
>   {
>   int i;
> @@ -111,36 +90,59 @@ static void copy_words_swapped(u32 *dst, const u32 *src, 
> size_t size)
>   static int get_byte_order(const u32 *bin_header, size_t size,
> enum xilinx_byte_order *byte_order)
>   {
> - if (size < sizeof(bin_format))
> + if (size < BIN_HEADER_LENGTH * sizeof(*bin_header))
>   return -EINVAL;
>   
> - if (bin_header[SYNC_WORD_OFFS] == SYNC_WORD) {
> + if (bin_header[SYNC_WORD_OFFSET] == SYNC_WORD) {
>   *byte_order = XILINX_BYTE_ORDER_BIT;
>   return 0;
>   }
>   
> - if (bin_header[SYNC_WORD_OFFS] == __swab32(SYNC_WORD)) {
> - *byte_order =  XILINX_BYTE_ORDER_BIN;
> + if (bin_header[SYNC_WORD_OFFSET] == __swab32(SYNC_WORD)) {
> + *byte_order = XILINX_BYTE_ORDER_BIN;
>   return 0;
>   }
>   
>   return -EINVAL;
>   }
>   
> -static int is_bin_header_valid(const u32 *bin_header, size_t size,
> -enum xilinx_byte_order byte_order)
> +static bool is_bin_header_valid(const u32 *bin_header, size_t 

Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path

2019-12-10 Thread Sascha Hauer
On Tue, Dec 10, 2019 at 09:15:52AM +0100, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> On 10.12.19 09:05, Sascha Hauer wrote:
> > On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote:
> > > Hi Sascha,
> > > 
> > > On 09.12.19 16:37, Sascha Hauer wrote:
> > > > On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote:
> > > > > Change environment device-path from using a separate label to
> > > > > referencing a device plus partname.
> > > > 
> > > > Why? The way it was is fine.
> > > 
> > > Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm 
> > > not
> > > aware of?
> > 
> > It feels more natural to directly point to the partition, that's why I
> > prefer that way. So if you don't have a good reason I suggest to convert
> > it the other way round for consistency
> 
> I will recheck if there was a specific reason for us to use the partname
> approach. But you can drop this patch for now.

I just tried. The series doesn't apply on current master. Could you
rebase it?

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path

2019-12-10 Thread Stefan Riedmüller

Hi Sascha,

On 10.12.19 09:05, Sascha Hauer wrote:

On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote:

Hi Sascha,

On 09.12.19 16:37, Sascha Hauer wrote:

On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote:

Change environment device-path from using a separate label to
referencing a device plus partname.


Why? The way it was is fine.


Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not
aware of?


It feels more natural to directly point to the partition, that's why I
prefer that way. So if you don't have a good reason I suggest to convert
it the other way round for consistency


I will recheck if there was a specific reason for us to use the partname 
approach. But you can drop this patch for now.


Thanks
Stefan



Sascha



___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox


Re: [PATCH v2 5/8] ARM: dts: imx6: phycard: Use partname for environment device-path

2019-12-10 Thread Sascha Hauer
On Tue, Dec 10, 2019 at 08:50:41AM +0100, Stefan Riedmüller wrote:
> Hi Sascha,
> 
> On 09.12.19 16:37, Sascha Hauer wrote:
> > On Mon, Dec 09, 2019 at 01:31:40PM +0100, Stefan Riedmueller wrote:
> > > Change environment device-path from using a separate label to
> > > referencing a device plus partname.
> > 
> > Why? The way it was is fine.
> 
> Just to be consistent with phyCORE and phyFLEX. Is there a downside I'm not
> aware of?

It feels more natural to directly point to the partition, that's why I
prefer that way. So if you don't have a good reason I suggest to convert
it the other way round for consistency

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |

___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox