lkml.org 
[lkml]   [2009]   [Aug]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch] x86 CPU detection for RDC

* Mark Kelly <mark@bifferos.com> wrote:

> +static void __cpuinit rdc_identify(struct cpuinfo_x86 *c)
> +{
> + u16 vendor, device;
> + u32 customer_id;
> +
> + /* 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 */

please use the customary (multi-line) comment style:

/*
* Comment .....
* ...... goes here.
*/

specified in Documentation/CodingStyle.

> + strcpy(c->x86_vendor_id, "RDC");
> + c->x86_vendor = X86_VENDOR_RDC;
> +
> + 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:
> + printk(KERN_INFO "Unrecognised Customer ID (0x%x) please "
> + "report to bifferos@yahoo.co.uk\n", customer_id);

I'd rather see them reported to linux-kernel@vger.kernel.org. Also,
please concatenate the string on a single line (and ignore the
checkpatch warning in this case, even if the line is over-long).

Also, please use pr_info() here. Plus, put 'RDC' into the printout
so that people know that the RDC cpu is unknown. (it's not clear
from this printk)

> + /* We'll default to the R321x since that's mentioned
> + elsewhere in the kernel sources */
> + strcpy(c->x86_model_id, "R321x");
> +
> + /* blank the vendor_id, so we get a warning that this
> + is unsupported, your system may be unstable etc...
> + Is there a better way? */

(these multi-line comments too need to be fixed.)

> + strcpy(c->x86_vendor_id, "");

hm, the default should already be an empty string (i.e. \0) - why
does this need to be cleared again?

Ingo


\
 
 \ /
  Last update: 2009-08-16 19:55    [W:0.053 / U:0.516 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site