public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] sf: Stop leaking memory
@ 2014-06-12 20:53 Marek Vasut
  2014-07-03 20:24 ` Jagan Teki
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2014-06-12 20:53 UTC (permalink / raw)
  To: u-boot

It's usually a common pattern to free() the memory that we allocated.
Implement this here to stop leaking memory. Also, add a debug output
when BAR configuration fails to follow suit.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
---
 drivers/mtd/spi/sf_ops.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

NOTE: I think we can do without the memory allocation here altogether.
      Is there any upper limit on the number of dummy bytes that can
      go with a SF command? If so, we can just allocate that buffer on
      a stack and be done with it.

diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
index ef91b92..29a7867 100644
--- a/drivers/mtd/spi/sf_ops.c
+++ b/drivers/mtd/spi/sf_ops.c
@@ -398,8 +398,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 #endif
 #ifdef CONFIG_SPI_FLASH_BAR
 		bank_sel = spi_flash_bank(flash, read_addr);
-		if (bank_sel < 0)
-			return ret;
+		if (bank_sel < 0) {
+			debug("SF: bank select failed\n");
+			break;
+		}
 #endif
 		remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
 				(bank_sel + 1)) - offset;
@@ -421,6 +423,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
 		data += read_len;
 	}
 
+	free(cmd);
 	return ret;
 }
 
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH] sf: Stop leaking memory
  2014-06-12 20:53 [U-Boot] [PATCH] sf: Stop leaking memory Marek Vasut
@ 2014-07-03 20:24 ` Jagan Teki
  2014-07-03 21:04   ` Marek Vasut
  0 siblings, 1 reply; 4+ messages in thread
From: Jagan Teki @ 2014-07-03 20:24 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 13, 2014 at 2:23 AM, Marek Vasut <marex@denx.de> wrote:
> It's usually a common pattern to free() the memory that we allocated.
> Implement this here to stop leaking memory. Also, add a debug output
> when BAR configuration fails to follow suit.
>
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Michal Simek <michal.simek@xilinx.com>
> Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> ---
>  drivers/mtd/spi/sf_ops.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> NOTE: I think we can do without the memory allocation here altogether.
>       Is there any upper limit on the number of dummy bytes that can
>       go with a SF command? If so, we can just allocate that buffer on
>       a stack and be done with it.
>
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index ef91b92..29a7867 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -398,8 +398,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>  #endif
>  #ifdef CONFIG_SPI_FLASH_BAR
>                 bank_sel = spi_flash_bank(flash, read_addr);
> -               if (bank_sel < 0)
> -                       return ret;
> +               if (bank_sel < 0) {
> +                       debug("SF: bank select failed\n");
> +                       break;
> +               }

This may not require, as definition have it already when fail to set bank.

>  #endif
>                 remain_len = ((SPI_FLASH_16MB_BOUN << flash->shift) *
>                                 (bank_sel + 1)) - offset;
> @@ -421,6 +423,7 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset,
>                 data += read_len;
>         }
>
> +       free(cmd);
>         return ret;
>  }
>
> --
> 2.0.0.rc2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

thanks!
-- 
Jagan.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH] sf: Stop leaking memory
  2014-07-03 20:24 ` Jagan Teki
@ 2014-07-03 21:04   ` Marek Vasut
  2014-07-13 14:43     ` Jagan Teki
  0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2014-07-03 21:04 UTC (permalink / raw)
  To: u-boot

On Thursday, July 03, 2014 at 10:24:44 PM, Jagan Teki wrote:
> On Fri, Jun 13, 2014 at 2:23 AM, Marek Vasut <marex@denx.de> wrote:
> > It's usually a common pattern to free() the memory that we allocated.
> > Implement this here to stop leaking memory. Also, add a debug output
> > when BAR configuration fails to follow suit.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Michal Simek <michal.simek@xilinx.com>
> > Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
> > ---
> > 
> >  drivers/mtd/spi/sf_ops.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > NOTE: I think we can do without the memory allocation here altogether.
> > 
> >       Is there any upper limit on the number of dummy bytes that can
> >       go with a SF command? If so, we can just allocate that buffer on
> >       a stack and be done with it.
> > 
> > diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> > index ef91b92..29a7867 100644
> > --- a/drivers/mtd/spi/sf_ops.c
> > +++ b/drivers/mtd/spi/sf_ops.c
> > @@ -398,8 +398,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash,
> > u32 offset,
> > 
> >  #endif
> >  #ifdef CONFIG_SPI_FLASH_BAR
> >  
> >                 bank_sel = spi_flash_bank(flash, read_addr);
> > 
> > -               if (bank_sel < 0)
> > -                       return ret;
> > +               if (bank_sel < 0) {
> > +                       debug("SF: bank select failed\n");
> > +                       break;
> > +               }
> 
> This may not require, as definition have it already when fail to set bank.

Feel free to drop this when applying.

Btw. the information printing is quite inconsistent in this stuff.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [U-Boot] [PATCH] sf: Stop leaking memory
  2014-07-03 21:04   ` Marek Vasut
@ 2014-07-13 14:43     ` Jagan Teki
  0 siblings, 0 replies; 4+ messages in thread
From: Jagan Teki @ 2014-07-13 14:43 UTC (permalink / raw)
  To: u-boot

On Fri, Jul 4, 2014 at 2:34 AM, Marek Vasut <marex@denx.de> wrote:
> On Thursday, July 03, 2014 at 10:24:44 PM, Jagan Teki wrote:
>> On Fri, Jun 13, 2014 at 2:23 AM, Marek Vasut <marex@denx.de> wrote:
>> > It's usually a common pattern to free() the memory that we allocated.
>> > Implement this here to stop leaking memory. Also, add a debug output
>> > when BAR configuration fails to follow suit.
>> >
>> > Signed-off-by: Marek Vasut <marex@denx.de>
>> > Cc: Michal Simek <michal.simek@xilinx.com>
>> > Cc: Jagannadha Sutradharudu Teki <jaganna@xilinx.com>
>> > ---
>> >
>> >  drivers/mtd/spi/sf_ops.c | 7 +++++--
>> >  1 file changed, 5 insertions(+), 2 deletions(-)
>> >
>> > NOTE: I think we can do without the memory allocation here altogether.
>> >
>> >       Is there any upper limit on the number of dummy bytes that can
>> >       go with a SF command? If so, we can just allocate that buffer on
>> >       a stack and be done with it.
>> >
>> > diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
>> > index ef91b92..29a7867 100644
>> > --- a/drivers/mtd/spi/sf_ops.c
>> > +++ b/drivers/mtd/spi/sf_ops.c
>> > @@ -398,8 +398,10 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash,
>> > u32 offset,
>> >
>> >  #endif
>> >  #ifdef CONFIG_SPI_FLASH_BAR
>> >
>> >                 bank_sel = spi_flash_bank(flash, read_addr);
>> >
>> > -               if (bank_sel < 0)
>> > -                       return ret;
>> > +               if (bank_sel < 0) {
>> > +                       debug("SF: bank select failed\n");
>> > +                       break;
>> > +               }
>>
>> This may not require, as definition have it already when fail to set bank.
>
> Feel free to drop this when applying.
>
> Btw. the information printing is quite inconsistent in this stuff.

Applied to u-boot-spi/master

thanks!
-- 
Jagan.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-07-13 14:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-12 20:53 [U-Boot] [PATCH] sf: Stop leaking memory Marek Vasut
2014-07-03 20:24 ` Jagan Teki
2014-07-03 21:04   ` Marek Vasut
2014-07-13 14:43     ` Jagan Teki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox