On 16.03.2010 00:45, Sean Nelson wrote:
> Signed-off-by: Sean Nelson <[email protected]>
>
I have edited the patch below quite a bit to show where I see a problem.
> ---
> chipdrivers.h | 5 +--
> flashchips.c | 32 ++++++++++++-------
> sst_fwhub.c | 96
> ++-------------------------------------------------------
> 3 files changed, 24 insertions(+), 109 deletions(-)
>
> diff --git a/sst_fwhub.c b/sst_fwhub.c
> index a325278..721a808 100644
> --- a/sst_fwhub.c
> +++ b/sst_fwhub.c
> @@ -83,101 +71,23 @@
>
> int printlock_sst_fwhub(struct flashchip *flash)
> {
> int i;
>
> for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
> check_sst_fwhub_block_lock(flash, i);
>
> return 0;
> }
>
> -int erase_sst_fwhub_sector(struct flashchip *flash, unsigned int offset,
> unsigned int page_size)
> -{
> - uint8_t blockstatus = clear_sst_fwhub_block_lock(flash, offset);
> -
> - if (blockstatus) {
> - printf("Sector lock clearing failed, not erasing sector "
> - "at 0x%06x\n", offset);
> - return 1;
> - }
> -
> [...]
> - return 0;
> -}
> -
> +int unlock_sst_fwhub(struct flashchip *flash)
> {
> int i;
>
> + for (i = 0; i < flash->total_size * 1024; i += flash->page_size)
> + clear_sst_fwhub_block_lock(flash, i);
>
> return 0;
> }
>
Note that the old code checks the return code of
clear_sst_fwhub_block_lock(), but the new code ignores the return code
completely. At the very least, I'd expect the code to complain loudly.
Ideal would be to try to unlock all even if unlock fails for one or
more. Basically, complain for each failed unlock, but continue and
return 0 only if all unlocks were sucessful.
With that addressed and a successful compile test, the patch is
Acked-by: Carl-Daniel Hailfinger <[email protected]>
Regards,
Carl-Daniel
--
"I do consider assignment statements and pointer variables to be among
computer science's most valuable treasures."
-- Donald E. Knuth
_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom
