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

[Aravind Gopalakrishnan]

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/