[PATCH] Kill hardcoded block erase on ICH SPI

[Carl-Daniel Hailfinger]

Kill hardcoded block erase on ICH SPI.
The existing code does not work for all SPI chips, and it just was a
band-aid to cope with locked down chipsets back in a time when there was
no eraseblock infrastructure.
Basically, this unbreaks a few SPI chips on ICH.

Signed-off-by: Carl-Daniel Hailfinger

Index: flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c
===================================================================
--- flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Revision 769)
+++ flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Arbeitskopie)
@@ -658,20 +658,16 @@
    int maxdata = 64;

spi_disable_blockprotect();
+	/* Erase first */
+	printf("Erasing flash before programming... ");
+	if (erase_flash(flash)) {
+		fprintf(stderr, "ERASE FAILED!\n");
+		return -1;
+	}
+	printf("done.\n");

printf("Programming page: \n");
-
    for (i = 0; i < total_size / erase_size; i++) {
-		/* FIMXE: call the chip-specific spi_block_erase_XX instead.
-		 * For this, we need to add a block erase function to
-		 * struct flashchip.
-		 */
-		rc = spi_block_erase_d8(flash, i * erase_size, erase_size);
-		if (rc) {
-			printf("Error erasing block at 0x%x\n", i);
-			break;
-		}
-
    	if (spi_controller == SPI_CONTROLLER_VIA)
    		maxdata = 16;

[Maciej Pijanka]

On Mon, 23 Nov 2009, Carl-Daniel Hailfinger wrote:
> Kill hardcoded block erase on ICH SPI.
> The existing code does not work for all SPI chips, and it just was a
> band-aid to cope with locked down chipsets back in a time when there was
> no eraseblock infrastructure.
> Basically, this unbreaks a few SPI chips on ICH.

Looks sane for me, but i don't have any ICH/SPI hardware to check, idea seems
to be right too, because it replace hardcoded one-of-erase-types with chip-based
erase.

Acked-by: Maciej Pijanka
> Signed-off-by: Carl-Daniel Hailfinger
> Index: flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c
>
> --- flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Revision 769)
> +++ flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Arbeitskopie)

[Adam Jurkowski]

Sorry for not posting to your original post, but I don't have it on my 
inbox...

I would still use "spi_block_erase_d8()" on the flashes where it is 
defined but replaced hard-coded call with "flash->erase_block[n]()".
However, erasing the whole flash is required if block_erasers is NULL. 
Then flash->erase() seems to fit better.

But maybe it's the solution for the future as the patch seems to work 
for all the boards at least.

best regards,
Adam

On 2009-11-24 15:51, Maciej Pijanka wrote:
> On Mon, 23 Nov 2009, Carl-Daniel Hailfinger wrote:
> > Kill hardcoded block erase on ICH SPI.
> > The existing code does not work for all SPI chips, and it just was a
> > band-aid to cope with locked down chipsets back in a time when there was
> > no eraseblock infrastructure.
> > Basically, this unbreaks a few SPI chips on ICH.
>
> Looks sane for me, but i don't have any ICH/SPI hardware to check, idea seems
> to be right too, because it replace hardcoded one-of-erase-types with chip-based
> erase.
>
> Acked-by: Maciej Pijanka
> > Signed-off-by: Carl-Daniel Hailfinger
> > Index: flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c
> >
> > --- flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Revision 769)
> > +++ flashrom-ich_spi_no_hardcoded_block_erase/ichspi.c	(Arbeitskop

[Carl-Daniel Hailfinger]

On 24.11.2009 18:47, Adam Jurkowski wrote:
> Sorry for not posting to your original post, but I don't have it on my 
> inbox...

No problem.
> I would still use "spi_block_erase_d8()" on the flashes where it is 
> defined but replaced hard-coded call with "flash->erase_block[n]()".
> However, erasing the whole flash is required if block_erasers is NULL. 
> Then flash->erase() seems to fit better.
>
> But maybe it's the solution for the future as the patch seems to work 
> for all the boards at least.

My current plan is to pick one of the blockwise flashing functions in
chip code, make it generic and call chip drivers from the generic code.
That way, chip drivers only have to provide a function which writes a
single block and a function which erases a single block, and the generic
code will walk each erase block and do the right thing (call
erase_block_foo and write_block_foo). That will simplify the chip
drivers a lot.

Regards,
Carl-Daniel

[Carl-Daniel Hailfinger]

On 24.11.2009 15:51, Maciej Pijanka wrote:
> On Mon, 23 Nov 2009, Carl-Daniel Hailfinger wrote:
> > Kill hardcoded block erase on ICH SPI.
> > The existing code does not work for all SPI chips, and it just was a
> > band-aid to cope with locked down chipsets back in a time when there was
> > no eraseblock infrastructure.
> > Basically, this unbreaks a few SPI chips on ICH.
>
> Looks sane for me, but i don't have any ICH/SPI hardware to check, idea seems
> to be right too, because it replace hardcoded one-of-erase-types with chip-based
> erase.
>
> Acked-by: Maciej Pijanka

Thanks, r777.

Regards,
Carl-Daniel