diff mbox series

[v1,2/2] imx: spl_imx_romapi: fix emmc fast boot mode case

Message ID 20231026073220.244387-3-marcel@ziswiler.com
State Accepted
Commit ac33a7976a5631e05fdb8f6c75be5f824dcf5229
Delegated to: Tom Rini
Headers show
Series imx: spl_imx_romapi: fixes | expand

Commit Message

Marcel Ziswiler Oct. 26, 2023, 7:32 a.m. UTC
From: Marcel Ziswiler <marcel.ziswiler@toradex.com>

This fixes a regression in the eMMC fast boot mode case where the buffer
was missing 464 bytes.

The code figures out how many bytes must at least be fetched to honor
the current read, rounds that up to the ss->pagesize [which is a no-op
in the USB download case because that has ->pagesize==1], fetches that
many bytes, but then recorded the original upper bound as the new end of
the valid data. However, this did not take into account the rounding up
to the ss->pagesize. Fix this by recording the actual bytes downloaded.

Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

---

 arch/arm/mach-imx/spl_imx_romapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rasmus Villemoes Oct. 26, 2023, 8:29 a.m. UTC | #1
On 26/10/2023 09.32, Marcel Ziswiler wrote:
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
> This fixes a regression in the eMMC fast boot mode case where the buffer
> was missing 464 bytes.
> 
> The code figures out how many bytes must at least be fetched to honor
> the current read, rounds that up to the ss->pagesize [which is a no-op
> in the USB download case because that has ->pagesize==1], fetches that
> many bytes, but then recorded the original upper bound as the new end of
> the valid data. However, this did not take into account the rounding up
> to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> 
> Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 

Thanks for reporting and fixing this, and sorry for the trouble.

Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Stefano Babic Oct. 26, 2023, 8:36 a.m. UTC | #2
On 26.10.23 10:29, Rasmus Villemoes wrote:
> On 26/10/2023 09.32, Marcel Ziswiler wrote:
>> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>
>> This fixes a regression in the eMMC fast boot mode case where the buffer
>> was missing 464 bytes.
>>
>> The code figures out how many bytes must at least be fetched to honor
>> the current read, rounds that up to the ss->pagesize [which is a no-op
>> in the USB download case because that has ->pagesize==1], fetches that
>> many bytes, but then recorded the original upper bound as the new end of
>> the valid data. However, this did not take into account the rounding up
>> to the ss->pagesize. Fix this by recording the actual bytes downloaded.
>>
>> Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
>> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>
> 
> Thanks for reporting and fixing this, and sorry for the trouble.
> 
> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> 

And thanks for all your work - I will apply this fix soon.

Regards,
Stefano
Fabio Estevam Oct. 26, 2023, 2:19 p.m. UTC | #3
On Thu, Oct 26, 2023 at 4:32 AM Marcel Ziswiler <marcel@ziswiler.com> wrote:
>
> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>
> This fixes a regression in the eMMC fast boot mode case where the buffer
> was missing 464 bytes.
>
> The code figures out how many bytes must at least be fetched to honor
> the current read, rounds that up to the ss->pagesize [which is a no-op
> in the USB download case because that has ->pagesize==1], fetches that
> many bytes, but then recorded the original upper bound as the new end of
> the valid data. However, this did not take into account the rounding up
> to the ss->pagesize. Fix this by recording the actual bytes downloaded.
>
> Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>

Thanks for testing mainline U-Boot and fixing it:

Reviewed-by: Fabio Estevam <festevam@gmail.com>
Marcel Ziswiler Nov. 2, 2023, 7:10 a.m. UTC | #4
Hi Stefano

On Thu, 2023-10-26 at 10:36 +0200, Stefano Babic wrote:
> On 26.10.23 10:29, Rasmus Villemoes wrote:
> > On 26/10/2023 09.32, Marcel Ziswiler wrote:
> > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > 
> > > This fixes a regression in the eMMC fast boot mode case where the buffer
> > > was missing 464 bytes.
> > > 
> > > The code figures out how many bytes must at least be fetched to honor
> > > the current read, rounds that up to the ss->pagesize [which is a no-op
> > > in the USB download case because that has ->pagesize==1], fetches that
> > > many bytes, but then recorded the original upper bound as the new end of
> > > the valid data. However, this did not take into account the rounding up
> > > to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> > > 
> > > Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT
> > > size")
> > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > 
> > 
> > Thanks for reporting and fixing this, and sorry for the trouble.
> > 
> > Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > 
> 
> And thanks for all your work - I will apply this fix soon.

