[flashrom] [PATCH] quick fix for chip locks

[Carl-Daniel Hailfinger]

On 19.02.2010 00:46, Sean Nelson wrote:
> Here's a very quick patch to fix the missing unlock code.
> Fixes missing unlock for certain chips:
>  * unlock_49lf00x
>     * Pm49fl002
>     * Pm49fl004
>
>  * unlock_49flxxxc
>     * SST49LF160C
>
>  * unlock_winbond_fwhub
>     * W39V080FA
>     * W39V080FA (dual mode)
>
> Fixes missing printlock for certain chip:
>  * printlock_w39v040c
>     * W39V040C
>
>
> Signed-off-by: Sean Nelson <audiohacked at gmail.com>

Thanks a lot. We desperately need this regression fix.
A few minor comments. Once you address them, this is

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

W39V040C seems to only have a printlock and not any unlock. Not
something you need to fix (no regression), so feel free to ignore this
comment.


> Index: flashrom.c
> ===================================================================
> --- flashrom.c	(revision 906)
> +++ flashrom.c	(working copy)
> @@ -832,18 +832,19 @@
>  	}
>  
>  	if (!flash || !flash->name)
>  		return NULL;
>  
>  	printf("Found chip \"%s %s\" (%d KB, %s) at physical address 0x%lx.\n",
>  	       flash->vendor, flash->name, flash->total_size,
>  	       flashbuses_to_text(flash->bustype), base);
>  
> +	flash->printlock(flash);
>   

This will crash and burn (NULL pointer dereference). Please check
if (flash->printlock)
before executing that pointer.


> @@ -1130,18 +1131,20 @@
>  	uint8_t *buf;
>  	unsigned long numbytes;
>  	FILE *image;
>  	int ret = 0;
>  	unsigned long size;
>  
>  	size = flash->total_size * 1024;
>  	buf = (uint8_t *) calloc(size, sizeof(char));
>  
> +	flash->unlock(flash);
>   

Same problem here.

Did you make sure that a simple probe-only execution doesn't unlock the
chip? Didn't have time to check (need to get back to my books).
Please also check that the codebase still compiles after "make distclean".

By the way, if you have the time to check all currently-unused files for
discrepancies between old functionality (that is, lock stuff) and new
functionality and you're reasonably sure that they are not needed
anymore, please send a separate patch (or rather, a list of files to
execute "svn rm" on) to kill them. I'll be happy to get rid of any dead
code.

Regards,
Carl-Daniel

-- 
"I do consider assignment statements and pointer variables to be among
computer science's most valuable treasures."
-- Donald E. Knuth