Am 20.08.2011 12:39 schrieb Stefan Tauner:
> Signed-off-by: Stefan Tauner <[email protected]>

Acked-by: Carl-Daniel Hailfinger <[email protected]>
with comments.

> diff --git a/ichspi.c b/ichspi.c
> index c38fbfc..a9327de 100644
> --- a/ichspi.c
> +++ b/ichspi.c
> @@ -1507,6 +1507,24 @@ static void prettyprint_ich9_reg_pr(int i)
>               msg_pdbg(", unused)\n");
>  }
>  
> +/* Set the read and write protection enable bits of PR register @i according 
> to

Change "Set" to "Set/clear".


> + * @read_prot and @write_prot. */
> +static void ich8_set_pr(int i, int read_prot, int write_prot)
> +{
> +     void *addr = ich_spibar + ICH9_REG_PR0 + (i * 4);
> +     uint32_t pr = mmio_readl(addr);
> +
> +     msg_gspew("PR%u is 0x%08x, ", i, pr);
> +     pr &= ~((1 << PR_RP_OFF) | (1 << PR_WP_OFF));
> +     if (read_prot)
> +             pr |= (1 << PR_RP_OFF);
> +     if (write_prot)
> +             pr |= (1 << PR_WP_OFF);

Having some logic which eliminated unnecessary changes of pr would be
nice. You could even print "unchanged\n" for that case.


> +     msg_gspew("trying to set it to 0x%08x ", pr);
> +     mmio_writel(pr, addr);

Really not rmmio_writel? We want to undo all non-permanent config
changes on shutdown.


> +     msg_gspew("resulted in 0x%08x.\n", mmio_readl(addr));
> +}
> +
>  static const struct spi_programmer spi_programmer_ich7 = {
>       .type = SPI_CONTROLLER_ICH7,
>       .max_data_read = 64,
> @@ -1648,6 +1666,10 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, 
> void *rcrb,
>               for(i = 0; i < 5; i++)
>                       do_ich9_spi_frap(tmp, i);
>  
> +             /* try to disable PR locks before printing them */
> +             if (!ichspi_lock)
> +                     for(i = 0; i < 5; i++)
> +                             ich8_set_pr(i, 0, 0);

ich8_ is an unusual prefix. Most other names use either ich7_ or ich9_
prefixes even if the functionality is present in ICH8.


>               for(i = 0; i < 5; i++)
>                       prettyprint_ich9_reg_pr(i);
>  


-- 
http://www.hailfinger.org/


_______________________________________________
flashrom mailing list
[email protected]
http://www.flashrom.org/mailman/listinfo/flashrom

Reply via email to