On Tue, Oct 3, 2017 at 6:47 AM, Rob Clark robdclark@gmail.com wrote:

personally I think we should try to figure out what is wrong on imx6 rather than blindly reverting.. without this change MBR partitioned disks might not generate unique device-paths for EFI boot.

If you can't get any debug logs from SPL build, perhaps you can try an old SPL image but main u-boot image with the logs I suggested? I guess the same issue should happen with the main u-boot image.

BR, -R

On Mon, Oct 2, 2017 at 9:50 PM, Fabio Estevam festevam@gmail.com wrote:

...

From: Fabio Estevam fabio.estevam@nxp.com

Since commit ff98cb90514d ("part: extract MBR signature from partitions") SPL boot on i.MX6 starts to fail:

U-Boot SPL 2017.09-00221-g0d6ab32 (Oct 02 2017 - 15:13:19) Trying to boot from MMC1 (hangs here)

Revert the part_test_dos() changes from this commit, so that SPL boot can be functional again.

Tested on a imx6q-cuboxi board.

Signed-off-by: Fabio Estevam fabio.estevam@nxp.com

disk/part_dos.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/disk/part_dos.c b/disk/part_dos.c index 1a36be0..213a8d8 100644 --- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -89,20 +89,14 @@ static int test_block_type(unsigned char *buffer)

static int part_test_dos(struct blk_desc *dev_desc) {

•   ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, dev_desc->blksz);
  

•   ALLOC_CACHE_ALIGN_BUFFER(unsigned char, mbr, dev_desc->blksz);
  

btw, if I had to take a guess, I'd say that perhaps blksz is smaller than 'legacy_mbr', so maybe rather than allocating blksize, it should be DIV_ROUND_UP(sizeof(legacy_mbr), dev_desc->blksz).. or I guess that could be simplified to not use division if blksz is a power of two

BR, -R

...

    if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1)
            return -1;

•   if (test_block_type((unsigned char *)mbr) != DOS_MBR)
  

•   if (test_block_type(mbr) != DOS_MBR)
            return -1;
  
  

•   if (dev_desc->sig_type == SIG_TYPE_NONE &&
  

•       mbr->unique_mbr_signature != 0) {
  

•           dev_desc->sig_type = SIG_TYPE_MBR;
  

•           dev_desc->mbr_sig = mbr->unique_mbr_signature;
  

•   }
  

•   return 0;
  

}

-- 2.7.4

On Tue, Oct 3, 2017 at 7:04 AM, Fabio Estevam festevam@gmail.com wrote:

On Tue, Oct 3, 2017 at 7:57 AM, Rob Clark robdclark@gmail.com wrote:

...

btw, if I had to take a guess, I'd say that perhaps blksz is smaller than 'legacy_mbr', so maybe rather than allocating blksize, it should be DIV_ROUND_UP(sizeof(legacy_mbr), dev_desc->blksz).. or I guess that could be simplified to not use division if blksz is a power of two

Yes, it does seem to be size related as we are size constraint in SPL.

Just tried your suggestion:

--- a/disk/part_dos.c +++ b/disk/part_dos.c @@ -89,7 +89,9 @@ static int test_block_type(unsigned char *buffer)

static int part_test_dos(struct blk_desc *dev_desc) {

•   ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr, dev_desc->blksz);
  

•   ALLOC_CACHE_ALIGN_BUFFER(legacy_mbr, mbr,
  

•                            DIV_ROUND_UP(sizeof(legacy_mbr),
  

•                            dev_desc->blksz));
  
    if (blk_dread(dev_desc, 0, 1, (ulong *)mbr) != 1)
            return -1;
  
  

and it does work for me :-)

Ok, I guess if blksz can actually be less than the mbr, we probably also need a similar fix in is_gpt_valid() (and also to pass the correct # of blks to blk_dread()).. I'll make a patch in a few..

BR, -R