Re: [LinuxBIOS] flashrom support for AMD CS5536

[Nikolay Petukhov]

2007/11/22, "Alan Alexander" :
> Hi all,
>
> I've recently checked out and built revision 2977 of flashrom in order
> to apply a simple BIOS configuration change to approx 200 embedded Linux
> devices.
> Unfortunately although flashrom is able to read the contents of the BIOS
> flash, writes fail to take (verification fails).
>
> The system basics are as follows:
>         IEI NanoLX Board (AMD LX800 + CS5536 + Winbond W83627EHG)
> http://www.ieiworld.com/en/product_IPC.asp?model=NANO-LX
>         Linux Kernel Version 2.6.20
>         Flash part identified by flashrom as Pm49FL004 at physical
> address 0xfff80000 (512 KB)
>
> I see from the source code that the CS5536 chipset is directly supported
> and that the code attempts to remove the write protection via MSR
> 0x1808. This all looks good and yet the writes fail to take. I assume
> therefore that this board is using some GPIOs to disable writes to the
> BIOS flash?
>
> Does it sound likely that GPIOs are causing the write problem that I am
> seeing? Does anyone know how I might go about figuring out which GPIOs
> to look at?
>
> Any help would be very appreciated.
>
> Best regards,
>
> Alan Alexander          mailto:[e-mail]
> Icera Inc, 2520 The Quadrant, Aztec west, Bristol BS32 4AQ, UK
> Tel. [phone]

I have the same problems with flashrom on iei/pcisa-lx.
http://www.ieiworld.com/en/product_IPC.asp?model=PCISA-LX (Flash Pm49FL004)

Info from pm49fl002/4's manual: Pm49fl002/4 write protected at power up.
This patch fix it.

[LinuxBIOS] flashrom support for AMD CS5536

[Corey Osgood]

Nikolay Petukhov wrote:
> Info from pm49fl002/4's manual: Pm49fl002/4 write protected at power up.
> This patch fix it.

I'm a bit confused, because I use the Pm49fl002/4 regularly, mostly in
BIOS Savior RD-1's. I've never had any problems writing to those chips.
This patch also needs a "Signed-off-by:" line, and a couple other things
below.
> diff -Nru LinuxBIOSv2-2978/util/flashrom/flashchips.c LinuxBIOSv2-2978-pm49fl00x/util/flashrom/flashchips.c

'svn diff' does pretty much the same thing, but easier to apply.
> diff -Nru LinuxBIOSv2-2978/util/flashrom/pm49fl004.c LinuxBIOSv2-2978-pm49fl00x/util/flashrom/pm49fl004.c
> --- LinuxBIOSv2-2978/util/flashrom/pm49fl004.c	2007-10-18 05:55:15.000000000 +0600
> +++ LinuxBIOSv2-2978-pm49fl00x/util/flashrom/pm49fl004.c	1970-01-01 05:00:00.000000000 +0500
> @@ -1,51 +0,0 @@
> -/*
> This file is part of the flashrom project.
> Copyright (C) 2004 Tyan Corporation
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> -#include <stdio.h>
> -#include "flash.h"
>
> -extern int exclude_start_page, exclude_end_page;
>
> -int write_49fl004(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;
> printf("Programming page: ");
> for (i = 0; i < total_size / page_size; i++) {
> if ((i >= exclude_start_page) && (i < exclude_end_page))
> 	continue;
> /* erase the page before programming */
> erase_block_jedec(bios, i * page_size);
> /* write to the sector */
> printf("%04d at address: 0x%08x", i, i * page_size);
> write_sector_jedec(bios, buf + i * page_size,
> 		   bios + i * page_size, page_size);
> 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");
> fflush(stdout);
> }
> printf("\n");
> return 0;
>
> -}

We should probably do 'svn mv' to this file, instead of removing it,
then patch against it.
> diff -Nru LinuxBIOSv2-2978/util/flashrom/pm49fl00x.c LinuxBIOSv2-2978-pm49fl00x/util/flashrom/pm49fl00x.c
> --- LinuxBIOSv2-2978/util/flashrom/pm49fl00x.c	1970-01-01 05:00:00.000000000 +0500
> +++ LinuxBIOSv2-2978-pm49fl00x/util/flashrom/pm49fl00x.c	2007-11-23 11:43:47.000000000 +0500
> @@ -0,0 +1,95 @@
> +/*
> This file is part of the flashrom project.
> Copyright (C) 2004 Tyan Corporation

Please add yourself here. I can't tell who the original author was, so
just leave the Tyan email blank.
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#include <stdio.h>
> +#include "flash.h"
>
> +extern int exclude_start_page, exclude_end_page;
>
> +static __inline__ int write_lockbits_49fl00x(volatile uint8_t *bios, int size,
> 			      unsigned char bits, int block_size)

Why inlined?
> +{
> int i, left = size;
> //printf("bios=0x%08lx\n", (unsigned long)bios);

just drop this. we don't like committing commented code ;)
> for (i = 0; left >= block_size; i++, left -= block_size) {
> /* pm49fl002 */
> if (block_size == 16384 && i%2)
> 	continue;
> //printf("lockbits at address=0x%08lx is 0x%01x\n", (unsigned long)0xFFC00000 - size + (i * block_size) + 2, *(bios + (i * block_size) + 2) );

same here
> *(bios + (i * block_size) + 2) = bits;
> //printf("lockbits at address=0x%08lx is 0x%01x\n", (unsigned long)0xFFC00000 - size + (i * block_size) + 2, *(bios + (i * block_size) + 2) );

and here
> }
> return 0;

I don't see any other return paths, any reason not to make this function
void?
> +}
>
> +int probe_49fl00x(struct flashchip *flash)
> +{
> int ret = probe_jedec(flash);
> if (ret == 1)
> map_flash_registers(flash);
> return ret;
>
> +}
>
> +int erase_49fl00x(struct flashchip *flash)
> +{
> int total_size = flash->total_size * 1024;
> int page_size = flash->page_size;
> write_lockbits_49fl00x(flash->virtual_registers, total_size, 0, page_size);
> return erase_chip_jedec(flash);
>
> +}
>
> +int write_49fl00x(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;
> /* unprotected */
> write_lockbits_49fl00x(flash->virtual_registers, total_size, 0, page_size);

IIRC, flashrom erases before writing. If that is the case, wouldn't this
be unnecessary?
> printf("Programming page: ");
> for (i = 0; i < total_size / page_size; i++) {
> if ((i >= exclude_start_page) && (i < exclude_end_page))
> 	continue;
> /* erase the page before programming */
> erase_block_jedec(bios, i * page_size);
> /* write to the sector */
> printf("%04d at address: 0x%08x", i, i * page_size);
> write_sector_jedec(bios, buf + i * page_size,
> 		   bios + i * page_size, page_size);
> 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");
> fflush(stdout);
> }
> printf("\n");
> /* protected */
> write_lockbits_49fl00x(flash->virtual_registers, total_size, 1, page_size);
> return 0;
>
> +}