diff mbox

[U-Boot] mxs: mxsboot: Add support for SD card generation for i.MX23

Message ID 201301241939.17482.marex@denx.de
State Changes Requested
Delegated to: Stefano Babic
Headers show

Commit Message

Marek Vasut Jan. 24, 2013, 6:39 p.m. UTC 
Dear Otavio Salvador,

> On Thu, Jan 24, 2013 at 4:08 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Thu, Jan 24, 2013 at 3:56 PM, Marek Vasut <marex@denx.de> wrote:
> >> > Dear Otavio Salvador,
> >> > 
> >> >> The mxsboot now receives the SoC type as parameter to generate binary
> >> >> compatible with the SoC. Currently the NAND support has not been add
> >> >> for i.MX23 as it is not yet supported in U-Boot.
> >> > 
> >> > Please fix the NAND support as well, then resubmit.
> >> 
> >> I won't work on NAND now; first I wish to fix the NAND driver support
> >> to later work in boot support.
> >> 
> >> > The patch basically does dd if=u-boot.sb ... bs=512 seek=4 ; any kind
> >> > of information can be stored in those first four blocks and the mx23
> >> > bootrom ignores it, so what's the gain of this?
> >> 
> >> Well, it works fine for users. A good gain in my opinion.
> > 
> > How is a simple documented dd if=... different? It's the same on imx, you
> > have to dd u-boot.imx with some offset.
> > 
> >> > I wonder, will MX28 bootrom ignore them as well? Then maybe we can get
> >> > rid of all this SD-specific junk.
> >> 
> >> Did not test but MX28 expects a BCB data structure (as said in 12.11.2
> >> - MX28RM) while MX23 does not. So I think MX28 won't work without the
> >> BCB.
> > 
> > Fabio?
> > 
> >> > Furthermore, I'd like to see all of this reworked as another plugin
> >> > for mkimage.
> >> 
> >> Yes; it would be a good long term solution but I don't want to hold it
> >> due any of above reasons. It works fine so improvements can be done
> >> later.
> > 
> > We already have a solution:
> > 
> > dd if=u-boot.sb of=/dev/sdX1 bs=512 seek=4
> > 
> > I think this is enough for now, until all is fixed in proper sequence.
> > That is, NAND driver and only after that, mxsboot for NAND _and_ SD .
> > For now, let's hold off this patch, add the above dd stuff into
> > documentation (doc/README.mx23) and then when all is ready, fix it all
> > properly please.
> 
> Well; you blocked olinuxino patch until mxsboot where ported and the
> dd with offset were not a solution for you, ... so ...

So, Fabio, can you please explain this to me? How does the SD boot work on mx23?

Does the inlined patch work on MX23 and MX28? We can use that as a temporary 
workaround.

> I prefer to have this as is and share documentation with mx28. The
> NAND ought to be done providing same interface so one doc for it all.
> I think change it in next version is wrong and confuse users.

Make a doc/README.mx23 with quirks needed for MX23. Once the issues are ironed 
out, adjust the readme. No problem.

PATCH:

Best regards,
Marek Vasut

Comments

Fabio Estevam Feb. 4, 2013, 6:12 p.m. UTC | #1 
On Thu, Jan 24, 2013 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:

