Re: [PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"

[Gary Bisson]

Hi,

On Fri, Nov 18, 2022 at 10:36:58AM -0500, Sean Anderson wrote:
> On 11/18/22 07:13, Gary Bisson wrote:
> > This reverts commit 62649165cb02ab95b57360bb362886935f524f26.
> > 
> > The patch decreased the write performance quite a bit.
> > Here is an example on an i.MX 8M Quad platform.
> > - Before the revert:
> > Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.113s]
> > Writing 'vendor'   OKAY [128.335s]
> > Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  0.802s]
> > Writing 'vendor'   OKAY [ 27.902s]
> > - After the revert:
> > Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.310s]
> > Writing 'vendor'   OKAY [ 18.041s]
> > Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  1.244s]
> > Writing 'vendor'   OKAY [  2.663s]
> > 
> > Considering that the patch only moves buffer around to avoid a warning
> > message about misaligned buffers, let's keep the best performances.
> 
> So what is the point of this warning?

Well the warning does say something true that the cache operation is not
aligned. Better ask Simon as he's the one who changed the print from a
debug to warn_non_spl one:
bcc53bf0958 arm: Show cache warnings in U-Boot proper only

BTW, in my case I couldn't see the misaligned messages, yet I saw the
performance hit described above.

Regards,
Gary

[PATCH] Revert "lib: sparse: Make CHUNK_TYPE_RAW buffer aligned"

[Gary Bisson]

This reverts commit 62649165cb02ab95b57360bb362886935f524f26.

The patch decreased the write performance quite a bit.
Here is an example on an i.MX 8M Quad platform.
- Before the revert:
Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.113s]
Writing 'vendor'   OKAY [128.335s]
Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  0.802s]
Writing 'vendor'   OKAY [ 27.902s]
- After the revert:
Sending sparse 'vendor' 1/2 (516436 KB)OKAY [  5.310s]
Writing 'vendor'   OKAY [ 18.041s]
Sending sparse 'vendor' 2/2 (76100 KB) OKAY [  1.244s]
Writing 'vendor'   OKAY [  2.663s]

Considering that the patch only moves buffer around to avoid a warning
message about misaligned buffers, let's keep the best performances.

Signed-off-by: Gary Bisson 
Signed-off-by: Troy Kisky 
---
 lib/image-sparse.c | 69 ++
 1 file changed, 8 insertions(+), 61 deletions(-)

diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index 5ec0f94ab3e..d80fdbbf58e 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,66 +46,9 @@
 #include 
 
 #include 
-#include 
 
 static void default_log(const char *ignored, char *response) {}
 
-static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
-  lbaint_t blk, lbaint_t blkcnt,
-  void *data,
-  char *response)
-{
-   lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
-   uint32_t *aligned_buf = NULL;
-
-   if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
-   write_blks = info->write(info, blk, n, data);
-   if (write_blks < n)
-   goto write_fail;
-
-   return write_blks;
-   }
-
-   aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * 
aligned_buf_blks);
-   if (!aligned_buf) {
-   info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
-   return -ENOMEM;
-   }
-
-   while (blkcnt > 0) {
-   n = min(aligned_buf_blks, blkcnt);
-   memcpy(aligned_buf, data, n * info->blksz);
-
-   /* write_blks might be > n due to NAND bad-blocks */
-   write_blks = info->write(info, blk + blks, n, aligned_buf);
-   if (write_blks < n) {
-   free(aligned_buf);
-   goto write_fail;
-   }
-
-   blks += write_blks;
-   data += n * info->blksz;
-   blkcnt -= n;
-   }
-
-   free(aligned_buf);
-   return blks;
-
-write_fail:
-   if (IS_ERR_VALUE(write_blks)) {
-   printf("%s: Write failed, block #" LBAFU " [" LBAFU "] 
(%lld)\n",
-  __func__, blk + blks, n, (long long)write_blks);
-   info->mssg("flash write failure", response);
-   return write_blks;
-   }
-
-   /* write_blks < n */
-   printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
-  __func__, blk + blks, n);
-   info->mssg("flash write failure(incomplete)", response);
-   return -1;
-}
-
 int write_sparse_image(struct sparse_storage *info,
   const char *part_name, void *data, char *response)
 {
@@ -209,11 +152,15 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
 
-   blks = write_sparse_chunk_raw(info, blk, blkcnt,
- data, response);
-   if (blks < 0)
+   blks = info->write(info, blk, blkcnt, data);
+   /* blks might be > blkcnt (eg. NAND bad-blocks) */
+   if (blks < blkcnt) {
+   printf("%s: %s" LBAFU " [" LBAFU "]\n",
+  __func__, "Write failed, block #",
+  blk, blks);
+   info->mssg("flash write failure", response);
return -1;
-
+   }
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
-- 
2.35.1

[PATCH] cmd: bcb: fix bcb struct alignment issue

[Gary Bisson]

Without this patch the bcb struct could be located at an odd address
which resulted in data not being copied to the buffer.

Here was the repro steps (from Mattijs):
=> mmc dev 1
=> bcb load 1 misc
=> bcb dump command
: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=> part start mmc 1 misc misc_start
=> mmc read ${loadaddr} ${misc_start} 4
=> bcb load 1 misc
=> bcb dump command
: 62 6f 6f 74 6f 6e 63 65 2d 62 6f 6f 74 6c 6f 61
0010: 64 65 72 00 00 00 00 00 00 00 00 00 00 00 00 00

This behavior was observed on an Amlogic A311D (ARM64) platform with a
recent GCC toolchain (11.2.0) but is most likely affecting other
platforms.

To avoid issues the structure is aligned on DMA minimum alignment value
as it is passed directly to the read function.

Signed-off-by: Gary Bisson 
---
 cmd/bcb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/bcb.c b/cmd/bcb.c
index 6b6f1e9a2f1..92f4d27990d 100644
--- a/cmd/bcb.c
+++ b/cmd/bcb.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 enum bcb_cmd {
BCB_CMD_LOAD,
@@ -24,7 +25,7 @@ enum bcb_cmd {
 
 static int bcb_dev = -1;
 static int bcb_part = -1;
-static struct bootloader_message bcb = { { 0 } };
+static struct bootloader_message bcb __aligned(ARCH_DMA_MINALIGN) = { { 0 } };
 
 static int bcb_cmd_get(char *cmd)
 {
-- 
2.34.1

[PATCH] usb: gadget: dwc2_udc_otg: set ep's desc during enable/disable

[Gary Bisson]

Fastboot support has been broken on platforms using dwc2 controller
since the gadget gets its max packet size from it.
This patch is the equivalent of 723fd5668ff which fixed the same issue
but for the chipidea controller.

Fixes: 27c9141b111 ("usb: gadget: fastboot: use correct max packet size")

Signed-off-by: Gary Bisson 
---
 drivers/usb/gadget/dwc2_udc_otg.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/dwc2_udc_otg.c 
b/drivers/usb/gadget/dwc2_udc_otg.c
index 2f318144421..fb10884755b 100644
--- a/drivers/usb/gadget/dwc2_udc_otg.c
+++ b/drivers/usb/gadget/dwc2_udc_otg.c
@@ -655,6 +655,7 @@ static int dwc2_ep_enable(struct usb_ep *_ep,
return -ESHUTDOWN;
}
 
+   _ep->desc = desc;
ep->stopped = 0;
ep->desc = desc;
ep->pio_irqs = 0;
@@ -695,6 +696,7 @@ static int dwc2_ep_disable(struct usb_ep *_ep)
/* Nuke all pending requests */
nuke(ep, -ESHUTDOWN);
 
+   _ep->desc = NULL;
ep->desc = 0;
ep->stopped = 1;
 
-- 
2.34.1

[PATCH] nitrogen6x: add missing pinctrl to fix mmc

[Gary Bisson]

Since commit f7ac30b042d, the pin muxing for mmc was removed from the
board file to be managed by DM_MMC which requires PINCTRL to work. It
made the change for sabrelite but nitrogen configs were forgotten.

Signed-off-by: Gary Bisson 
---
 configs/nitrogen6dl2g_defconfig | 2 ++
 configs/nitrogen6dl_defconfig   | 2 ++
 configs/nitrogen6q2g_defconfig  | 2 ++
 configs/nitrogen6q_defconfig| 2 ++
 configs/nitrogen6s1g_defconfig  | 2 ++
 configs/nitrogen6s_defconfig| 2 ++
 6 files changed, 12 insertions(+)

diff --git a/configs/nitrogen6dl2g_defconfig b/configs/nitrogen6dl2g_defconfig
index 593a43e5e79..20c5d302577 100644
--- a/configs/nitrogen6dl2g_defconfig
+++ b/configs/nitrogen6dl2g_defconfig
@@ -68,6 +68,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
diff --git a/configs/nitrogen6dl_defconfig b/configs/nitrogen6dl_defconfig
index 4bcc6756801..796bd66bc23 100644
--- a/configs/nitrogen6dl_defconfig
+++ b/configs/nitrogen6dl_defconfig
@@ -68,6 +68,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
diff --git a/configs/nitrogen6q2g_defconfig b/configs/nitrogen6q2g_defconfig
index 76fc53d5154..b42220db064 100644
--- a/configs/nitrogen6q2g_defconfig
+++ b/configs/nitrogen6q2g_defconfig
@@ -70,6 +70,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
diff --git a/configs/nitrogen6q_defconfig b/configs/nitrogen6q_defconfig
index fca3e5f5311..cc085594967 100644
--- a/configs/nitrogen6q_defconfig
+++ b/configs/nitrogen6q_defconfig
@@ -70,6 +70,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
diff --git a/configs/nitrogen6s1g_defconfig b/configs/nitrogen6s1g_defconfig
index 8b720b0d600..17133c5cd60 100644
--- a/configs/nitrogen6s1g_defconfig
+++ b/configs/nitrogen6s1g_defconfig
@@ -68,6 +68,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
diff --git a/configs/nitrogen6s_defconfig b/configs/nitrogen6s_defconfig
index a9d239e9be9..242580e3e9f 100644
--- a/configs/nitrogen6s_defconfig
+++ b/configs/nitrogen6s_defconfig
@@ -68,6 +68,8 @@ CONFIG_PHY_MICREL=y
 CONFIG_PHY_MICREL_KSZ90X1=y
 CONFIG_FEC_MXC=y
 CONFIG_MII=y
+CONFIG_PINCTRL=y
+CONFIG_PINCTRL_IMX6=y
 CONFIG_MXC_UART=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
-- 
2.34.1

[PATCH v2] disk: part_dos: update partition table entries after write

[Gary Bisson]

Fixes issues when switching from GPT to MBR partition tables.

Signed-off-by: Gary Bisson 
---
Changes for v2:
- added part_init() inside write_mbr_partitions(), as suggested by
  Heinrich
---
 disk/part_dos.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index f431925745c..60addc6e00d 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -423,6 +423,9 @@ int write_mbr_partitions(struct blk_desc *dev,
ext_part_sect = next_ebr;
}
 
+   /* Update the partition table entries*/
+   part_init(dev_desc);
+
return 0;
 }
 
@@ -499,6 +502,9 @@ int write_mbr_sector(struct blk_desc *dev_desc, void *buf)
return 1;
}
 
+   /* Update the partition table entries*/
+   part_init(dev_desc);
+
return 0;
 }
 
-- 
2.29.2

Re: [PATCH] disk: part_dos: update partition table entries after write

[Gary Bisson]

Hi Heinrich,

On Wed, Jan 27, 2021 at 09:56:13PM +0100, Heinrich Schuchardt wrote:
> On 1/27/21 9:19 PM, Gary Bisson wrote:
> > Fixes issues when switching from GPT to MBR partition tables.
> 
> This does not catch all cases of changing the MBR. See function
> write_mbr_partitions() with writes both the MBR and EBRs (if applicable).

Good catch! That's correct, I missed it for 2 reasons:
1- I focused on the fastboot code which indeed only calls
write_mbr_sector()
2- I did that work on 2020.10 U-Boot which didn't include
write_mbr_partitions().

> Android devices typically have more than 4 partitions. Why does fastboot
> not update the extended boot records?

Fastboot is not only used for Android, we generally use it to flash our
Linux images as well. I guess the code just hasn't been updated to
handle EBR although I'd say that most people use GPT, especially Android
users.

Anyway, I'll submit a v2 that take care of write_mbr_partitions().

Regards,
Gary

[PATCH] disk: part_dos: update partition table entries after write

[Gary Bisson]

Fixes issues when switching from GPT to MBR partition tables.

Signed-off-by: Gary Bisson 
---
Hi,

Sending this patch as a follow-up to the other one [1] doing the same
thing for GPT write.

Let me know if you have any questions.

Regards,
Gary

[1] https://lists.denx.de/pipermail/u-boot/2021-January/438764.html
---
 disk/part_dos.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/disk/part_dos.c b/disk/part_dos.c
index f431925745..470886f4bb 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -499,6 +499,9 @@ int write_mbr_sector(struct blk_desc *dev_desc, void *buf)
return 1;
}
 
+   /* Update the partition table entries*/
+   part_init(dev_desc);
+
return 0;
 }
 
-- 
2.29.2

Re: [PATCH] disk: part_efi: update partition table entries after write

[Gary Bisson]

Hi Heinrich,

On Tue, Jan 26, 2021 at 04:08:13PM +0100, Heinrich Schuchardt wrote:
> On 26.01.21 14:56, Gary Bisson wrote:
> > Fixes fastboot issues when switching from mbr to gpt partition tables.
> >
> > Signed-off-by: Gary Bisson 
> > ---
> > Hi,
> >
> > I hesitated calling this a RFC as I'm not sure everyone will like the
> > idea.
> >
> > Basically the issue encountered was the following:
> > - Device with its storage flashed with a MBR partition table
> > - Run fastboot to flash a new OS (and new partition table)
> > - After flashing a GPT partition table, U-Boot couldn't find any of the
> >   partition names from the GPT
> >   -> reason is that the partition table entries aren't updated
> >   -> so U-Boot still believes it's a MBR table inside and so its parsing
> >   fails
> >
> > This commit adds an update of the table entries inside the
> > write_mbr_and_gpt_partitions() function. At first I thought maybe this
> > should be added in fastboot file (fb_mmc.c) but couldn't think of a good
> > reason why this shouldn't happened in part_efi directly.
> >
> > Let me know if you have any questions.
> >
> > Regards,
> > Gary
> > ---
> >  disk/part_efi.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/disk/part_efi.c b/disk/part_efi.c
> > index 2f922662e6e..7a24e33d189 100644
> > --- a/disk/part_efi.c
> > +++ b/disk/part_efi.c
> > @@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc 
> > *dev_desc, void *buf)
> > return 1;
> > }
> >
> > +   /* Update the partition table entries*/
> > +   part_init(dev_desc);
> > +
> > return 0;
> >  }
> >  #endif
> >
> 
> This change is for GPT only. Don't we need the same for MBR partition
> tables (write_mbr_partitions())?