Any news on applying this fix? Note that master still stays broken as of today.

> Regards,
> Stefano

Cheers

Marcel
Marcel Ziswiler Nov. 16, 2023, 10:57 a.m. UTC | #5
Hi Tom

On Thu, 2023-11-02 at 08:09 +0100, Marcel Ziswiler wrote:
> Hi Stefano
> 
> On Thu, 2023-10-26 at 10:36 +0200, Stefano Babic wrote:
> > On 26.10.23 10:29, Rasmus Villemoes wrote:
> > > On 26/10/2023 09.32, Marcel Ziswiler wrote:
> > > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > 
> > > > This fixes a regression in the eMMC fast boot mode case where the buffer
> > > > was missing 464 bytes.
> > > > 
> > > > The code figures out how many bytes must at least be fetched to honor
> > > > the current read, rounds that up to the ss->pagesize [which is a no-op
> > > > in the USB download case because that has ->pagesize==1], fetches that
> > > > many bytes, but then recorded the original upper bound as the new end of
> > > > the valid data. However, this did not take into account the rounding up
> > > > to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> > > > 
> > > > Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT
> > > > size")
> > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > 
> > > 
> > > Thanks for reporting and fixing this, and sorry for the trouble.
> > > 
> > > Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > > 
> > 
> > And thanks for all your work - I will apply this fix soon.
> 
> Any news on applying this fix? Note that master still stays broken as of today.

And two weeks later still broken!

Anyway, I feel i.MX maintainership seems rather stalled at times most of the time. I don't want to criticise
anybody knowing how busy our trade can get. However, if it helps the cause Toradex would be ready to step up.
Let me know what you think. Thanks!

> > Regards,
> > Stefano

Cheers

Marcel
Tom Rini Nov. 16, 2023, 6:42 p.m. UTC | #6
On Thu, Nov 16, 2023 at 10:57:27AM +0000, Marcel Ziswiler wrote:
> Hi Tom
> 
> On Thu, 2023-11-02 at 08:09 +0100, Marcel Ziswiler wrote:
> > Hi Stefano
> > 
> > On Thu, 2023-10-26 at 10:36 +0200, Stefano Babic wrote:
> > > On 26.10.23 10:29, Rasmus Villemoes wrote:
> > > > On 26/10/2023 09.32, Marcel Ziswiler wrote:
> > > > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > > 
> > > > > This fixes a regression in the eMMC fast boot mode case where the buffer
> > > > > was missing 464 bytes.
> > > > > 
> > > > > The code figures out how many bytes must at least be fetched to honor
> > > > > the current read, rounds that up to the ss->pagesize [which is a no-op
> > > > > in the USB download case because that has ->pagesize==1], fetches that
> > > > > many bytes, but then recorded the original upper bound as the new end of
> > > > > the valid data. However, this did not take into account the rounding up
> > > > > to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> > > > > 
> > > > > Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT
> > > > > size")
> > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > > 
> > > > 
> > > > Thanks for reporting and fixing this, and sorry for the trouble.
> > > > 
> > > > Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > > > 
> > > 
> > > And thanks for all your work - I will apply this fix soon.
> > 
> > Any news on applying this fix? Note that master still stays broken as of today.
> 
> And two weeks later still broken!

I've put this in my not to forget queue for the next -rc.

> Anyway, I feel i.MX maintainership seems rather stalled at times most of the time. I don't want to criticise
> anybody knowing how busy our trade can get. However, if it helps the cause Toradex would be ready to step up.
> Let me know what you think. Thanks!

I would be happy to see an additional custodian for the imx tree, if
that's agreeable with Stefano.
Stefano Babic Nov. 17, 2023, 8:17 a.m. UTC | #7
Hi Tom, Marcel,