> PATCH:
> diff --git a/tools/mxsboot.c b/tools/mxsboot.c
> index 6c05aa4..d92c39f 100644
> --- a/tools/mxsboot.c
> +++ b/tools/mxsboot.c
> @@ -551,7 +551,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>
>         fsize = lseek(infd, 0, SEEK_END);
>         lseek(infd, 0, SEEK_SET);
> -       size = fsize + 512;
> +       size = fsize + 4 * 512;
>
>         buf = malloc(size);
>         if (!buf) {
> @@ -559,7 +559,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>                 goto err0;
>         }
>
> -       ret = read(infd, (uint8_t *)buf + 512, fsize);
> +       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
>         if (ret != fsize) {
>                 ret = -1;
>                 goto err1;
> @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int outfd)
>         cb->drv_info[0].chip_num = 0x0;
>         cb->drv_info[0].drive_type = 0x0;
>         cb->drv_info[0].tag = 0x1;
> -       cb->drv_info[0].first_sector_number = sd_sector + 1;
> -       cb->drv_info[0].sector_count = (size - 1) / 512;
> +       cb->drv_info[0].first_sector_number = sd_sector + 4;
> +       cb->drv_info[0].sector_count = (size - 4) / 512;
>
>         wr_size = write(outfd, buf, size);
>         if (wr_size != size) {

My mx28evk does not boot with this patch applied.
Otavio Salvador Feb. 4, 2013, 6:15 p.m. UTC | #2 
On Mon, Feb 4, 2013 at 4:12 PM, Fabio Estevam <festevam@gmail.com> wrote:
> On Thu, Jan 24, 2013 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
>
>> PATCH:
>> diff --git a/tools/mxsboot.c b/tools/mxsboot.c
>> index 6c05aa4..d92c39f 100644
>> --- a/tools/mxsboot.c
>> +++ b/tools/mxsboot.c
>> @@ -551,7 +551,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>>
>>         fsize = lseek(infd, 0, SEEK_END);
>>         lseek(infd, 0, SEEK_SET);
>> -       size = fsize + 512;
>> +       size = fsize + 4 * 512;
>>
>>         buf = malloc(size);
>>         if (!buf) {
>> @@ -559,7 +559,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>>                 goto err0;
>>         }
>>
>> -       ret = read(infd, (uint8_t *)buf + 512, fsize);
>> +       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
>>         if (ret != fsize) {
>>                 ret = -1;
>>                 goto err1;
>> @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int outfd)
>>         cb->drv_info[0].chip_num = 0x0;
>>         cb->drv_info[0].drive_type = 0x0;
>>         cb->drv_info[0].tag = 0x1;
>> -       cb->drv_info[0].first_sector_number = sd_sector + 1;
>> -       cb->drv_info[0].sector_count = (size - 1) / 512;
>> +       cb->drv_info[0].first_sector_number = sd_sector + 4;
>> +       cb->drv_info[0].sector_count = (size - 4) / 512;
>>
>>         wr_size = write(outfd, buf, size);
>>         if (wr_size != size) {
>
> My mx28evk does not boot with this patch applied.

As it does not work in mx28evk I'd prefer to use my previously
proposed patch as it keeps clear what is done for mx23 and mx28. What
people think?

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut Feb. 4, 2013, 6:45 p.m. UTC | #3 
Dear Fabio Estevam,

[...]

> > @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int outfd)
> > 
> >         cb->drv_info[0].chip_num = 0x0;
> >         cb->drv_info[0].drive_type = 0x0;
> >         cb->drv_info[0].tag = 0x1;
> > 
> > -       cb->drv_info[0].first_sector_number = sd_sector + 1;
> > -       cb->drv_info[0].sector_count = (size - 1) / 512;
> > +       cb->drv_info[0].first_sector_number = sd_sector + 4;
> > +       cb->drv_info[0].sector_count = (size - 4) / 512;
> > 
> >         wr_size = write(outfd, buf, size);
> >         if (wr_size != size) {
> 
> My mx28evk does not boot with this patch applied.

Can you add this section to arch/arm/cpu/arm926ejs/mxs/u-boot-imx28.bd

-->8--
options {
       driveTag = 0x00;
       flags = 0x01;
}
--8<--

and paste the output you get on serial port? the flags = 1 should enable debug 
output from the bootrom.

Best regards,
Marek Vasut
Marek Vasut Feb. 4, 2013, 6:46 p.m. UTC | #4 
Dear Otavio Salvador,

> On Mon, Feb 4, 2013 at 4:12 PM, Fabio Estevam <festevam@gmail.com> wrote:
> > On Thu, Jan 24, 2013 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
> >> PATCH:
> >> diff --git a/tools/mxsboot.c b/tools/mxsboot.c
> >> index 6c05aa4..d92c39f 100644
> >> --- a/tools/mxsboot.c
> >> +++ b/tools/mxsboot.c
> >> @@ -551,7 +551,7 @@ static int mx28_create_sd_image(int infd, int outfd)
> >> 
> >>         fsize = lseek(infd, 0, SEEK_END);
> >>         lseek(infd, 0, SEEK_SET);
> >> 
> >> -       size = fsize + 512;
> >> +       size = fsize + 4 * 512;
> >> 
> >>         buf = malloc(size);
> >>         if (!buf) {
> >> 
> >> @@ -559,7 +559,7 @@ static int mx28_create_sd_image(int infd, int outfd)
> >> 
> >>                 goto err0;
> >>         
> >>         }
> >> 
> >> -       ret = read(infd, (uint8_t *)buf + 512, fsize);
> >> +       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
> >> 
> >>         if (ret != fsize) {
> >>         
> >>                 ret = -1;
> >>                 goto err1;
> >> 
> >> @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int outfd)
> >> 
> >>         cb->drv_info[0].chip_num = 0x0;
> >>         cb->drv_info[0].drive_type = 0x0;
> >>         cb->drv_info[0].tag = 0x1;
> >> 
> >> -       cb->drv_info[0].first_sector_number = sd_sector + 1;
> >> -       cb->drv_info[0].sector_count = (size - 1) / 512;
> >> +       cb->drv_info[0].first_sector_number = sd_sector + 4;
> >> +       cb->drv_info[0].sector_count = (size - 4) / 512;
> >> 
> >>         wr_size = write(outfd, buf, size);
> >>         if (wr_size != size) {
> > 
> > My mx28evk does not boot with this patch applied.
> 
> As it does not work in mx28evk I'd prefer to use my previously
> proposed patch as it keeps clear what is done for mx23 and mx28. What
> people think?

NAK. Your patch is just adding churn, which the bootrom ignores. Did you manage 
to get reply from FSL why the bootrom ignores it already?

Best regards,
Marek Vasut
Otavio Salvador Feb. 4, 2013, 6:50 p.m. UTC | #5 
On Mon, Feb 4, 2013 at 4:46 PM, Marek Vasut <marex@denx.de> wrote:
> Dear Otavio Salvador,
>
>> On Mon, Feb 4, 2013 at 4:12 PM, Fabio Estevam <festevam@gmail.com> wrote:
>> > On Thu, Jan 24, 2013 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
>> >> PATCH:
>> >> diff --git a/tools/mxsboot.c b/tools/mxsboot.c
>> >> index 6c05aa4..d92c39f 100644
>> >> --- a/tools/mxsboot.c
>> >> +++ b/tools/mxsboot.c
>> >> @@ -551,7 +551,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>> >>
>> >>         fsize = lseek(infd, 0, SEEK_END);
>> >>         lseek(infd, 0, SEEK_SET);
>> >>
>> >> -       size = fsize + 512;
>> >> +       size = fsize + 4 * 512;
>> >>
>> >>         buf = malloc(size);
>> >>         if (!buf) {
>> >>
>> >> @@ -559,7 +559,7 @@ static int mx28_create_sd_image(int infd, int outfd)
>> >>
>> >>                 goto err0;
>> >>
>> >>         }
>> >>
>> >> -       ret = read(infd, (uint8_t *)buf + 512, fsize);
>> >> +       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
>> >>
>> >>         if (ret != fsize) {
>> >>
>> >>                 ret = -1;
>> >>                 goto err1;
>> >>
>> >> @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int outfd)
>> >>
>> >>         cb->drv_info[0].chip_num = 0x0;
>> >>         cb->drv_info[0].drive_type = 0x0;
>> >>         cb->drv_info[0].tag = 0x1;
>> >>
>> >> -       cb->drv_info[0].first_sector_number = sd_sector + 1;
>> >> -       cb->drv_info[0].sector_count = (size - 1) / 512;
>> >> +       cb->drv_info[0].first_sector_number = sd_sector + 4;
>> >> +       cb->drv_info[0].sector_count = (size - 4) / 512;
>> >>
>> >>         wr_size = write(outfd, buf, size);
>> >>         if (wr_size != size) {
>> >
>> > My mx28evk does not boot with this patch applied.
>>
>> As it does not work in mx28evk I'd prefer to use my previously
>> proposed patch as it keeps clear what is done for mx23 and mx28. What
>> people think?
>
> NAK. Your patch is just adding churn, which the bootrom ignores. Did you manage
> to get reply from FSL why the bootrom ignores it already?

Well; your patch does the same but reuses the churn from mx28. Nobody replied.

--
Otavio Salvador                             O.S. Systems
E-mail: otavio@ossystems.com.br  http://www.ossystems.com.br
Mobile: +55 53 9981-7854              http://projetos.ossystems.com.br
Marek Vasut Feb. 4, 2013, 8:58 p.m. UTC | #6 
Dear Otavio Salvador,

> On Mon, Feb 4, 2013 at 4:46 PM, Marek Vasut <marex@denx.de> wrote:
> > Dear Otavio Salvador,
> > 
> >> On Mon, Feb 4, 2013 at 4:12 PM, Fabio Estevam <festevam@gmail.com> wrote:
> >> > On Thu, Jan 24, 2013 at 4:39 PM, Marek Vasut <marex@denx.de> wrote:
> >> >> PATCH:
> >> >> diff --git a/tools/mxsboot.c b/tools/mxsboot.c
> >> >> index 6c05aa4..d92c39f 100644
> >> >> --- a/tools/mxsboot.c
> >> >> +++ b/tools/mxsboot.c
> >> >> @@ -551,7 +551,7 @@ static int mx28_create_sd_image(int infd, int
> >> >> outfd)
> >> >> 
> >> >>         fsize = lseek(infd, 0, SEEK_END);
> >> >>         lseek(infd, 0, SEEK_SET);
> >> >> 
> >> >> -       size = fsize + 512;
> >> >> +       size = fsize + 4 * 512;
> >> >> 
> >> >>         buf = malloc(size);
> >> >>         if (!buf) {
> >> >> 
> >> >> @@ -559,7 +559,7 @@ static int mx28_create_sd_image(int infd, int
> >> >> outfd)
> >> >> 
> >> >>                 goto err0;
> >> >>         
> >> >>         }
> >> >> 
> >> >> -       ret = read(infd, (uint8_t *)buf + 512, fsize);
> >> >> +       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
> >> >> 
> >> >>         if (ret != fsize) {
> >> >>         
> >> >>                 ret = -1;
> >> >>                 goto err1;
> >> >> 
> >> >> @@ -574,8 +574,8 @@ static int mx28_create_sd_image(int infd, int
> >> >> outfd)
> >> >> 
> >> >>         cb->drv_info[0].chip_num = 0x0;
> >> >>         cb->drv_info[0].drive_type = 0x0;
> >> >>         cb->drv_info[0].tag = 0x1;
> >> >> 
> >> >> -       cb->drv_info[0].first_sector_number = sd_sector + 1;
> >> >> -       cb->drv_info[0].sector_count = (size - 1) / 512;
> >> >> +       cb->drv_info[0].first_sector_number = sd_sector + 4;
> >> >> +       cb->drv_info[0].sector_count = (size - 4) / 512;
> >> >> 
> >> >>         wr_size = write(outfd, buf, size);
> >> >>         if (wr_size != size) {
> >> > 
> >> > My mx28evk does not boot with this patch applied.
> >> 
> >> As it does not work in mx28evk I'd prefer to use my previously
> >> proposed patch as it keeps clear what is done for mx23 and mx28. What
> >> people think?
> > 
> > NAK. Your patch is just adding churn, which the bootrom ignores. Did you
> > manage to get reply from FSL why the bootrom ignores it already?
> 
> Well; your patch does the same but reuses the churn from mx28. Nobody
> replied.

So let's wait for the official reply from FSL. Can you tell me the support 
ticket number please?

Best regards,
Marek Vasut
diff mbox

Patch

diff --git a/tools/mxsboot.c b/tools/mxsboot.c
index 6c05aa4..d92c39f 100644
--- a/tools/mxsboot.c
+++ b/tools/mxsboot.c
@@ -551,7 +551,7 @@  static int mx28_create_sd_image(int infd, int outfd)
 
        fsize = lseek(infd, 0, SEEK_END);
        lseek(infd, 0, SEEK_SET);
-       size = fsize + 512;
+       size = fsize + 4 * 512;
 
        buf = malloc(size);
        if (!buf) {
@@ -559,7 +559,7 @@  static int mx28_create_sd_image(int infd, int outfd)
                goto err0;
        }
 
-       ret = read(infd, (uint8_t *)buf + 512, fsize);
+       ret = read(infd, (uint8_t *)buf + 4 * 512, fsize);
        if (ret != fsize) {
                ret = -1;
                goto err1;
@@ -574,8 +574,8 @@  static int mx28_create_sd_image(int infd, int outfd)
        cb->drv_info[0].chip_num = 0x0;
        cb->drv_info[0].drive_type = 0x0;
        cb->drv_info[0].tag = 0x1;
-       cb->drv_info[0].first_sector_number = sd_sector + 1;
-       cb->drv_info[0].sector_count = (size - 1) / 512;
+       cb->drv_info[0].first_sector_number = sd_sector + 4;
+       cb->drv_info[0].sector_count = (size - 4) / 512;
 
        wr_size = write(outfd, buf, size);
        if (wr_size != size) {