I actually have another commit for the MBR partition[1], I just wanted
to send the GPT one first to see if it was an acceptable approach.

Regards,
Gary

[1] https://github.com/boundarydevices/u-boot-imx6/commit/5fc23bce

[PATCH] disk: part_efi: update partition table entries after write

[Gary Bisson]

Fixes fastboot issues when switching from mbr to gpt partition tables.

Signed-off-by: Gary Bisson 
---
Hi,

I hesitated calling this a RFC as I'm not sure everyone will like the
idea.

Basically the issue encountered was the following:
- Device with its storage flashed with a MBR partition table
- Run fastboot to flash a new OS (and new partition table)
- After flashing a GPT partition table, U-Boot couldn't find any of the
  partition names from the GPT
  -> reason is that the partition table entries aren't updated
  -> so U-Boot still believes it's a MBR table inside and so its parsing
  fails

This commit adds an update of the table entries inside the
write_mbr_and_gpt_partitions() function. At first I thought maybe this
should be added in fastboot file (fb_mmc.c) but couldn't think of a good
reason why this shouldn't happened in part_efi directly.

Let me know if you have any questions.

Regards,
Gary
---
 disk/part_efi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/disk/part_efi.c b/disk/part_efi.c
index 2f922662e6e..7a24e33d189 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -867,6 +867,9 @@ int write_mbr_and_gpt_partitions(struct blk_desc *dev_desc, 
void *buf)
return 1;
}
 
+   /* Update the partition table entries*/
+   part_init(dev_desc);
+
return 0;
 }
 #endif
-- 
2.29.2

Re: [PATCH v2] fastboot: getvar: fix partition-size return value

[Gary Bisson]

Hi Sam,

On Tue, Sep 01, 2020 at 01:31:46PM +0300, Sam Protsenko wrote:
> Hi Gary,
> 
> On Thu, 27 Aug 2020 at 11:51, Gary Bisson
>  wrote:
> >
> > The size returned by 'getvar partition-size' should be in bytes, not in
> > blocks as fastboot uses that value to generate empty partition when
> > running format [1].
> >
> > Note that the function was already returning the proper size in bytes
> > for NAND devices (see struct part_info details).
> >
> > [1]
> > https://android.googlesource.com/platform/system/core/+/refs/heads/android10-release/fastboot/fastboot.cpp#1500
> >
> > Signed-off-by: Gary Bisson 
> > ---
> 
> Thank you for the patch, all look good! As I understand from the
> changelog, v2 fixes sunxi build error found by Lukasz?

Indeed it does. The part_info struct for NAND devices was already
returning a size in bytes instead of blocks.

> Other than that:
> 
> Reviewed-by: Sam Protsenko 

Thanks,
Gary

[PATCH v2] fastboot: getvar: fix partition-size return value

[Gary Bisson]

The size returned by 'getvar partition-size' should be in bytes, not in
blocks as fastboot uses that value to generate empty partition when
running format [1].

Note that the function was already returning the proper size in bytes
for NAND devices (see struct part_info details).

[1]
https://android.googlesource.com/platform/system/core/+/refs/heads/android10-release/fastboot/fastboot.cpp#1500

Signed-off-by: Gary Bisson 
---
Changelog v1->v2:
- removed change for FASTBOOT_FLASH_NAND as not necessary (and therefore
  not building)
