diff mbox series

[6/7] mtd: nand: mxs_nand_spl Fix loop exit condition

Message ID 20220727093748.1415135-7-michael@amarulasolutions.com
State New
Headers
Series NAND new improvements |

Commit Message

Michael Trimarchi July 27, 2022, 9:37 a.m. UTC 
When size is 0 we need to stop the inner loop or we just waste
time to load all the block of the eraseblock

Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
---
 drivers/mtd/nand/raw/mxs_nand_spl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dario Binacchi July 27, 2022, 12:38 p.m. UTC | #1 
Hi Michael,

On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> When size is 0 we need to stop the inner loop or we just waste
> time to load all the block of the eraseblock
>
> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> ---
>  drivers/mtd/nand/raw/mxs_nand_spl.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> index 6d8ec5b3cb..c7ea09e2f9 100644
> --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> @@ -260,10 +260,10 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
>         page_offset = offs % mtd->writesize;
>         nand_page_per_block = mtd->erasesize / mtd->writesize;
>
> -       while (block <= lastblock && size > 0) {
> +       while (block <= lastblock) {
>                 if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
>                         /* Skip bad blocks */
> -                       while (page < nand_page_per_block) {
> +                       while (page < nand_page_per_block && size > 0) {
>                                 int curr_page = nand_page_per_block * block + page;
>
>                                 if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> --
> 2.34.1
>

What about thinking of a patch removing the last_block variable and using the
size > 0 check also for the external loop? IMHO we could use only the size and
block variables, lastblock can be removed, or am I missing something?

Dario
Michael Trimarchi July 27, 2022, 12:41 p.m. UTC | #2 
Hi Dario

On Wed, Jul 27, 2022 at 2:39 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> Hi Michael,
>
> On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > When size is 0 we need to stop the inner loop or we just waste
> > time to load all the block of the eraseblock
> >
> > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > ---
> >  drivers/mtd/nand/raw/mxs_nand_spl.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > index 6d8ec5b3cb..c7ea09e2f9 100644
> > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > @@ -260,10 +260,10 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> >         page_offset = offs % mtd->writesize;
> >         nand_page_per_block = mtd->erasesize / mtd->writesize;
> >
> > -       while (block <= lastblock && size > 0) {
> > +       while (block <= lastblock) {
> >                 if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
> >                         /* Skip bad blocks */
> > -                       while (page < nand_page_per_block) {
> > +                       while (page < nand_page_per_block && size > 0) {
> >                                 int curr_page = nand_page_per_block * block + page;
> >
> >                                 if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> > --
> > 2.34.1
> >
>
> What about thinking of a patch removing the last_block variable and using the
> size > 0 check also for the external loop? IMHO we could use only the size and
> block variables, lastblock can be removed, or am I missing something?

if size is 0 you are already in last block, so when you exit from
inner loop you should be already
out in outer loop. Even with the later patch, this code is totally end of life


Michael

>
> Dario
>
> --
>
> Dario Binacchi
>
> Embedded Linux Developer
>
> dario.binacchi@amarulasolutions.com
>
> __________________________________
>
>
> Amarula Solutions SRL
>
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
>
> T. +39 042 243 5310
> info@amarulasolutions.com
>
> www.amarulasolutions.com
Michael Trimarchi July 27, 2022, 12:44 p.m. UTC | #3 
Hi

Anyway let me check :D

Michael

On Wed, Jul 27, 2022 at 2:41 PM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi Dario
>
> On Wed, Jul 27, 2022 at 2:39 PM Dario Binacchi
> <dario.binacchi@amarulasolutions.com> wrote:
> >
> > Hi Michael,
> >
> > On Wed, Jul 27, 2022 at 11:38 AM Michael Trimarchi
> > <michael@amarulasolutions.com> wrote:
> > >
> > > When size is 0 we need to stop the inner loop or we just waste
> > > time to load all the block of the eraseblock
> > >
> > > Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > ---
> > >  drivers/mtd/nand/raw/mxs_nand_spl.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > > index 6d8ec5b3cb..c7ea09e2f9 100644
> > > --- a/drivers/mtd/nand/raw/mxs_nand_spl.c
> > > +++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
> > > @@ -260,10 +260,10 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> > >         page_offset = offs % mtd->writesize;
> > >         nand_page_per_block = mtd->erasesize / mtd->writesize;
> > >
> > > -       while (block <= lastblock && size > 0) {
> > > +       while (block <= lastblock) {
> > >                 if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
> > >                         /* Skip bad blocks */
> > > -                       while (page < nand_page_per_block) {
> > > +                       while (page < nand_page_per_block && size > 0) {
> > >                                 int curr_page = nand_page_per_block * block + page;
> > >
> > >                                 if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {
> > > --
> > > 2.34.1
> > >
> >
> > What about thinking of a patch removing the last_block variable and using the
> > size > 0 check also for the external loop? IMHO we could use only the size and
> > block variables, lastblock can be removed, or am I missing something?
>
> if size is 0 you are already in last block, so when you exit from
> inner loop you should be already
> out in outer loop. Even with the later patch, this code is totally end of life
>
>
> Michael
>
> >
> > Dario
> >
> > --
> >
> > Dario Binacchi
> >
> > Embedded Linux Developer
> >
> > dario.binacchi@amarulasolutions.com
> >
> > __________________________________
> >
> >
> > Amarula Solutions SRL
> >
> > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> >
> > T. +39 042 243 5310
> > info@amarulasolutions.com
> >
> > www.amarulasolutions.com
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com
diff mbox series

Patch

diff --git a/drivers/mtd/nand/raw/mxs_nand_spl.c b/drivers/mtd/nand/raw/mxs_nand_spl.c
index 6d8ec5b3cb..c7ea09e2f9 100644
--- a/drivers/mtd/nand/raw/mxs_nand_spl.c
+++ b/drivers/mtd/nand/raw/mxs_nand_spl.c
@@ -260,10 +260,10 @@  int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
 	page_offset = offs % mtd->writesize;
 	nand_page_per_block = mtd->erasesize / mtd->writesize;
 
-	while (block <= lastblock && size > 0) {
+	while (block <= lastblock) {
 		if (!is_badblock(mtd, mtd->erasesize * block, 1)) {
 			/* Skip bad blocks */
-			while (page < nand_page_per_block) {
+			while (page < nand_page_per_block && size > 0) {
 				int curr_page = nand_page_per_block * block + page;
 
 				if (mxs_read_page_ecc(mtd, page_buf, curr_page) < 0) {