Messages in this thread | | | From | Florian Fainelli <> | Subject | Re: [PATCH] x86: CPU detection for RDC System-on-Chip | Date | Mon, 17 May 2010 13:40:42 +0200 |
| |
On Monday 17 May 2010 00:07:48 H. Peter Anvin wrote: > On 05/16/2010 06:21 AM, Florian Fainelli wrote: > > diff --git a/arch/x86/kernel/cpu/rdc.c b/arch/x86/kernel/cpu/rdc.c > > new file mode 100644 > > index 0000000..909c2b5 > > --- /dev/null > > +++ b/arch/x86/kernel/cpu/rdc.c > > @@ -0,0 +1,71 @@ > > +/* > > + * See Documentation/x86/rdc.txt > > + * > > + * mark@bifferos.com > > + */ > > + > > +#include <linux/pci.h> > > +#include <asm/pci-direct.h> > > +#include "cpu.h" > > + > > + > > +static void __cpuinit rdc_identify(struct cpuinfo_x86 *c) > > +{ > > + u16 vendor, device; > > + u32 customer_id; > > + > > + if (!early_pci_allowed()) > > + return; > > + > > + /* RDC CPU is SoC (system-on-chip), Northbridge is always present */ > > + vendor = read_pci_config_16(0, 0, 0, PCI_VENDOR_ID); > > + device = read_pci_config_16(0, 0, 0, PCI_DEVICE_ID); > > + > > + if (vendor != PCI_VENDOR_ID_RDC || device != PCI_DEVICE_ID_RDC_R6020) > > + return; /* not RDC */ > > + /* > > + * NB: We could go on and check other devices, e.g. r6040 NIC, but > > + * that's probably overkill > > + */ > > + > > + customer_id = read_pci_config(0, 0, 0, 0x90); > > + > > + switch (customer_id) { > > + /* id names are from RDC */ > > + case 0x00321000: > > + strcpy(c->x86_model_id, "R3210/R3211"); > > + break; > > + case 0x00321001: > > + strcpy(c->x86_model_id, "AMITRISC20000/20010"); > > + break; > > + case 0x00321002: > > + strcpy(c->x86_model_id, "R3210X/Edimax"); > > + break; > > + case 0x00321003: > > + strcpy(c->x86_model_id, "R3210/Kcodes"); > > + break; > > + case 0x00321004: /* tested */ > > + strcpy(c->x86_model_id, "S3282/CodeTek"); > > + break; > > + case 0x00321007: > > + strcpy(c->x86_model_id, "R8610"); > > + break; > > + default: > > + pr_info("RDC CPU: Unrecognised Customer ID (0x%x) please " > > + "report to: linux-kernel@vger.kernel.org\n", > > + customer_id); > > + break; > > This pr_info() is completely useless... reporting things to LKML is very > likely to get lost in the din. If someone (like yourself) wants to be > the maintainer for it that's one thing, otherwise it's probably better > to tell them to file a bugzilla... or just report it as "unknown" like > we do for all other CPU vendors. > > > + } > > + > > + strcpy(c->x86_vendor_id, "RDC"); > > + c->x86_vendor = X86_VENDOR_RDC; > > +} > > + > > +static const struct cpu_dev __cpuinitconst rdc_cpu_dev = { > > + .c_vendor = "RDC", > > + .c_ident = { "RDC" }, > > + .c_identify = rdc_identify, > > + .c_x86_vendor = X86_VENDOR_RDC, > > +}; > > + > > +cpu_dev_register(rdc_cpu_dev); > > .c_ident here is bogus: c_ident is supposed to represent the CPUID > string, but this device apparently doesn't have CPUID.
What do you suggest to use instead? Shall I put the x86_model_id there?
> > This adds at least one PCI reference to every boot on every device. As > such, at least please read vendor and device as a single 32-bit > reference. Since this identification isn't actually used for any real > purpose (like workarounds) we could also set it up as a PCI quirk... but > it's probably better to just keep the same code flow. > > -hpa
| |