---
 drivers/fastboot/fb_getvar.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 52da34b1e37..d43f2cfee66 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -95,7 +95,7 @@ static const struct {
  *
  * @param[in] part_name Info for which partition name to look for
  * @param[in,out] response Pointer to fastboot response buffer
- * @param[out] size If not NULL, will contain partition size (in blocks)
+ * @param[out] size If not NULL, will contain partition size
  * @return Partition number or negative value on error
  */
 static int getvar_get_part_info(const char *part_name, char *response,
@@ -109,7 +109,7 @@ static int getvar_get_part_info(const char *part_name, char 
*response,
r = fastboot_mmc_get_part_info(part_name, _desc, _info,
   response);
if (r >= 0 && size)
-   *size = part_info.size;
+   *size = part_info.size * part_info.blksz;
 # elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
struct part_info *part_info;
 
-- 
2.28.0

Re: [PATCH] fastboot: getvar: fix partition-size return value

[Gary Bisson]

Hi Lukasz,

On Thu, Aug 27, 2020 at 08:25:51AM +0200, Lukasz Majewski wrote:
> Hi Gary,
> 
> > Hi Lukasz,
> > 
> > On Wed, Aug 26, 2020 at 11:36:51AM +0200, Lukasz Majewski wrote:
> > > Hi Gary,
> > >   
> > > > Hi,
> > > > 
> > > > Gentle ping on this patch. Hopefully Sam's email won't bounce this
> > > > time.  
> > > 
> > > You couldn't have better timing than now :-)
> > > 
> > > I'm now testing PR for Tom [1] and your original patch was causing
> > > some issues (probably it was correct when it was posted, but it was
> > > my fault that I'm going to pull it in now - my apologizes).
> > > 
> > > I've fixed it [2] - please check if this fix is OK.  
> > 
> > Actually it was wrong before too, thanks for catching it!
> > Reason is that I didn't build with FASTBOOT_FLASH_NAND config enabled
> > which I should have done to check the second part of the change...
> 
> Ok. I've found another issue with this patch - it has some issues with
> sunxi:
> 
> drivers/fastboot/fb_getvar.c: In function 'getvar_get_part_info':
> 
> +drivers/fastboot/fb_getvar.c:118:38: error: 'struct part_info' has no
> member named 'blksz'
> 
> +  118 |   *size = part_info->size * part_info->blksz;
> 
> +  |  ^~
> 
> +make[3]: *** [drivers/fastboot/fb_getvar.o] Error 1
> 
> The whole CI run can be found here:
> https://travis-ci.org/github/lmajewski/u-boot-dfu/builds/721449368

Thanks, I'll take a look.

> > > Now I'm hunting another issues with sandbox [3]. When fixed I will
> > > send the PR.  
> > 
> > Sounds good. Let me know if you need anything from me.
> 
> I think that the best solution would be if I drop this patch from
> the series and send PR (after some CI testing) without it. If you
> manage to fix it ASAP, then I will pull it immediately.

Sure let's do this, drop my patch for now, I'll re-submit when possible.

Regards,
Gary

Re: [PATCH] fastboot: getvar: fix partition-size return value

[Gary Bisson]

Hi Lukasz,

On Wed, Aug 26, 2020 at 11:36:51AM +0200, Lukasz Majewski wrote:
> Hi Gary,
> 
> > Hi,
> > 
> > Gentle ping on this patch. Hopefully Sam's email won't bounce this
> > time.
> 
> You couldn't have better timing than now :-)
> 
> I'm now testing PR for Tom [1] and your original patch was causing some
> issues (probably it was correct when it was posted, but it was my
> fault that I'm going to pull it in now - my apologizes).
> 
> I've fixed it [2] - please check if this fix is OK.

Actually it was wrong before too, thanks for catching it!
Reason is that I didn't build with FASTBOOT_FLASH_NAND config enabled
which I should have done to check the second part of the change...

> Now I'm hunting another issues with sandbox [3]. When fixed I will send
> the PR.

Sounds good. Let me know if you need anything from me.

Regards,
Gary

Re: [PATCH] fastboot: getvar: fix partition-size return value

[Gary Bisson]

Hi,

Gentle ping on this patch. Hopefully Sam's email won't bounce this time.

Let me know if you have any questions.

Regards,
Gary

On Wed, Jun 24, 2020 at 11:00:17AM +0200, Gary Bisson wrote:
> Hi,
> 
> Gentle ping on this patch. Anyone had a chance to review?
> 
> Regards,
> Gary
> 
> On Wed, May 06, 2020 at 10:12:28AM +0200, Gary Bisson wrote:
> > The size returned by 'getvar partition-size' should be in bytes, not in
> > blocks as fastboot uses that value to generate empty partition when
> > running format [1].
> > 
> > [1]
> > https://android.googlesource.com/platform/system/core/+/refs/heads/android10-release/fastboot/fastboot.cpp#1500
> > 
> > Signed-off-by: Gary Bisson 
> > ---
> > Hi,
> > 
> > Another test was to run 'fastboot getvar partition-size:system' on a
> > shipping Android phone, it will give you the size in bytes as well.
> > 
> > Regards,
> > Gary
> > ---
> >  drivers/fastboot/fb_getvar.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> > index 95cb434189..51a2bea86d 100644
> > --- a/drivers/fastboot/fb_getvar.c
> > +++ b/drivers/fastboot/fb_getvar.c
> > @@ -94,7 +94,7 @@ static const struct {
> >   *
> >   * @param[in] part_name Info for which partition name to look for
> >   * @param[in,out] response Pointer to fastboot response buffer
> > - * @param[out] size If not NULL, will contain partition size (in blocks)
> > + * @param[out] size If not NULL, will contain partition size
> >   * @return Partition number or negative value on error
> >   */
> >  static int getvar_get_part_info(const char *part_name, char *response,
> > @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char 
> > *part_name, char *response,
> > r = fastboot_mmc_get_part_info(part_name, _desc, _info,
> >response);
> > if (r >= 0 && size)
> > -   *size = part_info.size;
> > +   *size = part_info.size * part_info.blksz;
> >  # elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
> > struct part_info *part_info;
> >  
> > r = fastboot_nand_get_part_info(part_name, _info, response);
> > if (r >= 0 && size)
> > -   *size = part_info->size;
> > +   *size = part_info->size * part_info.blksz;
> >  # else
> > fastboot_fail("this storage is not supported in bootloader", response);
> > r = -ENODEV;
> > -- 
> > 2.26.2
> > 

Re: [PATCH] fastboot: getvar: fix partition-size return value

[Gary Bisson]

Hi,

Gentle ping on this patch. Anyone had a chance to review?

Regards,
Gary

On Wed, May 06, 2020 at 10:12:28AM +0200, Gary Bisson wrote:
> The size returned by 'getvar partition-size' should be in bytes, not in
> blocks as fastboot uses that value to generate empty partition when
> running format [1].
> 
> [1]
> https://android.googlesource.com/platform/system/core/+/refs/heads/android10-release/fastboot/fastboot.cpp#1500
> 
> Signed-off-by: Gary Bisson 
> ---
> Hi,
> 
> Another test was to run 'fastboot getvar partition-size:system' on a
> shipping Android phone, it will give you the size in bytes as well.
> 
> Regards,
> Gary
> ---
>  drivers/fastboot/fb_getvar.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
> index 95cb434189..51a2bea86d 100644
> --- a/drivers/fastboot/fb_getvar.c
> +++ b/drivers/fastboot/fb_getvar.c
> @@ -94,7 +94,7 @@ static const struct {
>   *
>   * @param[in] part_name Info for which partition name to look for
>   * @param[in,out] response Pointer to fastboot response buffer
> - * @param[out] size If not NULL, will contain partition size (in blocks)
> + * @param[out] size If not NULL, will contain partition size
>   * @return Partition number or negative value on error
>   */
>  static int getvar_get_part_info(const char *part_name, char *response,
> @@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, 
> char *response,
>   r = fastboot_mmc_get_part_info(part_name, _desc, _info,
>  response);
>   if (r >= 0 && size)
> - *size = part_info.size;
> + *size = part_info.size * part_info.blksz;
>  # elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
>   struct part_info *part_info;
>  
>   r = fastboot_nand_get_part_info(part_name, _info, response);
>   if (r >= 0 && size)
> - *size = part_info->size;
> + *size = part_info->size * part_info.blksz;
>  # else
>   fastboot_fail("this storage is not supported in bootloader", response);
>   r = -ENODEV;
> -- 
> 2.26.2
> 

[PATCH v2] cmd: avb: free partition buffer upon verify completion

[Gary Bisson]

Doing the same as the unittests for libavb [1].

Allows to run 'avb verify' multiple times which can be useful after a
failure to be able to re-flash the partition and try again.

[1]
https://android.googlesource.com/platform/external/avb/+/refs/tags/android-9.0.0_r37/test/avb_slot_verify_unittest.cc#156

Signed-off-by: Gary Bisson 
---
Hi,

Changelog v2:
- use avb_slot_verity_data_free as suggested by Igor

This was added because of the following scenario:
1- fastboot flash boot boot.img
2- avb verify
  -> fails because vbmeta wasn't updated
3- fastboot flash vbmeta vbmeta.img
4- avb verify
  -> fails because it can't allocate memory as previous buffer wasn't
freed

Let me know if you have any questions.

Regards,
Gary
---
 cmd/avb.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/cmd/avb.c b/cmd/avb.c
index a4de5c40a2..93d1a31819 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -312,6 +312,9 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
printf("Unknown error occurred\n");
}
 
+   if (out_data)
+   avb_slot_verify_data_free(out_data);
+
return res;
 }
 
-- 
2.26.2

Re: [PATCH] cmd: avb: free partition buffer upon verify completion

[Gary Bisson]

Hi Igor,

On Sun, May 10, 2020 at 01:30:53PM +0300, Igor Opaniuk wrote:
> Hi Gary,
> 
> Thanks for the patch and sorry for a late reply.

No problem.

> On Wed, May 6, 2020 at 5:06 PM Gary Bisson
>  wrote:
> >
> > Allows to run 'avb verify' multiple times which can be useful after a
> > failure to be able to re-flash the partition and try again.
> >
> > Signed-off-by: Gary Bisson 
> > ---
> > Hi,
> >
> > This was added because of the following scenario:
> > 1- fastboot flash boot boot.img
> > 2- avb verify
> >   -> fails because vbmeta wasn't updated
> > 3- fastboot flash vbmeta vbmeta.img
> > 4- avb verify
> >   -> fails because it can't allocate memory as previous buffer wasn't
> > freed
> >
> > Let me know if you have any questions.
> >
> > Regards,
> > Gary
> > ---
> >  cmd/avb.c | 8 
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/cmd/avb.c b/cmd/avb.c
> > index a4de5c40a2..67154de4e1 100644
> > --- a/cmd/avb.c
> > +++ b/cmd/avb.c
> > @@ -312,6 +312,14 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
> > printf("Unknown error occurred\n");
> > }
> >
> > +   /* Free image buffers now that verification is complete */
> > +   if (slot_result != AVB_SLOT_VERIFY_RESULT_ERROR_OOM) {
> Freeing out_data can be done not only when res is
> AVB_SLOT_VERIFY_RESULT_ERROR_OOM.
> For example, in the line 1196 in [1]:
> 
> AvbSlotVerifyResult sub_ret = avb_append_options(ops,
>  slot_data,
>  _vbmeta,
>  algorithm_type,
>  hashtree_error_mode);
> if (sub_ret != AVB_SLOT_VERIFY_RESULT_OK) {
> ret = sub_ret;
> goto fail;
> }

Indeed.

> > +   int i;
> > +   for (i = 0; i < (int)out_data->num_loaded_partitions; i++)
> > +   if (out_data->loaded_partitions[i].data)
> > +   free(out_data->loaded_partitions[i].data);
> 
> It's better to use avb_slot_verify_data_free()  for the whole out_data instead
> for proper cleanup. Check how it's done in unittests for libavb [2].

I'll check it out and offer a v2. Thanks for your feedback.

Regards,
Gary

[PATCH] cmd: avb: free partition buffer upon verify completion

[Gary Bisson]

Allows to run 'avb verify' multiple times which can be useful after a
failure to be able to re-flash the partition and try again.

Signed-off-by: Gary Bisson 
---
Hi,

This was added because of the following scenario:
1- fastboot flash boot boot.img
2- avb verify
  -> fails because vbmeta wasn't updated
3- fastboot flash vbmeta vbmeta.img
4- avb verify
  -> fails because it can't allocate memory as previous buffer wasn't
freed

Let me know if you have any questions.

Regards,
Gary
---
 cmd/avb.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/cmd/avb.c b/cmd/avb.c
index a4de5c40a2..67154de4e1 100644
--- a/cmd/avb.c
+++ b/cmd/avb.c
@@ -312,6 +312,14 @@ int do_avb_verify_part(cmd_tbl_t *cmdtp, int flag,
printf("Unknown error occurred\n");
}
 
+   /* Free image buffers now that verification is complete */
+   if (slot_result != AVB_SLOT_VERIFY_RESULT_ERROR_OOM) {
+   int i;
+   for (i = 0; i < (int)out_data->num_loaded_partitions; i++)
+   if (out_data->loaded_partitions[i].data)
+   free(out_data->loaded_partitions[i].data);
+   }
+
return res;
 }
 
-- 
2.26.2

[PATCH] fastboot: getvar: fix partition-size return value

[Gary Bisson]

The size returned by 'getvar partition-size' should be in bytes, not in
blocks as fastboot uses that value to generate empty partition when
running format [1].

[1]
https://android.googlesource.com/platform/system/core/+/refs/heads/android10-release/fastboot/fastboot.cpp#1500

Signed-off-by: Gary Bisson 
---
Hi,

Another test was to run 'fastboot getvar partition-size:system' on a
shipping Android phone, it will give you the size in bytes as well.

Regards,
Gary
---
 drivers/fastboot/fb_getvar.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/fastboot/fb_getvar.c b/drivers/fastboot/fb_getvar.c
index 95cb434189..51a2bea86d 100644
--- a/drivers/fastboot/fb_getvar.c
+++ b/drivers/fastboot/fb_getvar.c
@@ -94,7 +94,7 @@ static const struct {
  *
  * @param[in] part_name Info for which partition name to look for
  * @param[in,out] response Pointer to fastboot response buffer
- * @param[out] size If not NULL, will contain partition size (in blocks)
+ * @param[out] size If not NULL, will contain partition size
  * @return Partition number or negative value on error
  */
 static int getvar_get_part_info(const char *part_name, char *response,
@@ -108,13 +108,13 @@ static int getvar_get_part_info(const char *part_name, 
char *response,
r = fastboot_mmc_get_part_info(part_name, _desc, _info,
   response);
if (r >= 0 && size)
-   *size = part_info.size;
+   *size = part_info.size * part_info.blksz;
 # elif CONFIG_IS_ENABLED(FASTBOOT_FLASH_NAND)
struct part_info *part_info;
 
r = fastboot_nand_get_part_info(part_name, _info, response);
if (r >= 0 && size)
-   *size = part_info->size;
+   *size = part_info->size * part_info.blksz;
 # else
fastboot_fail("this storage is not supported in bootloader", response);
r = -ENODEV;
-- 
2.26.2

[U-Boot] [PATCH 2/2] imx: bootaux: fix stack and pc assignment on 64-bit platforms

[Gary Bisson]

Using ulong is wrong as its size depends on the Host CPU architecture
(32-bit vs. 64-bit) although the Cortex-M4 is always 32-bit.

Without this patch, the stack and PC are obviously wrong and it
generates an abort when used on 64-bit processors such as the i.MX8MQ.

Signed-off-by: Gary Bisson 
---
 arch/arm/mach-imx/imx_bootaux.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/imx_bootaux.c b/arch/arm/mach-imx/imx_bootaux.c
index a1ea5c13f1..3103001b7c 100644
--- a/arch/arm/mach-imx/imx_bootaux.c
+++ b/arch/arm/mach-imx/imx_bootaux.c
@@ -17,8 +17,8 @@ int arch_auxiliary_core_up(u32 core_id, ulong 
boot_private_data)
if (!boot_private_data)
return -EINVAL;
 
-   stack = *(ulong *)boot_private_data;
-   pc = *(ulong *)(boot_private_data + 4);
+   stack = *(u32 *)boot_private_data;
+   pc = *(u32 *)(boot_private_data + 4);
 
/* Set the stack and pc to M4 bootROM */
writel(stack, M4_BOOTROM_BASE_ADDR);
-- 
2.19.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 0/2] imx: fix M4 boot on i.MX8MQ processors

[Gary Bisson]

Hi,

This series fixes loading a M4 firmware into memory and start that M4
core using bootaux on i.MX8MQ platforms.

There were two issues:
1- the memory where the firmware is loaded (TCM) wasn't mapped
2- the bootaux code relied on ulong instead of u32 (M4 core is 32-bit)

This was tested on Nitrogen8M platform.

Regards,
Gary

Gary Bisson (2):
  imx: mx8m: add memory mapping for CAAM and TCM
  imx: bootaux: fix stack and pc assignment on 64-bit platforms

 arch/arm/mach-imx/imx_bootaux.c |  4 ++--
 arch/arm/mach-imx/mx8m/soc.c| 16 
 2 files changed, 18 insertions(+), 2 deletions(-)

-- 
2.19.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH 1/2] imx: mx8m: add memory mapping for CAAM and TCM

[Gary Bisson]

Otherwise can't boot the M4 core as it is impossible to load its
firmware into the TCM memory.

Signed-off-by: Gary Bisson 
---
 arch/arm/mach-imx/mx8m/soc.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/mach-imx/mx8m/soc.c b/arch/arm/mach-imx/mx8m/soc.c
index 46873aa8dd..11251c5f9a 100644
--- a/arch/arm/mach-imx/mx8m/soc.c
+++ b/arch/arm/mach-imx/mx8m/soc.c
@@ -77,6 +77,22 @@ static struct mm_region imx8m_mem_map[] = {
.size = 0x10UL,
.attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
 PTE_BLOCK_OUTER_SHARE
+   }, {
+   /* CAAM */
+   .virt = 0x10UL,
+   .phys = 0x10UL,
+   .size = 0x8000UL,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
+   }, {
+   /* TCM */
+   .virt = 0x7CUL,
+   .phys = 0x7CUL,
+   .size = 0x8UL,
+   .attrs = PTE_BLOCK_MEMTYPE(MT_DEVICE_NGNRNE) |
+PTE_BLOCK_NON_SHARE |
+PTE_BLOCK_PXN | PTE_BLOCK_UXN
}, {
/* OCRAM */
.virt = 0x90UL,
-- 
2.19.1

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH V5 2/2] imx6: sabrelite: update defconfig to use distro defaults

[Gary Bisson]

Hi Guillaume,

On Mon, Apr 16, 2018 at 02:47:38PM +0200, Guillaume GARDET wrote:
> Boot tested with boot.scr script and EFI/Grub2 on mmc0 and mmc1 slots on 
> sabrelite board.
> 
> Signed-off-by: Guillaume GARDET <guillaume.gar...@free.fr>
> Cc: Troy Kisky <troy.ki...@boundarydevices.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Fabio Estevam <fabio.este...@nxp.com>
> Cc: Gary Bisson <gary.bis...@boundarydevices.com>
> 
> ---
>  configs/mx6qsabrelite_defconfig | 15 +--
>  1 file changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/configs/mx6qsabrelite_defconfig b/configs/mx6qsabrelite_defconfig
> index 7499704058..fee33cfff1 100644
> --- a/configs/mx6qsabrelite_defconfig
> +++ b/configs/mx6qsabrelite_defconfig
> @@ -3,15 +3,15 @@ CONFIG_ARCH_MX6=y
>  CONFIG_SYS_TEXT_BASE=0x1780
>  CONFIG_TARGET_NITROGEN6X=y
>  CONFIG_CMD_HDMIDETECT=y
> +CONFIG_DISTRO_DEFAULTS=y
>  
> CONFIG_SYS_EXTRA_OPTIONS="IMX_CONFIG=board/boundary/nitrogen6x/nitrogen6q.cfg,MX6Q,DDR_MB=1024,SABRELITE"
>  CONFIG_BOOTDELAY=3
> +# CONFIG_USE_BOOTCOMMAND is not set
>  CONFIG_SYS_CONSOLE_OVERWRITE_ROUTINE=y
> -CONFIG_SUPPORT_RAW_INITRD=y
> +CONFIG_ARCH_MISC_INIT=y

The above config breaks the build since there's no arch_misc_init
function in the board file.

>  CONFIG_BOARD_EARLY_INIT_F=y
> -CONFIG_HUSH_PARSER=y
>  CONFIG_FASTBOOT=y
>  CONFIG_FASTBOOT_BUF_ADDR=0x1200
> -CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_MEMTEST=y
>  CONFIG_SYS_ALT_MEMTEST=y
>  # CONFIG_CMD_FLASH is not set

Also, can you add CMD_GPT so that GPT parition tables are supported?

Many of our OS images now rely on it.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH V5 1/2] imx6: Convert sabrelite and nitrogen6x boards to distro boot support

[Gary Bisson]

Hi Guillaume,

On Mon, Apr 16, 2018 at 02:47:37PM +0200, Guillaume GARDET wrote:
> Boot tested on sabrelite board.
> 
> Signed-off-by: Guillaume GARDET <guillaume.gar...@free.fr>
> Cc: Troy Kisky <troy.ki...@boundarydevices.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Fabio Estevam <fabio.este...@nxp.com>
> Cc: Gary Bisson <gary.bis...@boundarydevices.com>

Reviewed-by: Gary Bisson <gary.bis...@boundarydevices.com>

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH V4 1/2] imx6: Convert sabrelite and nitrogen6x boards to distro boot support

[Gary Bisson]

Hi Guillaume,

On Thu, Apr 12, 2018 at 03:28:21PM +0200, Guillaume GARDET wrote:
> Boot tested on sabrelite board.
> 
> Signed-off-by: Guillaume GARDET <guillaume.gar...@free.fr>
> Cc: Troy Kisky <troy.ki...@boundarydevices.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Fabio Estevam <fabio.este...@nxp.com>
> Cc: Gary Bisson <gary.bis...@boundarydevices.com>
> 
> ---
>  include/configs/nitrogen6x.h | 180 
> +--
>  1 file changed, 54 insertions(+), 126 deletions(-)
> 
> diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> index 7d2cf0bd8c..f6dce6c069 100644
> --- a/include/configs/nitrogen6x.h
> +++ b/include/configs/nitrogen6x.h
> @@ -101,140 +101,68 @@
>  #define CONFIG_DRIVE_TYPES CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC 
> CONFIG_DRIVE_USB
>  #define CONFIG_UMSDEVS CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC

Many of the macros above this line are obsolete with that patch, please
remove.

@@ -80,27 +80,6 @@

 #define CONFIG_PREBOOT ""

-#ifdef CONFIG_CMD_SATA
-#define CONFIG_DRIVE_SATA "sata "
-#else
-#define CONFIG_DRIVE_SATA
-#endif
-
-#ifdef CONFIG_CMD_MMC
-#define CONFIG_DRIVE_MMC "mmc "
-#else
-#define CONFIG_DRIVE_MMC
-#endif
-
-#ifdef CONFIG_USB_STORAGE
-#define CONFIG_DRIVE_USB "usb "
-#else
-#define CONFIG_DRIVE_USB
-#endif
-
-#define CONFIG_DRIVE_TYPES CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC CONFIG_DRIVE_USB
-#define CONFIG_UMSDEVS CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC
-
 #ifdef CONFIG_CMD_MMC

> +#ifdef CONFIG_CMD_MMC
> +#define DISTRO_BOOT_DEV_MMC(func) func(MMC, mmc, 0) func(MMC, mmc, 1)
> +#else
> +#define DISTRO_BOOT_DEV_MMC(func)
> +#endif
> +
> +#ifdef CONFIG_CMD_SATA
> +#define DISTRO_BOOT_DEV_SATA(func) func(SATA, sata, 0)
> +#else
> +#define DISTRO_BOOT_DEV_SATA(func)
> +#endif
> +
> +#ifdef CONFIG_USB_STORAGE
> +#define DISTRO_BOOT_DEV_USB(func) func(USB, usb, 0)
> +#else
> +#define DISTRO_BOOT_DEV_USB(func)
> +#endif
> +
> +#ifdef CONFIG_CMD_PXE
> +#define DISTRO_BOOT_DEV_PXE(func) func(PXE, pxe, na)
> +#else
> +#define DISTRO_BOOT_DEV_PXE(func)
> +#endif
> +
> +#ifdef CONFIG_CMD_DHCP
> +#define DISTRO_BOOT_DEV_DHCP(func) func(DHCP, dhcp, na)
> +#else
> +#define DISTRO_BOOT_DEV_DHCP(func)
> +#endif
> +
> +
>  #if defined(CONFIG_SABRELITE)
> +#define FDTFILE "fdtfile=imx6q-sabrelite.dtb\0"
> +#else
> +/* FIXME: nitrogen6x covers multiple configs. Define fdtfile for each 
> supported config. */
> +#define FDTFILE
> +#endif
> +
> +#define BOOT_TARGET_DEVICES(func) \
> + DISTRO_BOOT_DEV_MMC(func) \
> + DISTRO_BOOT_DEV_SATA(func) \
> + DISTRO_BOOT_DEV_USB(func) \
> + DISTRO_BOOT_DEV_PXE(func) \
> + DISTRO_BOOT_DEV_DHCP(func)
> +
> +#include 
> +
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> - "script=boot.scr\0" \
> - "uimage=uImage\0" \
>   "console=ttymxc1\0" \
>   "fdt_high=0x\0" \
>   "initrd_high=0x\0" \
> - "fdt_file=imx6q-sabrelite.dtb\0" \
> - "fdt_addr=0x1800\0" \
> - "boot_fdt=try\0" \
> + "fdt_addr_r=0x1800\0" \
> + FDTFILE \
> + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0"  \
> + "pxefile_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
> + "scriptaddr=" __stringify(CONFIG_LOADADDR) "\0" \
> + "ramdisk_addr_r=0x1300\0" \
> + "ramdiskaddr=0x1300\0" \
>   "ip_dyn=yes\0" \
>   "usb_pgood_delay=2000\0" \
> - "mmcdevs=0 1\0" \
> - "mmcpart=1\0" \
> - "mmcroot=/dev/mmcblk0p2 rootwait rw\0" \
> - "mmcargs=setenv bootargs console=${console},${baudrate} " \
> - "root=${mmcroot}\0" \
> - "loadbootscript=" \
> - "load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
> - "bootscript=echo Running bootscript from mmc ...; " \
> - "source\0" \
> - "loaduimage=load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
> - "loadfdt=load mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \
> - "mmcboot=echo Booting from mmc ...; " \
> - "run mmcargs; " \
> - "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> - "if run loadfdt; then " \
> - "bootm ${loadaddr} - ${fdt_addr}; " \
> - "else "

Re: [U-Boot] [PATCH V3 1/3] imx6: Define 'soc' env var for imx6 SoC

[Gary Bisson]

Hi Guillaume,

On Thu, Apr 12, 2018 at 02:48:07PM +0200, Guillaume Gardet wrote:
> 
> 
> Le 12/04/2018 à 14:36, Gary Bisson a écrit :
> > Hi Fabio, Guillaume
> > 
> > On Thu, Apr 12, 2018 at 08:14:51AM -0300, Fabio Estevam wrote:
> > > On Thu, Apr 12, 2018 at 5:13 AM, Guillaume Gardet
> > > <guillaume.gar...@free.fr> wrote:
> > > script that picks the correct dtb.
> > > > Ok. So, how would you like to proceed?
> > > > Remove the generic mx6 'soc' definition and use a board sepcific 
> > > > 'board_rev'
> > > > (or maybe a static definition if one configuration match a single dtb) 
> > > > to
> > > > define the right dtb?
> > > I prefer to keep the same solution as done in other boards sucn as
> > > mx6sabresd, wandboard and cuboxi.
> > Ok. My suggestion was to have something generic so that it matches the
> > fdt name set in config_distro_bootcmd.h
> > (${soc}-${board}${boardver}.dtb).
> > 
> > I especially like the naming in that standard, don't think board_rev
> > should be the SOC name.
> > 
> > This would also avoid redundancy in each board_late_init functions since
> > only the board name would need to be setup (and boardver in some cases
> > like wandboard).
> > 
> > Guillaume, please forget that point for now and just set your fdtfile
> > name in the config file. I'll update the board file later on.
> 
> Ok.
> 
> Could you just confirm which DTB should work with the various nitrogen6x 
> boards config, please?
> Are the following ok?
> * imx6q-nitrogen6x.dts => nitrogen6q2g_defconfig nitrogen6q_defconfig
> * imx6dl-nitrogen6x.dts     =>     nitrogen6dl2g_defconfig 
> nitrogen6dl_defconfig
> * imx6sx-nitrogen6sx.dts => nitrogen6s1g_defconfig 
> nitrogen6s_defconfig

No, imx6sx is different than imx6s. Solo is the same as imx6dl as far as
the kernl is concerned.

Maybe just leave fdt_file blank for nitrogen6x for now, it's only necessary
for PXE which isn't enabled in nitrogen6*defconfig.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH V3 1/3] imx6: Define 'soc' env var for imx6 SoC

[Gary Bisson]

Hi Fabio, Guillaume

On Thu, Apr 12, 2018 at 08:14:51AM -0300, Fabio Estevam wrote:
> On Thu, Apr 12, 2018 at 5:13 AM, Guillaume Gardet
>  wrote:
> script that picks the correct dtb.
> >>
> > Ok. So, how would you like to proceed?
> > Remove the generic mx6 'soc' definition and use a board sepcific 'board_rev'
> > (or maybe a static definition if one configuration match a single dtb) to
> > define the right dtb?
> 
> I prefer to keep the same solution as done in other boards sucn as
> mx6sabresd, wandboard and cuboxi.

Ok. My suggestion was to have something generic so that it matches the
fdt name set in config_distro_bootcmd.h
(${soc}-${board}${boardver}.dtb).

I especially like the naming in that standard, don't think board_rev
should be the SOC name.

This would also avoid redundancy in each board_late_init functions since
only the board name would need to be setup (and boardver in some cases
like wandboard).

Guillaume, please forget that point for now and just set your fdtfile
name in the config file. I'll update the board file later on.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH V3 1/3] imx6: Define 'soc' env var for imx6 SoC

[Gary Bisson]

Hi Guillaume,

On Wed, Apr 11, 2018 at 12:38:48PM +0200, Guillaume GARDET wrote:
> Signed-off-by: Guillaume GARDET <guillaume.gar...@free.fr>
> Cc: Troy Kisky <troy.ki...@boundarydevices.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Fabio Estevam <fabio.este...@nxp.com>
> Cc: Gary Bisson <gary.bis...@boundarydevices.com>
> 
> ---
>  arch/arm/mach-imx/mx6/soc.c | 34 ++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> index 9b3d8f69b2..c4cb752c76 100644
> --- a/arch/arm/mach-imx/mx6/soc.c
> +++ b/arch/arm/mach-imx/mx6/soc.c
> @@ -446,6 +446,40 @@ int arch_cpu_init(void)
>   return 0;
>  }
>  
> + #ifdef CONFIG_ARCH_MISC_INIT
> + int arch_misc_init(void)
> + {
> + #ifdef CONFIG_ENV_VARS_UBOOT_CONFIG
> + if (is_cpu_type(MXC_CPU_MX6QP))
> + env_set("soc", "imx6qp");
> + else if (is_cpu_type(MXC_CPU_MX6Q))
> + env_set("soc", "imx6q");
> + else if (is_cpu_type(MXC_CPU_MX6DP))
> + env_set("soc", "imx6dp");

If we want that soc variable to be used for dtb names, then the above
won't work. A i.MX6DP platform has its dtb named imx6qp-board.dtb.

> + else if (is_cpu_type(MXC_CPU_MX6D))
> + env_set("soc", "imx6d");

Same here, a Dual CPU actually uses a imx6q-board.dtb.

> + else if (is_mx6dl( ))
> + env_set("soc", "imx6dl");
> + else if (is_mx6sx( ))
> + env_set("soc", "imx6sx");
> + else if (is_mx6sl( ))
> + env_set("soc", "imx6sl");
> + else if (is_mx6solo( ))
> + env_set("soc", "imx6solo");

Same here, a Solo CPU uses a imx6dl-board.dtb.

> + else if (is_mx6ul( ))
> + env_set("soc", "imx6ul");
> + else if (is_mx6ull( ))
> + env_set("soc", "imx6ull");
> + else if (is_mx6sll( ))
> + env_set("soc", "imx6sll");
> + else
> + env_set("soc", "imx6");

In that case we most likely miss a CPU definition, maybe "unknown" would
be more explicit?

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] imx: nitrogen6x: Convert Sabrelite to distro boot support

[Gary Bisson]

Hi Fabio,

On Mon, Apr 09, 2018 at 02:27:31PM -0300, Fabio Estevam wrote:
> Hi Gary,
> 
> On Mon, Apr 9, 2018 at 11:40 AM, Gary Bisson
> <gary.bis...@boundarydevices.com> wrote:
> 
> >>  #define CONFIG_EXTRA_ENV_SETTINGS \
> >> - "script=boot.scr\0" \
> >> - "uimage=uImage\0" \
> >>   "console=ttymxc1\0" \
> >>   "fdt_high=0x\0" \
> >>   "initrd_high=0x\0" \
> >> - "fdt_file=imx6q-sabrelite.dtb\0" \
> >> + "fdtfile=imx6q-sabrelite.dtb\0" \
> >
> > I wish the default efi_fdtfile would work, Fabio is there any plan to
> > populate the ${soc} variable like it is done for i.MX7? [2]
> 
> It seems we didn't need to do this yet, but if you see a need for it,
> please post a patch.

Will do.

Thanks,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] imx: nitrogen6x: Convert Sabrelite to distro boot support

[Gary Bisson]

Hi Guillaume,

On Mon, Apr 09, 2018 at 05:17:44PM +0200, Guillaume Gardet wrote:
> 
> 
> Le 09/04/2018 à 16:40, Gary Bisson a écrit :
> > Hi Guillaume,
> > 
> > Thanks for your patch! Switching the upstream nitrogen6x configuration
> > to distro bootcmd has been in our todo list for some time since we also
> > use that in our own git repo.
> > 
> > On Fri, Apr 06, 2018 at 12:05:48PM +0200, Guillaume GARDET wrote:
> > > Boot tested with boot.scr script and EFI/Grub2 on mmc0 and mmc1 slots.
> > > 
> > > Signed-off-by: Guillaume GARDET 
> > > Cc: Troy Kisky 
> > > Cc: Stefano Babic 
> > > Cc: Fabio Estevam 
> > > ---
> > >   include/configs/nitrogen6x.h | 86 
> > > ++--
> > >   1 file changed, 18 insertions(+), 68 deletions(-)
> > > 
> > > diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> > > index c73cfb7f7e..2e2c499cb6 100644
> > > --- a/include/configs/nitrogen6x.h
> > > +++ b/include/configs/nitrogen6x.h
> > > @@ -102,82 +102,32 @@
> > >   #define CONFIG_UMSDEVS CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC
> > >   #if defined(CONFIG_SABRELITE)
> > Please also switch nitrogen6x to it, not only sabrelite.
> 
> Should we keep all the 6x scripts stuff or not?

No, we need to switch to something standard, let's remove it.

> > > +#define BOOT_TARGET_DEVICES(func) \
> > > + func(MMC, mmc, 0) \
> > > + func(MMC, mmc, 1) \
> > > + func(SATA, sata, 0) \
> > > + func(USB, usb, 0) \
> > > + func(PXE, pxe, na) \
> > That currently doesn't build because CMD_PXE isn't selected in the
> > sabrelite defconfig.
> > "include/config_distro_bootcmd.h:319:2: error: expected ‘}’ before
> > ‘BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE’"
> > 
> > Which brings a good point, all the above should depend on the storage
> > command being present, like done here [1].
> 
> No, we just need to update the defconfig, see V2 of this patch sent earlier 
> today.

I know updating the defconfig makes it build. My point is that it would
be more flexible to do like in the link provided so that someone that
removes CMD_PXE from the defconfig for any reason can still build.

> > > + func(DHCP, dhcp, na)
> > > +
> > > +#include 
> > > +
> > >   #define CONFIG_EXTRA_ENV_SETTINGS \
> > > - "script=boot.scr\0" \
> > > - "uimage=uImage\0" \
> > >   "console=ttymxc1\0" \
> > >   "fdt_high=0x\0" \
> > >   "initrd_high=0x\0" \
> > > - "fdt_file=imx6q-sabrelite.dtb\0" \
> > > + "fdtfile=imx6q-sabrelite.dtb\0" \
> > I wish the default efi_fdtfile would work, Fabio is there any plan to
> > populate the ${soc} variable like it is done for i.MX7? [2]
> > Then a small patch in the nitrogen6x.c would set the proper board env
> > variable.
> > 
> > That way there would be no difference in this header between sabrelite
> > vs. nitrogen6x.
> > 
> > >   "fdt_addr=0x1800\0" \
> > Do we need to keep fdt_addr since we define fdt_addr_r below?
> 
> It was for backward compatibility, if people had some script using it.

I think it should be dropped.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH] imx: nitrogen6x: Convert Sabrelite to distro boot support

[Gary Bisson]

Hi Guillaume,

Thanks for your patch! Switching the upstream nitrogen6x configuration
to distro bootcmd has been in our todo list for some time since we also
use that in our own git repo.

On Fri, Apr 06, 2018 at 12:05:48PM +0200, Guillaume GARDET wrote:
> Boot tested with boot.scr script and EFI/Grub2 on mmc0 and mmc1 slots.
> 
> Signed-off-by: Guillaume GARDET 
> Cc: Troy Kisky 
> Cc: Stefano Babic 
> Cc: Fabio Estevam 
> ---
>  include/configs/nitrogen6x.h | 86 
> ++--
>  1 file changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> index c73cfb7f7e..2e2c499cb6 100644
> --- a/include/configs/nitrogen6x.h
> +++ b/include/configs/nitrogen6x.h
> @@ -102,82 +102,32 @@
>  #define CONFIG_UMSDEVS CONFIG_DRIVE_SATA CONFIG_DRIVE_MMC
>  
>  #if defined(CONFIG_SABRELITE)

Please also switch nitrogen6x to it, not only sabrelite.

> +#define BOOT_TARGET_DEVICES(func) \
> + func(MMC, mmc, 0) \
> + func(MMC, mmc, 1) \
> + func(SATA, sata, 0) \
> + func(USB, usb, 0) \
> + func(PXE, pxe, na) \

That currently doesn't build because CMD_PXE isn't selected in the
sabrelite defconfig.
"include/config_distro_bootcmd.h:319:2: error: expected ‘}’ before
‘BOOT_TARGET_DEVICES_references_PXE_without_CONFIG_CMD_DHCP_or_PXE’"

Which brings a good point, all the above should depend on the storage
command being present, like done here [1].

> + func(DHCP, dhcp, na)
> +
> +#include 
> +
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> - "script=boot.scr\0" \
> - "uimage=uImage\0" \
>   "console=ttymxc1\0" \
>   "fdt_high=0x\0" \
>   "initrd_high=0x\0" \
> - "fdt_file=imx6q-sabrelite.dtb\0" \
> + "fdtfile=imx6q-sabrelite.dtb\0" \

I wish the default efi_fdtfile would work, Fabio is there any plan to
populate the ${soc} variable like it is done for i.MX7? [2]
Then a small patch in the nitrogen6x.c would set the proper board env
variable.

That way there would be no difference in this header between sabrelite
vs. nitrogen6x.

>   "fdt_addr=0x1800\0" \

Do we need to keep fdt_addr since we define fdt_addr_r below?

> - "boot_fdt=try\0" \
> + "fdt_addr_r=0x1800\0" \
> + "kernel_addr_r=" __stringify(CONFIG_LOADADDR) "\0"  \
> + "pxefile_addr_r=" __stringify(CONFIG_LOADADDR) "\0" \
> + "scriptaddr=" __stringify(CONFIG_LOADADDR) "\0" \
> + "ramdisk_addr_r=0x1300\0" \
> + "ramdiskaddr=0x1300\0" \
>   "ip_dyn=yes\0" \
>   "usb_pgood_delay=2000\0" \
> - "mmcdevs=0 1\0" \
> - "mmcpart=1\0" \
> - "mmcroot=/dev/mmcblk0p2 rootwait rw\0" \
> - "mmcargs=setenv bootargs console=${console},${baudrate} " \
> - "root=${mmcroot}\0" \
> - "loadbootscript=" \
> - "load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${script};\0" \
> - "bootscript=echo Running bootscript from mmc ...; " \
> - "source\0" \
> - "loaduimage=load mmc ${mmcdev}:${mmcpart} ${loadaddr} ${uimage}\0" \
> - "loadfdt=load mmc ${mmcdev}:${mmcpart} ${fdt_addr} ${fdt_file}\0" \
> - "mmcboot=echo Booting from mmc ...; " \
> - "run mmcargs; " \
> - "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> - "if run loadfdt; then " \
> - "bootm ${loadaddr} - ${fdt_addr}; " \
> - "else " \
> - "if test ${boot_fdt} = try; then " \
> - "bootm; " \
> - "else " \
> - "echo WARN: Cannot load the DT; " \
> - "fi; " \
> - "fi; " \
> - "else " \
> - "bootm; " \
> - "fi;\0" \
> - "netargs=setenv bootargs console=${console},${baudrate} " \
> - "root=/dev/nfs " \
> - "ip=dhcp nfsroot=${serverip}:${nfsroot},v3,tcp\0" \
> - "netboot=echo Booting from net ...; " \
> - "run netargs; " \
> - "if test ${ip_dyn} = yes; then " \
> - "setenv get_cmd dhcp; " \
> - "else " \
> - "setenv get_cmd tftp; " \
> - "fi; " \
> - "${get_cmd} ${uimage}; " \
> - "if test ${boot_fdt} = yes || test ${boot_fdt} = try; then " \
> - "if ${get_cmd} ${fdt_addr} ${fdt_file}; then " \
> - "bootm ${loadaddr} - ${fdt_addr}; " \
> - "else " \
> - "if test ${boot_fdt} = try; then " \
> - "bootm; " \
> - "else " \
> - "echo WARN: Cannot load the DT; " \
> - "fi; " \
> - "fi; " \
> - "else " \
> - "bootm; " \
> - "fi;\0"
> 

[U-Boot] [PATCH] arm: cache: exit maintenance functions when cache disabled

[Gary Bisson]

No need to check the region range and send commands when the cache
isn't even enabled.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
Hi all,

This is a follow-up to this thread:
https://lists.denx.de/pipermail/u-boot/2017-March/283423.html

Although what started the conversation was the sparse-image flashing
procedure, it appears cache maintenance functions don't check on cache
status.

Regards,
Gary
---
 arch/arm/cpu/armv7/cache_v7.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
index c4bbcc3cc3..992cdeaa6e 100644
--- a/arch/arm/cpu/armv7/cache_v7.c
+++ b/arch/arm/cpu/armv7/cache_v7.c
@@ -117,6 +117,9 @@ void flush_dcache_all(void)
  */
 void invalidate_dcache_range(unsigned long start, unsigned long stop)
 {
+   if (!dcache_status())
+   return;
+
check_cache_range(start, stop);
 
v7_dcache_maint_range(start, stop, ARMV7_DCACHE_INVAL_RANGE);
@@ -131,6 +134,9 @@ void invalidate_dcache_range(unsigned long start, unsigned 
long stop)
  */
 void flush_dcache_range(unsigned long start, unsigned long stop)
 {
+   if (!dcache_status())
+   return;
+
check_cache_range(start, stop);
 
v7_dcache_maint_range(start, stop, ARMV7_DCACHE_CLEAN_INVAL_RANGE);
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [ PATCH] ARM: cache: runtime flag to suppress the "cache misaligned" message

[Gary Bisson]

Hi Marek, All,

On Tue, Mar 07, 2017 at 03:52:23AM +0100, Marek Vasut wrote:
> On 03/06/2017 11:21 PM, Steve Rae wrote:
> > The "chunks" in the "fastboot sparse image" are not aligned,
> > resulting in many "cached misaligned" messages from
> > check_cache_range(). Implement a runtime flag to suppress this
> > message, and use this flag when processing the "fastboot sparse
> > image".
> 
> Let me understand this, what we add here is a patch to silence a warning
> which correctly indicates something totally wrong is happening instead
> of fixing that problem ?
> 
> From me, that is a NAK and if my understanding of this is correct, this
> patch will go in only over my dead body.
> 
> > Signed-off-by: Steve Rae <steve@raedomain.com> Reported-by: Gary
> > Bisson <gary.bis...@boundarydevices.com>
> > 
> > --- A little background: In the code that I am testing, the "sparse"
> > image is 350,723,024 bytes and is downloaded into an aligned buffer
> > on the device. This image/buffer contains a header ('a') and 13903
> > "chunks" (b1/c1 through bx/cx):
> 
> OT: Can you please fix your mailer to break lines at 80 chars ?
> I reflowed the mail and the ascii art turned crap.
> 
> > +---+++++ +++ | a | b1 | c1 | b2 | c2 |
> > ... | bx | cx | +---+++++ +++ where: a  =
> > the "sparse image" header (28 bytes) b* = the "chunk" header
> > (12 bytes) c* = the "chunk" data  (which is always an exact
> > multiple of CONFIG_SYS_CACHELINE_SIZE) Whenever the "chunk" type is
> > CHUNK_TYPE_RAW, then the data in the current 'c' (the "chunk" data)
> > needs to be written into the flash as is, using the pointer to the
> > first byte of this 'c' as the starting address for the blk_dwrite().
> > Because of the 28 byte image header, and the 12 byte "chunk"
> > header(s), it is extremely unlikely that this starting address will
> > be aligned - resulting in many "CACHE: Misaligned operation at range
> > [, ]" messages while these 13093 "chunks" are processed. In
> > the code that I am testing, this message is being generated by the
> > call to flush_cache(start_addr, trans_bytes) in the
> > sdhci_send_command() function.
> 
> You can set this up such that the [a] part starts at offset +24 bytes, then
> the [c] will be aligned.

This will align the first chunk but there's no guarantee the next one
will be aligned.

> > Copying each "chuck data" to an aligned buffer before calling
> > blk_dwrite() in order to avoid this message would be highly
> > inefficient -- the desire is to download the code and flash it as
> > fast as possible!
> 
> Fast is great, but not if this makes the transfer unreliable, which will
> happen if you cannot reliably flush/invalidate cache and you decide to
> ignore this clear warning you're getting.

Actually fast isn't the problem, it is more than we can't afford to copy
each chunk to an aligned buffer because of size constraints. Although we
could break down "big" chunks to smaller one I guess.

> > Therefore, I propose this patch which provides a runtime flag to
> > suppress this warning message, which is used when processing these
> > fastboot "sparse" images.
> 
> This is wrong.

Yes I agree that hiding the warning isn't solving anything. However I
expected that disabling dcache would remove the warning but it doesn't.

Shouldn't the cache maintenance functions like flush_dcache_range()
check the dcache_status() before doing anything?

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] arm: cache: misaligned operation with fastboot

[Gary Bisson]

Hi Steve,

On Fri, Mar 3, 2017 at 12:03 AM, Steve Rae <steve@raedomain.com> wrote:
>
> Hi Gary,
>
> On Thu, Mar 2, 2017 at 3:12 AM, Lukasz Majewski <lu...@denx.de> wrote:
>>
>> Hi,
>>
>> > Hi Fabio, Lukasz,
>> >
>> > On Wed, Feb 15, 2017 at 02:24:40PM -0200, Fabio Estevam wrote:
>> > > On Wed, Feb 15, 2017 at 2:04 PM, Gary Bisson
>> > > <gary.bis...@boundarydevices.com> wrote:
>> > > > Hi,
>> > > >
>> > > > I've been testing fastboot to flash a sparse image on a i.MX6Q
>> > > > platform (Nitrogen6x) with U-Boot v2017.01.
>> > > >
>> > > > This test shows a lot of "misaligned operation" traces:
>> > > > => fastboot 0
>> > > > Starting download of 415679660 bytes
>> > > > ...
>> > > > downloading of 415679660 bytes finished
>> > > > Flashing sparse image at offset 82124
>> > > > Flashing Sparse Image
>> > > > CACHE: Misaligned operation at range [1228, 12028028]
>> > > > CACHE: Misaligned operation at range [12028044, 1208f044]
>> > > > CACHE: Misaligned operation at range [1208f060, 123fd060]
>> > > > ...
>> > > >
>> > > > Has any of you seen that behavior?
>> > > >
>> > > > Note that this behavior only happens for sparse images, flashing
>> > > > a raw image doesn't exhibit the problem.
> 
> yes -- I have seen these warning messages when flashing a "sparse" image
> - they don't seem to cause any issues,

True, the flashing process is successful. But avoiding those warnings
would provide a better user experience. We've had customers asking for
it since to them it looks like something isn't copied properly.

> - I haven't had time to investigate yet

Well the whole image is downloaded to an aligned address which is
fine. But then the header is skipped which gives a non-aligned data
address for the write command:
http://git.denx.de/?p=u-boot.git;a=blob;f=common/image-sparse.c;hb=HEAD#l153

Here is the backtrace when that happens (not sure if it helps):
http://pastebin.com/Qun8uMXq

Not sure what would be the proper way to fix, copy the buffer to align
it seems inefficient.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

Re: [U-Boot] arm: cache: misaligned operation with fastboot

[Gary Bisson]

Hi Fabio, Lukasz,

On Wed, Feb 15, 2017 at 02:24:40PM -0200, Fabio Estevam wrote:
> On Wed, Feb 15, 2017 at 2:04 PM, Gary Bisson
> <gary.bis...@boundarydevices.com> wrote:
> > Hi,
> >
> > I've been testing fastboot to flash a sparse image on a i.MX6Q platform
> > (Nitrogen6x) with U-Boot v2017.01.
> >
> > This test shows a lot of "misaligned operation" traces:
> > => fastboot 0
> > Starting download of 415679660 bytes
> > ...
> > downloading of 415679660 bytes finished
> > Flashing sparse image at offset 82124
> > Flashing Sparse Image
> > CACHE: Misaligned operation at range [1228, 12028028]
> > CACHE: Misaligned operation at range [12028044, 1208f044]
> > CACHE: Misaligned operation at range [1208f060, 123fd060]
> > ...
> >
> > Has any of you seen that behavior?
> >
> > Note that this behavior only happens for sparse images, flashing a raw
> > image doesn't exhibit the problem.
> 
> Adding DFU maintainer, Lukasz on Cc.

Any update on this? Has anyone been able to reproduce?

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/listinfo/u-boot

[U-Boot] arm: cache: misaligned operation with fastboot

[Gary Bisson]

Hi,

I've been testing fastboot to flash a sparse image on a i.MX6Q platform
(Nitrogen6x) with U-Boot v2017.01.

This test shows a lot of "misaligned operation" traces:
=> fastboot 0
Starting download of 415679660 bytes
...
downloading of 415679660 bytes finished
Flashing sparse image at offset 82124
Flashing Sparse Image
CACHE: Misaligned operation at range [1228, 12028028]
CACHE: Misaligned operation at range [12028044, 1208f044]
CACHE: Misaligned operation at range [1208f060, 123fd060]
...

Has any of you seen that behavior?

Note that this behavior only happens for sparse images, flashing a raw
image doesn't exhibit the problem.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] imx: nitrogen6x: fix USB host initialization

[Gary Bisson]

USB Host scanning has been broken since v2016.05.

This is due to all the USB changes that happened between v2016.03
and v2016.05, especially:
2ef117fe4f usb: Remove 200 ms delay in usb_hub_port_connect_change()
a22a264ec3 usb: Change power-on / scanning timeout handling

So we need to increase the init delay to 2s using the usb_pgood_delay
environment variable.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 include/configs/nitrogen6x.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
index c22fed70a4..72ff5c1eab 100644
--- a/include/configs/nitrogen6x.h
+++ b/include/configs/nitrogen6x.h
@@ -137,6 +137,7 @@
"fdt_addr=0x1800\0" \
"boot_fdt=try\0" \
"ip_dyn=yes\0" \
+   "usb_pgood_delay=2000\0" \
"mmcdevs=0 1\0" \
"mmcpart=1\0" \
"mmcroot=/dev/mmcblk0p2 rootwait rw\0" \
@@ -206,6 +207,7 @@
 #define CONFIG_EXTRA_ENV_SETTINGS \
"bootdevs=" CONFIG_DRIVE_TYPES "\0" \
"umsdevs=" CONFIG_UMSDEVS "\0" \
+   "usb_pgood_delay=2000\0" \
"console=ttymxc1\0" \
"clearenv=if sf probe || sf probe || sf probe 1 ; then " \
"sf erase 0xc 0x2000 && " \
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] cmd: sata: fix init command return value

[Gary Bisson]

Since commit aa6ab905b2, sata_initialize returns -1 if init_sata or
scan_sata fails. But this return value becomes the do_sata return
value which is equivalent to CMD_RET_USAGE.

In case one issues 'sata init' and that the hardware fails to
initialize, there's no need to display the command usage. Instead
the command shoud just return the CMD_RET_FAILURE value.

Fixes: aa6ab905b2 (sata: fix sata command can not being executed bug)

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 cmd/sata.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/sata.c b/cmd/sata.c
index f56622acc2..4c53022ff6 100644
--- a/cmd/sata.c
+++ b/cmd/sata.c
@@ -28,14 +28,15 @@ static int do_sata(cmd_tbl_t *cmdtp, int flag, int argc, 
char * const argv[])
if (sata_curr_device != -1)
sata_stop();
 
-   return sata_initialize();
+   return (sata_initialize() < 0) ?
+   CMD_RET_FAILURE : CMD_RET_SUCCESS;
}
 
/* If the user has not yet run `sata init`, do it now */
if (sata_curr_device == -1) {
rc = sata_initialize();
if (rc == -1)
-   return rc;
+   return CMD_RET_FAILURE;
sata_curr_device = rc;
}
 
-- 
2.11.0

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] README: mxc_hab: Adapt the CONFIG_SECURE_BOOT text to Kconfig

[Gary Bisson]

Hi Fabio

On Thu, Jan 05, 2017 at 09:33:08PM -0200, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.este...@nxp.com>
> 
> Commit 6e1f4d2652e79 ("arm: imx-common: add SECURE_BOOT option to
> Kconfig") moved the CONFIG_SECURE_BOOT option to Kconfig, so update
> the mxc_hab README file to reflect that.
> 
> Signed-off-by: Fabio Estevam <fabio.este...@nxp.com>

Good catch, completely forgot about the README.

Reviewed-by: Gary Bisson <gary.bis...@boundarydevices.com>

Thanks,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] tools: imximage: display DCD block offset, length

[Gary Bisson]

Hi Eric, All,

On Wed, Nov 16, 2016 at 05:13:41PM -0700, Eric Nelson wrote:
> These values can be used to sign a U-Boot image for use when
> loading an image through the Serial Download Protocol (SDP).
> 
> Note that the address of 0x91 is usable with the stock
> configuration of imx_usb_loader on i.MX6 and i.MX7 SOCs:
> 
> https://github.com/boundarydevices/imx_usb_loader/blob/master/mx6_usb_work.conf#L3
> 
> Refer to the section on imx_usb_loader in this post for more
> details:
> 
> https://boundarydevices.com/high-assurance-boot-hab-dummies/
> 
> Signed-off-by: Eric Nelson 

