Subject: Re: [PATCH 4/5] PowerPC 74xx: Katana Qp base support

From: Arnd Bergmann <[email-masked]>
Date: Sat, 24 Nov 2007 19:51:59 +0100

On Friday 16 November 2007, Andrei Dolnikov wrote:

> +static int __init katanaqp_is_monarch(void)
> +{
> +	return !(in_8((volatile char *)(cpld_base + KATANAQP_CPLD_PSR)) &
> +		 KATANAQP_CPLD_PSR_PMCM);
> +}

The pointer here needs to be __iomem, not volatile. Same in other places.
Please use 'sparse' to check your code for bugs like this.

> +
> +static void __init katanaqp_setup_arch(void)
> +{
> +	struct device_node *cpld;
> +	const unsigned int *reg;
> +
> +	/*
> +	 * ioremap cpld registers in case they are later
> +	 * needed by katanaqp_reset_board().
> +	 */
> +	cpld = of_find_node_by_path("/mv64x60@f8100000/cpld@f8200000");

It doesn't sounds good to hardcode the path for this device.
Instead, it would be much better to look for the 'compatible' property
here.

> +static int __init katanaqp_of_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "cfi-flash");
> +	if (np)
> +		of_platform_device_create(np, "of-flash", NULL);
> +
> +	return 0;
> +}
> +
> +device_initcall(katanaqp_of_init);

This should be done automatically using of_platform_bus_probe().

	Arnd <><