Skip to contentSkip to conversation

sf: Stop leaking memory

PATCH New Marek Vasut 2 participants 4 messages Jun 12, 2014 – Jul 13, 2014 Memory technology devices (MTD)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 sf: Stop leaking memory

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.

Cc Michal Simek <[email protected]>
Cc Jagannadha Sutradharudu Teki <[email protected]>
---
 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.

drivers/mtd/spi/sf_ops.c +5 -2
index ef91b92..29a7867 100644
@@ -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;
 }
 
Signature · 1 line
2.0.0.rc2
#1 lore raw

PATCH 1/1 sf: Stop leaking memory

Quoted 21 lines · Marek Vasut

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.

Cc Michal Simek <[email protected]>
Cc Jagannadha Sutradharudu Teki <[email protected]>
---
 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.

drivers/mtd/spi/sf_ops.c +0 -0
index ef91b92..29a7867 100644

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, · line 404

Quoted 9 lines
drivers/mtd/spi/sf_ops.c +4 -2
@@ -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.

Quoted 14 lines
drivers/mtd/spi/sf_ops.c +1 -1
 #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

Quoted 4 lines

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

thanks!

Signature · 1 line
Jagan.
#2 lore raw

PATCH 1/1 sf: Stop leaking memory

Quoted 31 lines · Jagan Teki
Quoted 30 lines · Marek Vasut

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.

Cc Michal Simek <[email protected]>
Cc Jagannadha Sutradharudu Teki <[email protected]>

---

 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.

drivers/mtd/spi/sf_ops.c +0 -0
index ef91b92..29a7867 100644
@@ -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);
 
Quoted 8 lines

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, · line 404

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +4 -2
-               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

#3 lore raw

PATCH 1/1 sf: Stop leaking memory

Quoted 41 lines · Marek Vasut
Quoted 31 lines · Jagan Teki
Quoted 30 lines · Marek Vasut

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.

Cc Michal Simek <[email protected]>
Cc Jagannadha Sutradharudu Teki <[email protected]>

---

 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.

drivers/mtd/spi/sf_ops.c +0 -0
index ef91b92..29a7867 100644
@@ -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);
 
Quoted 8 lines

drivers/mtd/spi/sf_ops.c int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, · line 404

Quoted 6 lines
drivers/mtd/spi/sf_ops.c +4 -2
-               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.

Quoted 3 lines

Feel free to drop this when applying.

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

Applied to u-boot-spi/master

thanks!

Signature · 1 line
Jagan.