Thanks, indeed such patch would ease the life of anybody that needs to
deal with HAB when creating the CSF files.

> ---
>  tools/imximage.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/imximage.c b/tools/imximage.c
> index c9e42ec..2cd8d88 100644
> --- a/tools/imximage.c
> +++ b/tools/imximage.c
> @@ -281,7 +281,6 @@ static void set_dcd_rst_v2(struct imx_header *imxhdr, 
> uint32_t dcd_len,
>   d = (struct dcd_v2_cmd *)(((char *)d) + len);
>  
>   len = (char *)d - (char *)_v2->header;
> -

Is this part of the patch intended?

>   dcd_v2->header.tag = DCD_HEADER_TAG;
>   dcd_v2->header.length = cpu_to_be16(len);
>   dcd_v2->header.version = DCD_VERSION;
> @@ -501,10 +500,19 @@ static void print_hdr_v2(struct imx_header *imx_hdr)
>   printf("Entry Point:  %08x\n", (uint32_t)fhdr_v2->entry);
>   if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
>   (imximage_csf_size != UNDEFINED)) {
> + uint16_t dcdlen;
> + int offs;
> +
> + dcdlen = hdr_v2->data.dcd_table.header.length;
> + offs = (char *)_v2->data.dcd_table
> + - (char *)hdr_v2;
> +
>   printf("HAB Blocks:   %08x %08x %08x\n",
>  (uint32_t)fhdr_v2->self, 0,

This isn't part of the patch, but why is self cast into a uint32_t
although it's already a uint32_t?

>  hdr_v2->boot_data.size - imximage_ivt_offset -
>  imximage_csf_size);
> + printf("DCD Blocks:   0091 %08x %08x\n",
> +offs, be16_to_cpu(dcdlen));
>   }

