Re: [coreboot] flashrom: Remove dead M-Systems Disk on Chip code

[Peter Stuge]

On Sun, Jun 22, 2008 at 12:24:25PM +0200, Stefan Reinauer wrote:
> > The write function does nothing but print text. It has a call to
> > write_page_md2802() commented out, but that function does not
> > exist. This is dead code with ugly #ifdefs.
>
> Absolutely NACK.

Please reconsider my patch.
> The reason the code is not compiled in per default is that the
> probing takes forever. Still, no reason to drop support.
>
> I don't think we should drop features because we don't like a few
> ifdefs.

That is not the most important reason. Please look at the code. Probe
may actually work, but read, erase and write simply can not:

int read_md2802(struct flashchip *flash, uint8_t *buf)
{
        return 0;
}

int erase_md2802(struct flashchip *flash)
{
        volatile uint8_t *bios = flash->virtual_memory;

return 1;

*(volatile uint8_t *)(bios + 0x5555) = 0xAA;
        *(volatile uint8_t *)(bios + 0x2AAA) = 0x55;  
        *(volatile uint8_t *)(bios + 0x5555) = 0x80;

*(volatile uint8_t *)(bios + 0x5555) = 0xAA;
        *(volatile uint8_t *)(bios + 0x2AAA) = 0x55;
        *(volatile uint8_t *)(bios + 0x5555) = 0x10;
}

int write_md2802(struct flashchip *flash, uint8_t *buf)
{
        int i;
        int total_size = flash->total_size * 1024;
        int page_size = flash->page_size;
        volatile uint8_t *bios = flash->virtual_memory;

return (1);

Immediate return. But even if it had not been there:

erase_md2802(flash);
        if (*bios != (uint8_t) 0xff) {
                printf("ERASE FAILED!\n");
                return -1;
        }
        printf("Programming page: ");
        for (i = 0; i < total_size / page_size; i++) {
                printf("%04d at address: 0x%08x", i, i * page_size);
                //write_page_md2802(bios, buf + i * page_size, bios + i * page_size, page_size);

This call is commented out because write_page_md2802() does not exist.

printf("\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b\b");
        }
        printf("\n");
        //protect_md2802(bios);

return 0;
}

The simple fact is that this flash driver is not complete. If a patch
had been sent to the list that added a driver like this today it
would have been NAKed by a screaming horde of people. :)

I don't know how to justify keeping this code around.

//Peter