Re: [PATCH 6/7] AMD Geode GX/LX support

[Andi Kleen <ak@suse.de>]

"Jordan Crouse" <jordan.crouse@amd.com> writes:

> This patch adds support for the hardware RNG device on the Geode LX
> processor.  As a side note, the LX processor also includes a hardware

Interesting. Wish the mainstream AMD K8 CPUs had one too :)

> AES encryption engine, support for which is not included here because
> I'm not one to increase the kernel source size if it doesn't need to be.

If it's faster than the i386 assembly version I think you
should add it.

> +#ifdef CONFIG_MGEODE_LX
> +static int __init geode_init(struct pci_dev *dev);
> +static void geode_cleanup(void);
> +static unsigned int geode_data_present (void);
> +static u32 geode_data_read (void);
> +#endif

Declarations don't need ifdefs.

> +static u32 geode_data_read(void) {
> +	u32 val;
> +
> +	val = *((u32 *) (geode_rng_base + GEODE_RNG_DATA_REG));

This should use readl

> +	return val;
> +}
> +
> +static unsigned int geode_data_present(void) {

The bracket should be on an own line. Further occurrences.


> +}
> +
> +static int geode_init(struct pci_dev *dev) {
> +	u32 rng_base = pci_resource_start(dev, 0);

This should be unsigned long

> +	if (!rng_base) return 1;
> +
> +	geode_rng_base = ioremap(rng_base, 0x58);

This should be ioremap_nocache() 

> +
> +	if (geode_rng_base == NULL) {
> +		printk(KERN_ERR PFX "Cannot ioremap RNG memory\n");
> +		return -EBUSY;
> +	}
> +
> +	printk(KERN_INFO PFX "Geode RNG registers at %p\n", geode_rng_base);

I would advise to not print virtual addresses into the kernel log.
They are usually completely useless to the user. Either physical
or nothing.


-Andi