[coreboot] [PATCH] flashrom: something

[Carl-Daniel Hailfinger]

On 16.05.2008 03:30, Peter Stuge wrote:
> On Fri, May 16, 2008 at 03:10:10AM +0200, Stefan Reinauer wrote:
>   
>>> OK, I forgot what this patch does, but I'm pretty sure it's justified.
>>>       
>> Fine. Please resend, when you figured it out.
>>
>> Until then, NACK.
>>     
>
> NAK from me too.
>   

OK, I figured one part out:
Drop erase_sector_39sf020() which is an identical copy of
erase_sector_28sf040().

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: sst39sf020.c
===================================================================
--- sst39sf020.c	(Revision 3326)
+++ sst39sf020.c	(Arbeitskopie)
@@ -22,21 +22,6 @@
 #include <stdint.h>
 #include "flash.h"
 
-#define AUTO_PG_ERASE1		0x20
-#define AUTO_PG_ERASE2		0xD0
-
-static __inline__ int erase_sector_39sf020(volatile uint8_t *bios,
-					   unsigned long address)
-{
-	*bios = AUTO_PG_ERASE1;
-	*(bios + address) = AUTO_PG_ERASE2;
-
-	/* wait for Toggle bit ready         */
-	toggle_ready_jedec(bios);
-
-	return 0;
-}
-
 int write_39sf020(struct flashchip *flash, uint8_t *buf)
 {
 	int i;



> Looking at the patch it does not change flashrom behavior in any way:
>
> It removes one function, sst39sf020.c:erase_sector_39sf020()
>   

Explained above.

> It makes two functions public in flash.h
>   

Dropped.

> It changes sst28sf040.c:erase_sector_28sf040() from static to public
>   

And it removes __inline__. This brings erase_sector_28sf040() more in
line with the style of most other chip-specific functions in the
flashrom tree.

> It adds sst28sf040.c:erase_all_sectors_28sf040()
>   

That's a preparation for better per-sector erasing.

> It does not add or change calls to any of these functions, so
> apparently they are not used and not important at the moment.
>
> Maybe erase_sector_39sf020() does not work. After testing, I could
> agree to remove it.
>   

No, it just was a duplicate.

> The static vs. public functions simply do not seem to matter. AFAIK
> sector erasing is never done outside the chip drivers though? I think
> that's a good thing.
>
> Please try to find why these changes came about.
>   
And the other part:
Add a per-sector chip erase function for SST 28SF* and compatible.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: sst28sf040.c
===================================================================
--- sst28sf040.c	(Revision 3326)
+++ sst28sf040.c	(Arbeitskopie)
@@ -58,8 +58,7 @@
 	tmp = *(volatile uint8_t *)(bios + 0x041A);
 }
 
-static __inline__ int erase_sector_28sf040(volatile uint8_t *bios,
-					   unsigned long address)
+int erase_sector_28sf040(volatile uint8_t *bios, unsigned long address)
 {
 	*bios = AUTO_PG_ERASE1;
 	*(bios + address) = AUTO_PG_ERASE2;
@@ -119,6 +118,20 @@
 	return 0;
 }
 
+int erase_all_sectors_28sf040(struct flashchip *flash)
+{
+	int i;
+	int total_size = flash->total_size * 1024;
+	int page_size = flash->page_size;
+	volatile uint8_t *bios = flash->virtual_memory;
+
+	for (i = 0; i < total_size / page_size; i++) {
+		/* erase the page before programming */
+		erase_sector_28sf040(bios, i * page_size);
+	}
+	return 0;
+}
+
 int erase_28sf040(struct flashchip *flash)
 {
 	volatile uint8_t *bios = flash->virtual_memory;