linux-kernel - Re: [patch] x86 CPU detection for RDC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090816175204.GA28553@elte.hu>
Date:	Sun, 16 Aug 2009 19:52:04 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Mark Kelly <mark@...feros.com>
Cc:	linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com>,
	Florian Fainelli <florian@...nwrt.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [patch] x86 CPU detection for RDC


* Mark Kelly <mark@...feros.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@...oo.co.uk\n", customer_id);

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