[flashrom] [PATCH 04/10] sbxxx: Handle active IMCs in AMD chipsets.

[Carl-Daniel Hailfinger]

Am 10.07.2013 21:17 schrieb Stefan Tauner:
> Detect and temporarily disable the IMC while accessing the flash.
> Disable writes on default, but allow the user to enforce it.
>
> Signed-off-by: Rudolf Marek <r.marek at assembler.cz>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> ---
>  imc.c            | 171 +++++++++++++++++++++++++++++++++++++++++++++++++++++++

I already mentioned the suboptimal file name and public function names
on IRC. You said:
"[...] best would be to name it amd_imc.c and name the one external
visible function amd_imc_shutdown [...]"
That change would be appreciated.


> diff --git a/chipset_enable.c b/chipset_enable.c
> index 3979347..c884c80 100644
> --- a/chipset_enable.c
> +++ b/chipset_enable.c
> @@ -922,6 +922,10 @@ static int enable_flash_sb600(struct pci_dev *dev, const char *name)
>  	uint8_t reg;
>  	int ret;
>  
> +	if (imc_shutdown(dev) != 0) {

Hm. Can we avoid calling this on SB600 (if SB600 doesn't support IMC at
all)?


> +		return ERROR_FATAL;
> +	}
> +
>  	/* Clear ROM protect 0-3. */
>  	for (reg = 0x50; reg < 0x60; reg += 4) {
>  		prot = pci_read_long(dev, reg);
> diff --git a/sb600spi.c b/sb600spi.c
> index a5c00d8..7043aec 100644
> --- a/sb600spi.c
> +++ b/sb600spi.c
> @@ -210,10 +211,26 @@ int sb600_probe_spi(struct pci_dev *dev)
>  	struct pci_dev *smbus_dev;
>  	uint32_t tmp;
>  	uint8_t reg;
> +	bool amd_imc_force = false;

Ah yes, the joy of having a bool type in C99. It makes sense here.

>From a quick glance the code looks sane otherwise. I'll try to evaluate
it in light of the recent discussions about AMD IMC and associated problems.

Regards,
Carl-Daniel

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