Subject: Re: [PATCH 7/13] powerpc: Add arch/powerpc mv64x60 MPSC platform data setup

From: Arnd Bergmann <[email-masked]>
Date: Thu, 26 Apr 2007 02:14:07 +0200

On Thursday 26 April 2007, Mark A. Greer wrote:
> +static int __init mv64x60_mpsc_platform_device_init(void)
> +{
> +       struct device_node *np = NULL;
> +       unsigned int i;
> +       struct platform_device *pdev;
> +       int err;
> +
> +       for (i = 0; (np = of_find_compatible_node(np, "serial", "mpsc")); i++) {
> +               struct resource r[5];
> +               struct mpsc_pdata pdata;
> +               const unsigned int *prop;
> +               const phandle *ph;
> +               struct device_node *sdma, *brg;

This looks wrong to me. See drivers/serial/of_serial.c to find how we do it for
8250 compatible serial ports. You should probably just add your serial port
stuff in there as well, instead of doing your own scanning of the device tree.

> +               pdev = platform_device_register_simple(MPSC_CTLR_NAME, i, r, 5);
> +               if (IS_ERR(pdev)) {
> +                       err = PTR_ERR(pdev);
> +                       goto ret_node_put;
> +               }

Now this really needs some explanation.

Why the heck do you have a platform device that gets its resources from
nonstandard properties of a serial port?

	Arnd <><