On 16.11.23 19:42, Tom Rini wrote:
> On Thu, Nov 16, 2023 at 10:57:27AM +0000, Marcel Ziswiler wrote:
>> Hi Tom
>>
>> On Thu, 2023-11-02 at 08:09 +0100, Marcel Ziswiler wrote:
>>> Hi Stefano
>>>
>>> On Thu, 2023-10-26 at 10:36 +0200, Stefano Babic wrote:
>>>> On 26.10.23 10:29, Rasmus Villemoes wrote:
>>>>> On 26/10/2023 09.32, Marcel Ziswiler wrote:
>>>>>> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>>>>>
>>>>>> This fixes a regression in the eMMC fast boot mode case where the buffer
>>>>>> was missing 464 bytes.
>>>>>>
>>>>>> The code figures out how many bytes must at least be fetched to honor
>>>>>> the current read, rounds that up to the ss->pagesize [which is a no-op
>>>>>> in the USB download case because that has ->pagesize==1], fetches that
>>>>>> many bytes, but then recorded the original upper bound as the new end of
>>>>>> the valid data. However, this did not take into account the rounding up
>>>>>> to the ss->pagesize. Fix this by recording the actual bytes downloaded.
>>>>>>
>>>>>> Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT
>>>>>> size")
>>>>>> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>>>>>>
>>>>>
>>>>> Thanks for reporting and fixing this, and sorry for the trouble.
>>>>>
>>>>> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
>>>>>
>>>>
>>>> And thanks for all your work - I will apply this fix soon.
>>>
>>> Any news on applying this fix? Note that master still stays broken as of today.
>>
>> And two weeks later still broken!
> 
> I've put this in my not to forget queue for the next -rc.
> 
>> Anyway, I feel i.MX maintainership seems rather stalled at times most of the time. I don't want to criticise
>> anybody knowing how busy our trade can get. However, if it helps the cause Toradex would be ready to step up.
>> Let me know what you think. Thanks!
> 
> I would be happy to see an additional custodian for the imx tree, if
> that's agreeable with Stefano.

I have rather less time for U-Boot and it won't be better in the near 
future. Fabio is co-maintainer and he has accepted to take over the 
repository and go on maintaining u-boot-imx. Just let him some time to 
be confident with patchwork and CI.

Thanks,
Stefano
Tom Rini Nov. 17, 2023, 2:25 p.m. UTC | #8
On Fri, Nov 17, 2023 at 09:17:27AM +0100, Stefano Babic wrote:
> Hi Tom, Marcel,
> 
> On 16.11.23 19:42, Tom Rini wrote:
> > On Thu, Nov 16, 2023 at 10:57:27AM +0000, Marcel Ziswiler wrote:
> > > Hi Tom
> > > 
> > > On Thu, 2023-11-02 at 08:09 +0100, Marcel Ziswiler wrote:
> > > > Hi Stefano
> > > > 
> > > > On Thu, 2023-10-26 at 10:36 +0200, Stefano Babic wrote:
> > > > > On 26.10.23 10:29, Rasmus Villemoes wrote:
> > > > > > On 26/10/2023 09.32, Marcel Ziswiler wrote:
> > > > > > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > > > > 
> > > > > > > This fixes a regression in the eMMC fast boot mode case where the buffer
> > > > > > > was missing 464 bytes.
> > > > > > > 
> > > > > > > The code figures out how many bytes must at least be fetched to honor
> > > > > > > the current read, rounds that up to the ss->pagesize [which is a no-op
> > > > > > > in the USB download case because that has ->pagesize==1], fetches that
> > > > > > > many bytes, but then recorded the original upper bound as the new end of
> > > > > > > the valid data. However, this did not take into account the rounding up
> > > > > > > to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> > > > > > > 
> > > > > > > Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT
> > > > > > > size")
> > > > > > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > > > > > > 
> > > > > > 
> > > > > > Thanks for reporting and fixing this, and sorry for the trouble.
> > > > > > 
> > > > > > Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > > > > > 
> > > > > 
> > > > > And thanks for all your work - I will apply this fix soon.
> > > > 
> > > > Any news on applying this fix? Note that master still stays broken as of today.
> > > 
> > > And two weeks later still broken!
> > 
> > I've put this in my not to forget queue for the next -rc.
> > 
> > > Anyway, I feel i.MX maintainership seems rather stalled at times most of the time. I don't want to criticise
> > > anybody knowing how busy our trade can get. However, if it helps the cause Toradex would be ready to step up.
> > > Let me know what you think. Thanks!
> > 
> > I would be happy to see an additional custodian for the imx tree, if
> > that's agreeable with Stefano.
> 
> I have rather less time for U-Boot and it won't be better in the near
> future. Fabio is co-maintainer and he has accepted to take over the
> repository and go on maintaining u-boot-imx. Just let him some time to be
> confident with patchwork and CI.

OK. Fabio, please send me your patchwork username off-list and I'll get
you added and you can start managing patches there. I've started using
some of the b4 + patchwork integration just recently and it's very nice.
Fabio Estevam Nov. 17, 2023, 4:13 p.m. UTC | #9
Hi Tom,

On Fri, Nov 17, 2023 at 11:25 AM Tom Rini <trini@konsulko.com> wrote:

> OK. Fabio, please send me your patchwork username off-list and I'll get
> you added and you can start managing patches there. I've started using
> some of the b4 + patchwork integration just recently and it's very nice.

In the meantime, could you pleas directly apply this series from Marcel?

Thanks
Tom Rini Nov. 17, 2023, 4:46 p.m. UTC | #10
On Fri, Nov 17, 2023 at 01:13:07PM -0300, Fabio Estevam wrote:
> Hi Tom,
> 
> On Fri, Nov 17, 2023 at 11:25 AM Tom Rini <trini@konsulko.com> wrote:
> 
> > OK. Fabio, please send me your patchwork username off-list and I'll get
> > you added and you can start managing patches there. I've started using
> > some of the b4 + patchwork integration just recently and it's very nice.
> 
> In the meantime, could you pleas directly apply this series from Marcel?

I've put it in my list for before the next -rc.
Tom Rini Nov. 17, 2023, 7:42 p.m. UTC | #11
On Thu, Oct 26, 2023 at 09:32:20AM +0200, Marcel Ziswiler wrote:

> From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> 
> This fixes a regression in the eMMC fast boot mode case where the buffer
> was missing 464 bytes.
> 
> The code figures out how many bytes must at least be fetched to honor
> the current read, rounds that up to the ss->pagesize [which is a no-op
> in the USB download case because that has ->pagesize==1], fetches that
> many bytes, but then recorded the original upper bound as the new end of
> the valid data. However, this did not take into account the rounding up
> to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> 
> Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
> Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> Reviewed-by: Fabio Estevam <festevam@gmail.com>

Applied to u-boot/master, thanks!
Marcel Ziswiler Nov. 20, 2023, 8:08 a.m. UTC | #12
On Fri, 2023-11-17 at 14:42 -0500, Tom Rini wrote:
> On Thu, Oct 26, 2023 at 09:32:20AM +0200, Marcel Ziswiler wrote:
> 
> > From: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > 
> > This fixes a regression in the eMMC fast boot mode case where the buffer
> > was missing 464 bytes.
> > 
> > The code figures out how many bytes must at least be fetched to honor
> > the current read, rounds that up to the ss->pagesize [which is a no-op
> > in the USB download case because that has ->pagesize==1], fetches that
> > many bytes, but then recorded the original upper bound as the new end of
> > the valid data. However, this did not take into account the rounding up
> > to the ss->pagesize. Fix this by recording the actual bytes downloaded.
> > 
> > Fixes: 4b4472438f5a ("imx: spl_imx_romapi: avoid tricky use of spl_load_simple_fit() to get full FIT size")
> > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> > Acked-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> > Reviewed-by: Fabio Estevam <festevam@gmail.com>
> 
> Applied to u-boot/master, thanks!

Finally our weekly master/upstream CI is passing on all i.MX 8M Plus SKUs again!

Thanks!

Cheers

Marcel
diff mbox series

Patch

diff --git a/arch/arm/mach-imx/spl_imx_romapi.c b/arch/arm/mach-imx/spl_imx_romapi.c
index cb220869b50..5eb5a3d3c4a 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -162,7 +162,7 @@  static ulong spl_romapi_read_stream(struct spl_load_info *load, ulong sector,
 			return 0;
 		}
 
-		ss->end = end;
+		ss->end += bytes;
 	}
 
 	memcpy(buf, (void *)(sector), count);