Re: [PATCH V4] edac, amd64_edac: Modify usage of amd64_read_dct_pci_cfg()

[Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>]

On 9/15/2014 10:08 AM, Borislav Petkov wrote:
>>>>   	amd64_read_pci_cfg(pvt->F3, F10_ONLINE_SPARE, &pvt->online_spare);
>>>> -	amd64_read_dct_pci_cfg(pvt, DCLR0, &pvt->dclr0);
>>>> -	amd64_read_dct_pci_cfg(pvt, DCHR0, &pvt->dchr0);
>>>> -
>>>> -	if (!dct_ganging_enabled(pvt)) {
>>>> -		amd64_read_dct_pci_cfg(pvt, DCLR1, &pvt->dclr1);
>>>> -		amd64_read_dct_pci_cfg(pvt, DCHR1, &pvt->dchr1);
>>>> -	}
>>>> +	amd64_read_dct_pci_cfg(pvt, 0, DCLR0, &pvt->dclr0);
>>>> +	amd64_read_dct_pci_cfg(pvt, 0, DCHR0, &pvt->dchr0);
>>>>   	pvt->ecc_sym_sz = 4;
>>>>   	if (pvt->fam >= 0x10) {
>>>> +		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
>>>> +		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
>>> This doesn't look equivalent - above we're checking whether we're ganged
>>> now you're doing it for >= F10h. Why?
>>>
>>> Because only F10h supports ganging?
>> That's right.
>> If ganging is enabled (which is a condition we check for in
>> amd64_read_dct_pci_cfg();
>> Then we return 0.
> Right, but you're reading them now even if you don't have to. So why are
> you even changing this? What's wrong with doing:
>
> 	if (!dct_ganging_enabled(pvt)) {
> 		amd64_read_dct_pci_cfg(pvt, 1, DCLR0, &pvt->dclr1);
> 		amd64_read_dct_pci_cfg(pvt, 1, DCHR0, &pvt->dchr1);
> 	}
>
> which is much clearer than doing the family check.
>

Just thought we could remove one more condition..
Allright, I'll change this.

-Aravind.