Not sure if "DCD Blocks" is the best naming, cause it really just
applies to SDP protocol.

This got me thinking and I think the printf above should also show the
Blocks for encryption which is also missing right now.

What about something like the snippet below?

if (fhdr_v2->csf && (imximage_ivt_offset != UNDEFINED) &&
(imximage_csf_size != UNDEFINED)) {
uint16_t dcdlen;
uint32_t dcdoff;
uint32_t entryoff;

dcdlen = hdr_v2->data.dcd_table.header.length;
dcdoff = (char *)_v2->data.dcd_table
- (char *)hdr_v2;
entryoff = fhdr_v2->entry - fhdr_v2->self;

printf("[HAB][Signature]\n");
printf("Blocks:   %08x %08x %08x\n",
   (uint32_t)fhdr_v2->self, 0,
   hdr_v2->boot_data.size - imximage_ivt_offset -
   imximage_csf_size);
printf("[HAB][Encryption]\n");
printf("Blocks:   %08x %08x %08x\n",
   fhdr_v2->self, 0, dcdoff + be16_to_cpu(dcdlen));
printf("Blocks:   %08x %08x %08x\n",
   fhdr_v2->entry, entryoff,
   hdr_v2->boot_data.size - imximage_ivt_offset -
   imximage_csf_size - entryoff);
printf("[HAB][SDP]\n");
printf("Blocks:   0091 %08x %08x\n",
   dcdoff, be16_to_cpu(dcdlen));
}

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 4/5] mx7_common: add secure boot support

[Gary Bisson]

Selecting the proper options to enable the build of the HAB tools.

Note, this support is disabled by default, one will have to select
the SECURE_BOOT configuration through menuconfig to enable it.

See doc/README.mxc_hab for more details.

Also remove duplicate options from board config headers.

Cc: Stefan Agner <stefan.ag...@toradex.com>
Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 include/configs/colibri_imx7.h | 4 
 include/configs/mx7_common.h   | 9 +
 include/configs/mx7dsabresd.h  | 4 
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/configs/colibri_imx7.h b/include/configs/colibri_imx7.h
index 9da219c..486f2a4 100644
--- a/include/configs/colibri_imx7.h
+++ b/include/configs/colibri_imx7.h
@@ -29,10 +29,6 @@
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN  (32 * SZ_1M)
 
-/* Uncomment to enable secure boot support */
-/* #define CONFIG_SECURE_BOOT */
-#define CONFIG_CSF_SIZE0x4000
-
 #define CONFIG_CMD_BMODE
 
 /* Network */
diff --git a/include/configs/mx7_common.h b/include/configs/mx7_common.h
index 7295fa6..c339e24 100644
--- a/include/configs/mx7_common.h
+++ b/include/configs/mx7_common.h
@@ -76,4 +76,13 @@
 #define CONFIG_ARMV7_PSCI_NR_CPUS  2
 #define CONFIG_ARMV7_SECURE_BASE   0x0090
 
+/* Secure boot (HAB) support */
+#ifdef CONFIG_SECURE_BOOT
+#define CONFIG_CSF_SIZE0x2000
+#define CONFIG_SYS_FSL_SEC_COMPAT  4
+#define CONFIG_FSL_CAAM
+#define CONFIG_CMD_DEKBLOB
+#define CONFIG_SYS_FSL_SEC_LE
+#endif
+
 #endif
diff --git a/include/configs/mx7dsabresd.h b/include/configs/mx7dsabresd.h
index 822d81f..f2d5dea 100644
--- a/include/configs/mx7dsabresd.h
+++ b/include/configs/mx7dsabresd.h
@@ -24,10 +24,6 @@
 
 #define CONFIG_DISPLAY_BOARDINFO
 
-/* Uncomment to enable secure boot support */
-/* #define CONFIG_SECURE_BOOT */
-#define CONFIG_CSF_SIZE0x4000
-
 /* Network */
 #define CONFIG_FEC_MXC
 #define CONFIG_MII
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 5/5] nitrogen6x: add secure boot support

[Gary Bisson]

Declaring a CSF section makes the imximage tool increase the size of
data to be loaded by the BootROM and also adds a pointer to that CSF
section in the IVT header to the BootROM can check the signature.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 board/boundary/nitrogen6x/nitrogen6dl.cfg   | 3 +++
 board/boundary/nitrogen6x/nitrogen6dl2g.cfg | 3 +++
 board/boundary/nitrogen6x/nitrogen6q.cfg| 3 +++
 board/boundary/nitrogen6x/nitrogen6q2g.cfg  | 3 +++
 board/boundary/nitrogen6x/nitrogen6s.cfg| 3 +++
 board/boundary/nitrogen6x/nitrogen6s1g.cfg  | 3 +++
 6 files changed, 18 insertions(+)

diff --git a/board/boundary/nitrogen6x/nitrogen6dl.cfg 
b/board/boundary/nitrogen6x/nitrogen6dl.cfg
index 1cdccad..5c3e961 100644
--- a/board/boundary/nitrogen6x/nitrogen6dl.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6dl.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg 
b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
index 516d67e..fe19ed0 100644
--- a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6q.cfg 
b/board/boundary/nitrogen6x/nitrogen6q.cfg
index b6642e6..60e1885 100644
--- a/board/boundary/nitrogen6x/nitrogen6q.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6q.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6q2g.cfg 
b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
index fe6dfc1..7a3ee94 100644
--- a/board/boundary/nitrogen6x/nitrogen6q2g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6s.cfg 
b/board/boundary/nitrogen6x/nitrogen6s.cfg
index ca30cd6..2540b7b 100644
--- a/board/boundary/nitrogen6x/nitrogen6s.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6s.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6s1g.cfg 
b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
index b1489fb..946af7b 100644
--- a/board/boundary/nitrogen6x/nitrogen6s1g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 2/5] arm: imx-common: introduce back usec2ticks

[Gary Bisson]

From: Peng Fan <van.free...@gmail.com>

This commit "2bb014820c49a63902103bac710bc86b5772e843"
do some clean up to use the code in lib/time.c.
But usec2ticks is still being used by security related job ring code.
Bring back the function to avoid build break when CONFIG_FSL_CAAM
is defined.
The computation logic has been changed, using 64-bit variable
to ease the process, making it work on older (MX5) platforms.

Signed-off-by: Peng Fan <van.free...@gmail.com>
Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com>
Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 arch/arm/imx-common/timer.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
index a01590c..a04c7ae 100644
--- a/arch/arm/imx-common/timer.c
+++ b/arch/arm/imx-common/timer.c
@@ -119,3 +119,19 @@ ulong get_tbclk(void)
 {
return gpt_get_clk();
 }
+
+/*
+ * This function is intended for SHORT delays only.
+ * It will overflow at around 10 seconds @ 400MHz,
+ * or 20 seconds @ 200MHz.
+ */
+unsigned long usec2ticks(unsigned long _usec)
+{
+   unsigned long long usec = _usec;
+
+   usec *= get_tbclk();
+   usec += 99;
+   do_div(usec, 100);
+
+   return usec;
+}
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 1/5] arm: imx-common: add SECURE_BOOT option to Kconfig

[Gary Bisson]

So the option can easily be selected through menuconfig.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 arch/arm/imx-common/Kconfig | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig
index 1b7da5a..1a09a2c 100644
--- a/arch/arm/imx-common/Kconfig
+++ b/arch/arm/imx-common/Kconfig
@@ -17,3 +17,10 @@ config IMX_BOOTAUX
depends on ARCH_MX7 || ARCH_MX6
help
  bootaux [addr] to boot auxiliary core.
+
+config SECURE_BOOT
+   bool "Support i.MX HAB features"
+   depends on ARCH_MX7 || ARCH_MX6 || ARCH_MX5
+   help
+ This option enables the support for secure boot (HAB).
+ See doc/README.mxc_hab for more details.
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 3/5] mx6_common: add secure boot support

[Gary Bisson]

Selecting the proper options to enable the build of the HAB tools.

Note, this support is disabled by default, one will have to select
the SECURE_BOOT configuration through menuconfig to enable it.

See doc/README.mxc_hab for more details.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 include/configs/mx6_common.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/include/configs/mx6_common.h b/include/configs/mx6_common.h
index fb49322..02afe8a 100644
--- a/include/configs/mx6_common.h
+++ b/include/configs/mx6_common.h
@@ -93,4 +93,13 @@
 #define CONFIG_CMD_FUSE
 #define CONFIG_MXC_OCOTP
 
+/* Secure boot (HAB) support */
+#ifdef CONFIG_SECURE_BOOT
+#define CONFIG_CSF_SIZE0x2000
+#define CONFIG_SYS_FSL_SEC_COMPAT  4
+#define CONFIG_FSL_CAAM
+#define CONFIG_CMD_DEKBLOB
+#define CONFIG_SYS_FSL_SEC_LE
+#endif
+
 #endif
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 0/5] Add i.MX HAB encryption support

[Gary Bisson]

Hi all,

This patch is actually a follow-up of the one from Peng Fan:
https://patchwork.ozlabs.org/patch/616568/

As he mentioned at the time, the current i.MX source code doesn't allow
to build a U-Boot that supports encryption features (dek commands).
The reason is that the commit 2bb01482 did some cleanup, removing usec2ticks
which is mandatory for the Freescale/NXP job ring code to build.

At the time of the original patch, Tom/Stefano said that a configuration
would need to leverage those options and some of the config should be
moved to Kconfig. The goal of this series is to enable it in the common
mx6/7 configuration files.

The series also adds the missing CSF declaration in nitrogen6* cfg files.

Changes v1->v2:
- Simplified the help comment of the SECURE_BOOT config option
- Moved SECURE_BOOT options to mx6/7_common.h

Let me know your thoughts.

Regards,
Gary

Gary Bisson (4):
  arm: imx-common: add SECURE_BOOT option to Kconfig
  mx6_common: add secure boot support
  mx7_common: add secure boot support
  nitrogen6x: add secure boot support

Peng Fan (1):
  arm: imx-common: introduce back usec2ticks

 arch/arm/imx-common/Kconfig |  7 +++
 arch/arm/imx-common/timer.c | 16 
 board/boundary/nitrogen6x/nitrogen6dl.cfg   |  3 +++
 board/boundary/nitrogen6x/nitrogen6dl2g.cfg |  3 +++
 board/boundary/nitrogen6x/nitrogen6q.cfg|  3 +++
 board/boundary/nitrogen6x/nitrogen6q2g.cfg  |  3 +++
 board/boundary/nitrogen6x/nitrogen6s.cfg|  3 +++
 board/boundary/nitrogen6x/nitrogen6s1g.cfg  |  3 +++
 include/configs/colibri_imx7.h  |  4 
 include/configs/mx6_common.h|  9 +
 include/configs/mx7_common.h|  9 +
 include/configs/mx7dsabresd.h   |  4 
 12 files changed, 59 insertions(+), 8 deletions(-)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] nitrogen6x: add secure boot support

[Gary Bisson]

Hi Eric, all,

