On Tue, Aug 17, 2010 at 06:21:38PM +0200, Jean Delvare wrote:
[...]
> > +static int is_rev_g_desktop(u8 model)
> > +{
> > + u32 brandidx;
> > +
> > + if (model < 0x69)
> > + return 0;
> > +
> > + if (model == 0xc1 || model == 0x6c || model == 0x7c ||
> > + model == 0x6f || model == 0x7f)
> > + return 0;
> > +
> > + if (model == 0x6b) {
> > + /* differentiate between AM2 and ASB1 */
> > + brandidx = cpuid_ebx(0x80000001);
> > + brandidx = (brandidx >> 9) & 0x1f;
> > + if (brandidx > 0xa)
>
> This will only catch the "AMD Sempron(tm) Processor 2RRU" entry, if I
> read both the code and the datasheet right?
No, it will catch only the dual-core CPUs
0Bh 0h AMD Turion(tm) Neo X2 Dual Core Processor L6RR (ASB1)
0Ch 0h AMD Athlon(tm) Neo X2 Dual Core Processor L3RR (ASB1)
See revision 3.46 of the RG:
http://support.amd.com/us/Processor_TechDocs/33610.pdf
> Is this correct? What about the 3 other ASB1 entries in table 8? And
> why are you comparing with 0xa while this specific value doesn't
> match any CPU model?
Hmm, wait let me double check this. (Thought the single core variants were
not affected by this problem)
Damn!
Need to update this patch. I did not check table 2 which lists
single-core AM2 CPUs.
model 0x6f and 0x7f (both single core) are also provided as ASB1 and
AM2.
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
> > +
> > static int __devinit k8temp_probe(struct pci_dev *pdev,
> > const struct pci_device_id *id)
> > {
> > @@ -179,9 +201,7 @@ static int __devinit k8temp_probe(struct pci_dev *pdev,
> > "wrong - check erratum #141\n");
> > }
> >
> > - if ((model >= 0x69) &&
> > - !(model == 0xc1 || model == 0x6c || model == 0x7c ||
> > - model == 0x6b || model == 0x6f || model == 0x7f)) {
> > + if (is_rev_g_desktop(model))
> > /*
> > * RevG desktop CPUs (i.e. no socket S1G1 or
> > * ASB1 parts) need additional offset,
> > @@ -189,7 +209,6 @@ static int __devinit k8temp_probe(struct pci_dev *pdev,
> > * ambient temperature
> > */
> > data->temp_offset = 21000;
> > - }
> >
> > break;
> > }
>
> I also do not like the brace change. While technically correct, it
> could lead to mistakes on future changes, because the large comment
> doesn't make it immediately obvious that this is currently a
> single-statement branch.
Ok.
Thanks a lot for reviewing.
I'll come up with an updated patch.
Andreas
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
