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

[Dale Farnsworth]

On Thu, Apr 26, 2007 at 01:24:19PM +0200, Arnd Bergmann wrote:
> On Thursday 26 April 2007, Dale Farnsworth wrote:
> > > 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.
> > 
> > Unfortunately, this hardware is very much non-8250 compatible.
> 
> That shouldn't matter much. The driver is not 8250 specific by itself,
> it's just that right now it doesn't know about any other chips.

Hmm, I wouldn't call that a driver, I'd call it OF interface
glue used to register the driver.  I guess I could put the
platform_device_register call for the mpsc driver in that file.
But I think that will increase, rather than reduce complexity.

> 
> If you find that there is more code that needs to be added than what is already
> in there, you could also create a new one based on of_serial for your port.
> 
> I can understand why you want to keep using the platform_device here, and in the
> other places, but if you do then please use platform_device_register from an
> of_platform_driver probe function instead of platform_device_register_simple
> so you can set the parent to the of_device.
> 
> > > > +???????????pdev = platform_device_register_simple(MPSC_CTLR_NAME, i, r, 5);
> > > > +???????????if (IS_ERR(pdev)) {
> > > > +???????????????????err = PTR_ERR(pdev);
> > > > +???????????????????goto ret_node_put;
> > > > +???????????}

Can you coerce your mailer to stop munging tabs?

> > > 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?
> > 
> > There is an existing mpsc driver usable on both MIPS and powerpc platforms
> > that requires these non-standard properties.
> 
> Ok, I see where some of the limitations come from. However, instead of
> introducing the new "sdma" and "brg" properties, why not just add the
> register ranges to the "reg" property, like other drivers do?

The sdma and brg are not simply properties of the serial device, these
nodes represent hardware modules that may be shared by multiple drivers.

-Dale