On Tue, Aug 23, 2016 at 05:35:14PM -0700, Eric Nelson wrote:
> Hi Gary,
> 
> On 08/23/2016 02:55 PM, Gary Bisson wrote:
> > Selecting the proper options to enable the build of the HAB tools.
> > 
> > Also adding a CSF section to the imx final image so it can contain
> > the signature information.
> > 
> > Note, this support is disabled by default, one will have to select
> > the SECURE_BOOT configuration through menuconfig to enable it.
> > 
> > Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
> > ---
> >  board/boundary/nitrogen6x/nitrogen6dl.cfg   | 3 +++
> >  board/boundary/nitrogen6x/nitrogen6dl2g.cfg | 3 +++
> >  board/boundary/nitrogen6x/nitrogen6q.cfg| 3 +++
> >  board/boundary/nitrogen6x/nitrogen6q2g.cfg  | 3 +++
> >  board/boundary/nitrogen6x/nitrogen6s.cfg| 3 +++
> >  board/boundary/nitrogen6x/nitrogen6s1g.cfg  | 3 +++
> >  include/configs/nitrogen6x.h| 9 +
> >  7 files changed, 27 insertions(+)
> > 
> > diff --git a/board/boundary/nitrogen6x/nitrogen6dl.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6dl.cfg
> > index 1cdccad..5c3e961 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6dl.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6dl.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
> > index 516d67e..fe19ed0 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/board/boundary/nitrogen6x/nitrogen6q.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6q.cfg
> > index b6642e6..60e1885 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6q.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6q.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/board/boundary/nitrogen6x/nitrogen6q2g.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
> > index fe6dfc1..7a3ee94 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6q2g.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/board/boundary/nitrogen6x/nitrogen6s.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6s.cfg
> > index ca30cd6..2540b7b 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6s.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6s.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/board/boundary/nitrogen6x/nitrogen6s1g.cfg 
> > b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
> > index b1489fb..946af7b 100644
> > --- a/board/boundary/nitrogen6x/nitrogen6s1g.cfg
> > +++ b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
> > @@ -20,6 +20,9 @@ BOOT_FROM  spi
> >  
> >  #define __ASSEMBLY__
> >  #include 
> > +#ifdef CONFIG_SECURE_BOOT
> > +CSF CONFIG_CSF_SIZE
> > +#endif
> >  #include "asm/arch/mx6-ddr.h"
> >  #include "asm/arch/iomux.h"
> >  #include "asm/arch/crm_regs.h"
> > diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
> > index b651eb3..3281e42 1

Re: [U-Boot] [PATCH 2/3] arm: imx-common: introduce back usec2ticks

[Gary Bisson]

Hi Eric, all,

On Tue, Aug 23, 2016 at 05:30:36PM -0700, Eric Nelson wrote:
> Hi Gary and Peng,
> 
> On 08/23/2016 02:55 PM, Gary Bisson wrote:
> > From: Peng Fan <van.free...@gmail.com>
> > 
> > This commit "2bb014820c49a63902103bac710bc86b5772e843"
> > do some clean up to use the code in lib/time.c.
> > But usec2ticks is still being used by security related job ring code.
> > Bring back the function to avoid build break when CONFIG_FSL_CAAM
> > is defined.
> > The computation logic has been changed, using 64-bit variable
> > to ease the process, making it work on older (MX5) platforms.
> > 
> > Signed-off-by: Peng Fan <van.free...@gmail.com>
> > Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com>
> > Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
> > ---
> >  arch/arm/imx-common/timer.c | 16 
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
> > index a01590c..a04c7ae 100644
> > --- a/arch/arm/imx-common/timer.c
> > +++ b/arch/arm/imx-common/timer.c
> > @@ -119,3 +119,19 @@ ulong get_tbclk(void)
> >  {
> > return gpt_get_clk();
> >  }
> > +
> > +/*
> > + * This function is intended for SHORT delays only.
> > + * It will overflow at around 10 seconds @ 400MHz,
> > + * or 20 seconds @ 200MHz.
> > + */
> > +unsigned long usec2ticks(unsigned long _usec)
> > +{
> > +   unsigned long long usec = _usec;
> > +
> > +   usec *= get_tbclk();
> > +   usec += 99;
> > +   do_div(usec, 100);
> > +
> > +   return usec;
> > +}
> > 
> 
> What about the version in imx-common/syscounter.c?
> 
> It seems that only one should be needed.

Well syscounter.c is only built for mx7 platforms whereas timer.c only
applies to mx6|mx5 (see Makefile).

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] arm: imx-common: add SECURE_BOOT option to Kconfig

[Gary Bisson]

Hi Eric, all,

On Tue, Aug 23, 2016 at 05:24:48PM -0700, Eric Nelson wrote:
> Nicely done Gary!
> 
> On 08/23/2016 02:55 PM, Gary Bisson wrote:
> > So the option can easily be selected through menuconfig.
> > 
> > Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
> > ---
> >  arch/arm/imx-common/Kconfig | 9 +
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig
> > index 1b7da5a..5ee230e 100644
> > --- a/arch/arm/imx-common/Kconfig
> > +++ b/arch/arm/imx-common/Kconfig
> > @@ -17,3 +17,12 @@ config IMX_BOOTAUX
> > depends on ARCH_MX7 || ARCH_MX6
> > help
> >   bootaux [addr] to boot auxiliary core.
> > +
> > +config SECURE_BOOT
> > +   bool "Support i.MX HAB features"
> > +   depends on ARCH_MX7 || ARCH_MX6 || ARCH_MX5
> > +   help
> > + This option enables the support for secure boot (HAB) which
> > + includes adding a CSF section to the final imx image and
>^^^
> This doesn't add a CSF section.
> 
> Perhaps this should say "can enable a conditional section of an
> i.MX configuration (.cfg) file when producing an imx image".

Well now that you pointed out this sentence, I realize that it doesn't
even "add" the section per say, it just declares it in the IVT header.
It is then up to you to create and concatenate the CSF binary to the
u-boot.imx image.

For those not familiar with HAB:
https://boundarydevices.com/high-assurance-boot-hab-dummies/

Maybe in V2 I should keep it simple and just point to the README:
This option enables the support for secure boot (HAB).
See doc/README.mxc_hab for more details.

> > + some security-related commands such as 'hab_status'.
> > + See doc/README.mxc_hab for more details.
> > 
> 
> You should probably include a note in README.mxc_hab about use
> in .cfg files.

Yes that is a good point, will do in V2.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 1/3] arm: imx-common: add SECURE_BOOT option to Kconfig

[Gary Bisson]

So the option can easily be selected through menuconfig.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 arch/arm/imx-common/Kconfig | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/imx-common/Kconfig b/arch/arm/imx-common/Kconfig
index 1b7da5a..5ee230e 100644
--- a/arch/arm/imx-common/Kconfig
+++ b/arch/arm/imx-common/Kconfig
@@ -17,3 +17,12 @@ config IMX_BOOTAUX
depends on ARCH_MX7 || ARCH_MX6
help
  bootaux [addr] to boot auxiliary core.
+
+config SECURE_BOOT
+   bool "Support i.MX HAB features"
+   depends on ARCH_MX7 || ARCH_MX6 || ARCH_MX5
+   help
+ This option enables the support for secure boot (HAB) which
+ includes adding a CSF section to the final imx image and
+ some security-related commands such as 'hab_status'.
+ See doc/README.mxc_hab for more details.
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 3/3] nitrogen6x: add secure boot support

[Gary Bisson]

Selecting the proper options to enable the build of the HAB tools.

Also adding a CSF section to the imx final image so it can contain
the signature information.

Note, this support is disabled by default, one will have to select
the SECURE_BOOT configuration through menuconfig to enable it.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 board/boundary/nitrogen6x/nitrogen6dl.cfg   | 3 +++
 board/boundary/nitrogen6x/nitrogen6dl2g.cfg | 3 +++
 board/boundary/nitrogen6x/nitrogen6q.cfg| 3 +++
 board/boundary/nitrogen6x/nitrogen6q2g.cfg  | 3 +++
 board/boundary/nitrogen6x/nitrogen6s.cfg| 3 +++
 board/boundary/nitrogen6x/nitrogen6s1g.cfg  | 3 +++
 include/configs/nitrogen6x.h| 9 +
 7 files changed, 27 insertions(+)

diff --git a/board/boundary/nitrogen6x/nitrogen6dl.cfg 
b/board/boundary/nitrogen6x/nitrogen6dl.cfg
index 1cdccad..5c3e961 100644
--- a/board/boundary/nitrogen6x/nitrogen6dl.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6dl.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg 
b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
index 516d67e..fe19ed0 100644
--- a/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6dl2g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6q.cfg 
b/board/boundary/nitrogen6x/nitrogen6q.cfg
index b6642e6..60e1885 100644
--- a/board/boundary/nitrogen6x/nitrogen6q.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6q.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6q2g.cfg 
b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
index fe6dfc1..7a3ee94 100644
--- a/board/boundary/nitrogen6x/nitrogen6q2g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6q2g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6s.cfg 
b/board/boundary/nitrogen6x/nitrogen6s.cfg
index ca30cd6..2540b7b 100644
--- a/board/boundary/nitrogen6x/nitrogen6s.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6s.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/board/boundary/nitrogen6x/nitrogen6s1g.cfg 
b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
index b1489fb..946af7b 100644
--- a/board/boundary/nitrogen6x/nitrogen6s1g.cfg
+++ b/board/boundary/nitrogen6x/nitrogen6s1g.cfg
@@ -20,6 +20,9 @@ BOOT_FROM  spi
 
 #define __ASSEMBLY__
 #include 
+#ifdef CONFIG_SECURE_BOOT
+CSF CONFIG_CSF_SIZE
+#endif
 #include "asm/arch/mx6-ddr.h"
 #include "asm/arch/iomux.h"
 #include "asm/arch/crm_regs.h"
diff --git a/include/configs/nitrogen6x.h b/include/configs/nitrogen6x.h
index b651eb3..3281e42 100644
--- a/include/configs/nitrogen6x.h
+++ b/include/configs/nitrogen6x.h
@@ -35,6 +35,15 @@
 #define CONFIG_SF_DEFAULT_MODE (SPI_MODE_0)
 #endif
 
+/* Secure boot (HAB) support */
+#ifdef CONFIG_SECURE_BOOT
+#define CONFIG_CSF_SIZE0x2000
+#define CONFIG_SYS_FSL_SEC_COMPAT  4
+#define CONFIG_FSL_CAAM
+#define CONFIG_CMD_DEKBLOB
+#define CONFIG_SYS_FSL_SEC_LE
+#endif
+
 /* I2C Configs */
 #define CONFIG_SYS_I2C
 #define CONFIG_SYS_I2C_MXC
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 2/3] arm: imx-common: introduce back usec2ticks

[Gary Bisson]

From: Peng Fan <van.free...@gmail.com>

This commit "2bb014820c49a63902103bac710bc86b5772e843"
do some clean up to use the code in lib/time.c.
But usec2ticks is still being used by security related job ring code.
Bring back the function to avoid build break when CONFIG_FSL_CAAM
is defined.
The computation logic has been changed, using 64-bit variable
to ease the process, making it work on older (MX5) platforms.

Signed-off-by: Peng Fan <van.free...@gmail.com>
Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com>
Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
 arch/arm/imx-common/timer.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/arch/arm/imx-common/timer.c b/arch/arm/imx-common/timer.c
index a01590c..a04c7ae 100644
--- a/arch/arm/imx-common/timer.c
+++ b/arch/arm/imx-common/timer.c
@@ -119,3 +119,19 @@ ulong get_tbclk(void)
 {
return gpt_get_clk();
 }
+
+/*
+ * This function is intended for SHORT delays only.
+ * It will overflow at around 10 seconds @ 400MHz,
+ * or 20 seconds @ 200MHz.
+ */
+unsigned long usec2ticks(unsigned long _usec)
+{
+   unsigned long long usec = _usec;
+
+   usec *= get_tbclk();
+   usec += 99;
+   do_div(usec, 100);
+
+   return usec;
+}
-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 0/3] Add i.MX HAB encryption support

[Gary Bisson]

Hi all,

This patch is actually a follow-up of the one from Peng Fan:
https://patchwork.ozlabs.org/patch/616568/

As he mentioned at the time, the current i.MX source code doesn't allow
to build a U-Boot that supports encryption features (dek commands).
The reason is that the commit 2bb01482 did some cleanup, removing usec2ticks
which is mandatory for the Freescale/NXP job ring code to build.

At the time of the original patch, Tom/Stefano said that a configuration
would need to leverage those options and some of the config should be
moved to Kconfig. The goal of this series is to enable it with the
nitrogen6x platform configuration.

As a first version, this series only adds the CONFIG_SECURE_BOOT to the
imx-common Kconfig so it can be enabled using menuconfig.

If this is not sufficient, the nitrogen6*_defconfig could be duplicated
into nitrogen6*_hab_defconfig but it sounds like a lot of copy just to
enable HAB features. No need to say that we would prefer keeping our
default defconfig without the HAB feature enabled.

Then I think the variables could actually be moved from nitrogen6x.h
to mx6_common.h and mx7_common.h. Maybe need to split it in 2 since
CONFIG_CSF_SIZE is sufficient to achieve binary signature. The other
options are only required if you want to encrypt the binary (which
requires the use of dek_blob command.

Let me know your thoughts.

Regards,
Gary


Gary Bisson (2):
  arm: imx-common: add SECURE_BOOT option to Kconfig
  nitrogen6x: add secure boot support

Peng Fan (1):
  arm: imx-common: introduce back usec2ticks

 arch/arm/imx-common/Kconfig |  9 +
 arch/arm/imx-common/timer.c | 16 
 board/boundary/nitrogen6x/nitrogen6dl.cfg   |  3 +++
 board/boundary/nitrogen6x/nitrogen6dl2g.cfg |  3 +++
 board/boundary/nitrogen6x/nitrogen6q.cfg|  3 +++
 board/boundary/nitrogen6x/nitrogen6q2g.cfg  |  3 +++
 board/boundary/nitrogen6x/nitrogen6s.cfg|  3 +++
 board/boundary/nitrogen6x/nitrogen6s1g.cfg  |  3 +++
 include/configs/nitrogen6x.h|  9 +
 9 files changed, 52 insertions(+)

-- 
2.8.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH V4 2/3] mx7: psci: add basic psci support

[Gary Bisson]

Hi Peng,

On Thu, Mar 31, 2016 at 3:33 PM, Peng Fan <van.free...@gmail.com> wrote:
> Hi Gary,
>
> On Thu, Mar 31, 2016 at 01:17:07PM +0200, Gary Bisson wrote:
>>Hi all,
>>
>>Sorry to revive an old thread but I have some questions about thit patch.
>>
>>On Fri, Oct 23, 2015 at 10:13:04AM +0800, Peng Fan wrote:
>>> 1. add basic psci support for imx7 chip.
>>> 2. support cpu_on and cpu_off.
>>> 3. switch to non-secure mode when boot linux kernel.
>>> 4. set csu allow accessing all peripherial register in non-secure mode.
>>>
>>> Signed-off-by: Frank Li 
>>> Signed-off-by: Peng Fan 
>>> Cc: Stefano Babic 
>>> Cc: Fabio Estevam 
>>> ---
>>> [snip]
>>> diff --git a/arch/arm/include/asm/arch-mx7/imx-regs.h 
>>> b/arch/arm/include/asm/arch-mx7/imx-regs.h
>>> index 4dc11ee..9213374 100644
>>> --- a/arch/arm/include/asm/arch-mx7/imx-regs.h
>>> +++ b/arch/arm/include/asm/arch-mx7/imx-regs.h
>>> @@ -866,6 +866,9 @@ struct cspi_regs {
>>>  ECSPI3_BASE_ADDR, \
>>>  ECSPI4_BASE_ADDR
>>>
>>> +#define CSU_INIT_SEC_LEVEL0 0x00FF00FF
>>> +#define CSU_NUM_REGS64
>>
>>In the security documentation (revA) it is said that there are 40 CSL,
>>why is it 64 here?
>>
>>Also, although this seems to work, later on when the kernel boots I get
>>the following CAAM errors:
>>caam 3090.caam: failed to acquire DECO 0
>>...
>>caam 3090.caam: failed to acquire DECO 0
>>caam 3090.caam: failed to instantiate RNG
>>caam: probe of 3090.caam failed with error -11
>
> This patch will let SoC switch to non sec mode. I have little knowledge
> of CAAM, I guess it works in sec mode. So when kernel boots up, it will
> complains a lot...

Seems like a safe assumption to say CAAM requires to be run in secure mode.

>>If I revert this patch and therefore leave the CSU to its default state
>>at bootup the above CAAM issue disappears, do you have any idea why?
>
>
> Revert this patch, then all code runs in sec mode.
>
>>
>>As a FYI, I am using U-Boot v2016.03 + a few patches that adds support
>>for our i.MX7 Nitrogen7 board. You can find the repo here:
>>https://github.com/boundarydevices/u-boot-imx6/tree/boundary-v2016.03
>>
>>Also, if I base U-Boot on top of NXP repo it works too since this csu/psci
>>support isn't there:
>>http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/?id=rel_imx_3.14.52_1.1.0_ga
>
> Yeah. NXP code not switch to non sec mode.
>
> You can revert this patch in your vendor tree. I am not sure whether you
> have tested linux upstream tree, or you use caam code from NXP vendor
> tree. If you use NXP vendor tree, and use uboot upstream code, sure
> caam will complain errros. We would like to use PSCI and work in
> non-sec mode, but still some works need to be done.

Thanks for your feedback. We are using our own tree based on NXP
vendor one (3.14.52_1.1.0_ga).

We usually use upstream U-Boot with this kernel tree. I guess the
easiest option here is actually to add CONFIG_MX7_SEC to our board
config, this avoids to revert the patches.

Thanks,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH V4 2/3] mx7: psci: add basic psci support

[Gary Bisson]

Hi all,

Sorry to revive an old thread but I have some questions about thit patch.

On Fri, Oct 23, 2015 at 10:13:04AM +0800, Peng Fan wrote:
> 1. add basic psci support for imx7 chip.
> 2. support cpu_on and cpu_off.
> 3. switch to non-secure mode when boot linux kernel.
> 4. set csu allow accessing all peripherial register in non-secure mode.
> 
> Signed-off-by: Frank Li 
> Signed-off-by: Peng Fan 
> Cc: Stefano Babic 
> Cc: Fabio Estevam 
> ---
> [snip]
> diff --git a/arch/arm/include/asm/arch-mx7/imx-regs.h 
> b/arch/arm/include/asm/arch-mx7/imx-regs.h
> index 4dc11ee..9213374 100644
> --- a/arch/arm/include/asm/arch-mx7/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx7/imx-regs.h
> @@ -866,6 +866,9 @@ struct cspi_regs {
>   ECSPI3_BASE_ADDR, \
>   ECSPI4_BASE_ADDR
>  
> +#define CSU_INIT_SEC_LEVEL0  0x00FF00FF
> +#define CSU_NUM_REGS 64

In the security documentation (revA) it is said that there are 40 CSL,
why is it 64 here?

Also, although this seems to work, later on when the kernel boots I get
the following CAAM errors:
caam 3090.caam: failed to acquire DECO 0
...
caam 3090.caam: failed to acquire DECO 0
caam 3090.caam: failed to instantiate RNG
caam: probe of 3090.caam failed with error -11

If I revert this patch and therefore leave the CSU to its default state
at bootup the above CAAM issue disappears, do you have any idea why?

As a FYI, I am using U-Boot v2016.03 + a few patches that adds support
for our i.MX7 Nitrogen7 board. You can find the repo here:
https://github.com/boundarydevices/u-boot-imx6/tree/boundary-v2016.03

Also, if I base U-Boot on top of NXP repo it works too since this csu/psci
support isn't there:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/tree/?id=rel_imx_3.14.52_1.1.0_ga

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Disabling/Enabling the Data Cache

[Gary Bisson]

Albert, Fabio, All,

On Tue, Sep 22, 2015 at 09:21:37PM +0200, Albert ARIBAUD wrote:
> Hi Fabio,
> 
> Le Tue, 22 Sep 2015 16:01:05 -0300, Fabio Estevam 
> a ?crit :
> 
> > Hi,
> > 
> > On a mx6q (armv7) board when I disable and enable the Data Cache the
> > following issue is observed:
> > 
> > => dcache
> > Data (writethrough) Cache is ON
> > => dcache off
> > => dcache on
> > data abort
> > pc : [<4ff3d340>]  lr : [<4ff3b598>]
> > reloc pc : [<17802340>]lr : [<17800598>]
> > sp : 4f538d50  ip :  fp : 4f841188
> > r10: 4ffa4fd4  r9 : 4f538eb0 r8 : 0002
> > r7 : 4f539de8  r6 :  r5 :   r4 : 
> > r3 : 00a02000  r2 : 3245 r1 : 4ff94732  r0 : 0001
> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> > Resetting CPU ...
> > 
> > resetting ...
> > 
> > 
> > U-Boot 2015.10-rc3-24232-g324714b-dirty (Sep 22 2015 - 15:08:16 -0300)
> > 
> > CPU:   Freescale i.MX6DL rev1.1 996 MHz (running at 792 MHz)
> > 
> > Does anyone have any ideas about this?
> 
> No idea with so little context, but the (reloc) pc should indicate where
> in the code the abort happened. Maybe that will give us a clue.

Does anyone have an update on this issue? I just experienced the same
problem with an i.MX6SX:
Hit any key to stop autoboot:  0 
=> dcache off
=> dcache on 
data abort
pc : []  lr : []
reloc pc : [<87800564>]lr : [<878005f4>]
sp : bf347d70  ip :  fp : bf34bb48
r10: bffa3c58  r9 : bf347ed8 r8 : 0002
r7 : bf349c28  r6 :  r5 :   r4 : 
r3 : 00a02000  r2 : 3243 r1 : bff99a70  r0 : 0001
Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
Resetting CPU ...

Looking at the reloc pc:
87800550 :
87800550:   e59f304cldr r3, [pc, #76]   ; 878005a4

87800554:   e5932104ldr r2, [r3, #260]  ; 0x104
87800558:   e3822501orr r2, r2, #4194304;
0x40
8780055c:   e5832104str r2, [r3, #260]  ; 0x104
87800560:   e5932100ldr r2, [r3, #256]  ; 0x100
87800564:   e3c22001bic r2, r2, #1

Then looking at arch/arm/cpu/armv7/mx6/soc.c, it looks like the cause
is:
http://git.denx.de/?p=u-boot.git;a=commit;h=b4ed9f8

When commenting out "setbits_le32(>pl310_aux_ctrl,
L310_SHARED_ATT_OVERRIDE_ENABLE)", the problem disappears.

Should we unset the bit22 when disabling cache?

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] Disabling/Enabling the Data Cache

[Gary Bisson]

Adding Albert (wrong address in first e-mail).

Regards,
Gary

On Tue, Nov 24, 2015 at 12:35 PM, Gary Bisson
<gary.bis...@boundarydevices.com> wrote:
> Albert, Fabio, All,
>
> On Tue, Sep 22, 2015 at 09:21:37PM +0200, Albert ARIBAUD wrote:
>> Hi Fabio,
>>
>> Le Tue, 22 Sep 2015 16:01:05 -0300, Fabio Estevam 
>> a ?crit :
>>
>> > Hi,
>> >
>> > On a mx6q (armv7) board when I disable and enable the Data Cache the
>> > following issue is observed:
>> >
>> > => dcache
>> > Data (writethrough) Cache is ON
>> > => dcache off
>> > => dcache on
>> > data abort
>> > pc : [<4ff3d340>]  lr : [<4ff3b598>]
>> > reloc pc : [<17802340>]lr : [<17800598>]
>> > sp : 4f538d50  ip :  fp : 4f841188
>> > r10: 4ffa4fd4  r9 : 4f538eb0 r8 : 0002
>> > r7 : 4f539de8  r6 :  r5 :   r4 : 
>> > r3 : 00a02000  r2 : 3245 r1 : 4ff94732  r0 : 0001
>> > Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>> > Resetting CPU ...
>> >
>> > resetting ...
>> >
>> >
>> > U-Boot 2015.10-rc3-24232-g324714b-dirty (Sep 22 2015 - 15:08:16 -0300)
>> >
>> > CPU:   Freescale i.MX6DL rev1.1 996 MHz (running at 792 MHz)
>> >
>> > Does anyone have any ideas about this?
>>
>> No idea with so little context, but the (reloc) pc should indicate where
>> in the code the abort happened. Maybe that will give us a clue.
>
> Does anyone have an update on this issue? I just experienced the same
> problem with an i.MX6SX:
> Hit any key to stop autoboot:  0
> => dcache off
> => dcache on
> data abort
> pc : []  lr : []
> reloc pc : [<87800564>]lr : [<878005f4>]
> sp : bf347d70  ip :  fp : bf34bb48
> r10: bffa3c58  r9 : bf347ed8 r8 : 0002
> r7 : bf349c28  r6 :  r5 :   r4 : 
> r3 : 00a02000  r2 : 3243 r1 : bff99a70  r0 : 0001
> Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
> Resetting CPU ...
>
> Looking at the reloc pc:
> 87800550 :
> 87800550:   e59f304cldr r3, [pc, #76]   ; 878005a4
> <v7_outer_cache_enable+0x54>
> 87800554:   e5932104ldr r2, [r3, #260]  ; 0x104
> 87800558:   e3822501orr r2, r2, #4194304;
> 0x40
> 8780055c:   e5832104str r2, [r3, #260]  ; 0x104
> 87800560:   e5932100ldr r2, [r3, #256]  ; 0x100
> 87800564:   e3c22001bic r2, r2, #1
>
> Then looking at arch/arm/cpu/armv7/mx6/soc.c, it looks like the cause
> is:
> http://git.denx.de/?p=u-boot.git;a=commit;h=b4ed9f8
>
> When commenting out "setbits_le32(>pl310_aux_ctrl,
> L310_SHARED_ATT_OVERRIDE_ENABLE)", the problem disappears.
>
> Should we unset the bit22 when disabling cache?
>
> Regards,
> Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] fs: ext4: fix symlink read function

[Gary Bisson]

Hi Suriyan,

On Sun, Sep 27, 2015 at 8:05 AM, Suriyan Bhavani
 wrote:
> Hi Gary,
>Recently you submitted a patch to fix the ext4 symlnik issue.
>
> Can you please tell me the actual commands that failed or gave wrong
> results?

The load command (tested on ext4 partition) was failing with an error
that said the file didn't exist since ext4fs_read_file was returning 0
(success) but this return value was considered a failure.

> I ask cause I am trying to do the below:
> 1. Incorporate symlnk test in fs-test.sh
> 2. use host instead of sb as sb is deprecated
> 3. Reproduce the issue without your fix, so I can see what is failing
>
> I haven't been able to do 3. above (by reverting your fix to recreate the
> error conditions). I tried the below commands in sandbox and they worked as
> expected. (by creating a soft link pointing to 1MB.file)

Have you reverted my patch only or did you go back to a prior commit
of 9f12cd0 which is where the API changed?

> # generic command check
>
> ls host 0:0
>
> size host 0:0 1MB.file.link
>
> load host 0:0 0x0108 1MB.file.link
>
> md5sum 0x0108 $filesize
>
>
> # ext command check
>
> ext4size host 0:0 1MB.file.link
>
> ext4load host 0:0 0x0108 1MB.file.link
>
> md5sum 0x0108 $filesize

That sounds about right, that is the kind of test I wanted to add.
Could you share the output of the ls command on the link file?

> Would be nice to throw some light on this issue so I can build a test case.

Well I hope my answer will help. I wanted to update the test script
but the sandbox didn't seem to work (I've sent an email about it).

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] fs: ext4: fix symlink read function

[Gary Bisson]

Hi Simon,

On Tue, Sep 8, 2015 at 5:56 AM, Simon Glass <s...@chromium.org> wrote:
> Hi Gary,
>
> On 7 September 2015 at 03:20, Gary Bisson
> <gary.bis...@boundarydevices.com> wrote:
>> Since last API changes for files >2GB, the read of symlink is broken as
>> ext4fs_read_file now returns 0 instead of the length of the actual read.
>>
>> Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
>> ---
>> Hi all,
>>
>> Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
>> read symlinks any more. This is due to the API changes made to
>> ext4fs_read_file (commit 9f12cd0).
>>
>> This patch makes the read symlink check both for errors (status < 0) and
>> actual read length.
>>
>> Regards,
>> Gary
>
> Thanks for fixing this. Does the filesystem test (test/fs-test.sh)
> show this error? How could we enhance the test to detect this?

I wasn't aware of this testing script, a quick look at it makes me
think it doesn't check anything symlink-related. I guess it would
require to add SYMLINK_FILE that points to either SMALL_FILE or
BIG_FILE, read it and check the md5sum.

Regards,
Gary
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH] fs: ext4: fix symlink read function

[Gary Bisson]

Since last API changes for files >2GB, the read of symlink is broken as
ext4fs_read_file now returns 0 instead of the length of the actual read.

Signed-off-by: Gary Bisson <gary.bis...@boundarydevices.com>
---
Hi all,

Switching from an old v2014.07 to v2015.07 we've noticed that we couldn't
read symlinks any more. This is due to the API changes made to
ext4fs_read_file (commit 9f12cd0).

This patch makes the read symlink check both for errors (status < 0) and
actual read length.

Regards,
Gary
---
 fs/ext4/ext4_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index cab5465..e2ab145 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2040,7 +2040,7 @@ static char *ext4fs_read_symlink(struct ext2fs_node *node)
status = ext4fs_read_file(diro, 0,
   __le32_to_cpu(diro->inode.size),
   symlink, );
-   if (status == 0) {
+   if ((status < 0) || (actread == 0)) {
free(symlink);
return 0;
}
-- 
2.5.1

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot