[coreboot] [PATCH] flashrom: Infrastructure for ICH9 merge

Stefan Reinauer stepan at coresystems.de
Wed May 14 16:35:07 CEST 2008


Carl-Daniel Hailfinger wrote:
> Add more infrastructure for flashrom ICH9 support.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Good direction, but I have to NACK for a few things:

> +	/* Calculate the Root Complex Register Block address */
>  	tmp &= 0xffffc000;
> -	printf_debug("Root Complex Base Address Register = 0x%x\n", tmp);
> -	rcba = mmap(0, 0x3510, PROT_READ, MAP_SHARED, fd_mem, (off_t)tmp);
> -	if (rcba == MAP_FAILED) {
> +	printf_debug("Root Complex Register Block address = 0x%x\n", tmp);
> +	rcrb = mmap(0, 0x3A00, PROT_READ, MAP_SHARED, fd_mem, (off_t)tmp);
>   
The rcrb has a size of 16K (0x4000). Please map the whole bar. Mapping 
14.5K of a 16K bar seems a bit artificial.

> +	ich_spibar = mmap(0, 0x200, PROT_READ, MAP_SHARED, fd_mem_uncached,
> +			 (off_t)tmp + (off_t)spibar);
>   
The SPI bar is not a bar itself, but it is part of the RCBA. Please just 
map this once and make ich_spibar point into that.


> Index: flashrom-ich9/flashrom.c
> ===================================================================
> --- flashrom-ich9/flashrom.c	(Revision 3311)
> +++ flashrom-ich9/flashrom.c	(Arbeitskopie)
> @@ -45,6 +45,7 @@
>  int exclude_start_page, exclude_end_page;
>  int force = 0, verbose = 0;
>  int fd_mem;
> +int fd_mem_uncached;
>  
>  struct pci_dev *pci_dev_find(uint16_t vendor, uint16_t device)
>  {
> @@ -387,6 +388,12 @@
>  		       ". You need to be root.");
>  		exit(1);
>  	}
> +	/* Open the memory device UNCACHED. That's important for MMIO. */
> +	if ((fd_mem_uncached = open(MEM_DEV, O_RDWR|O_SYNC)) < 0) {
> +		perror("Error: Can not access memory using " MEM_DEV
> +		       ". You need to be root.");
> +		exit(1);
> +	}
>   
Please don't open the memory device twice. Instead just change the 
already existing open on the memory device to be uncached.

With above changes:

Acked-by: Stefan Reinauer <stepan at coresystems.de>

Stefan



-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080514/b7222621/attachment.sig>


More information about the coreboot mailing list