| Message ID | 1402606438-7602-1-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
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@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot thanks!
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
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!
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; }
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.