Subject: Re: [RFC][PATCH] fsl_soc: add support for fsl_spi

From: Anton Vorontsov <[email-masked]>
Date: Fri, 27 Jul 2007 15:12:42 +0400

On Thu, Jul 26, 2007 at 11:45:08AM -0500, Scott Wood wrote:
> On Thu, Jul 26, 2007 at 05:56:55PM +0400, Anton Vorontsov wrote:
> > +void (*fsl_spi_activate_cs)(u8 cs, u8 polarity) = NULL;
> > +EXPORT_SYMBOL(fsl_spi_activate_cs);
> > +void (*fsl_spi_deactivate_cs)(u8 cs, u8 polarity) = NULL;
> > +EXPORT_SYMBOL(fsl_spi_deactivate_cs);
> 
> What are these for?  It looks like you're using them to set pins,
> but that should be done in the firmware (and if the firmware sucks, then
> do it in the platform code at bootup).

Nope, this is not to set [initial state of] pins. SPI layer needs
this to trigger MMC's chipselect, it's wired as usual GPIO pin, which
should be asserted (and then deasserted) by software at proper time.

Every board may have different implementation of
activate_cs/deactivate_cs, in mpc8323_rdb case it's implemented using
par_io_data_set().

(1)
Kumar Gala suggested to not use arch_initcall, but call fsl_spi_init()
explicitly by board file. So these global *_cs functions will go away.

> > +static int __init fsl_spi_of_init(void)
> > +{
> > +	struct device_node *np;
> > +	unsigned int i;
> > +
> > +	for (np = NULL, i = 1;
> > +	     (np = of_find_compatible_node(np, "spi", "fsl_spi")) != NULL;
> > +	     i++) {
> 
> s/fsl_spi/fsl,mpc8323-spi/, please.

Will do for mpc832x_rdb. Should I also prepare patch to change all
instances of "fsl_spi" in the arch/powerpc/boot/dts/*?

> Why not make the fsl spi driver an of_platform device?

Well, I guess this is rhetoric question. Is fsl_soc.c's
OF<->platform_devices bindings officially deprecated?

If they're, I'll surely volunteer to convert it to of_platform.
But then (1) should be solved another way, yet don't know how.

> -Scott

Thanks!

-- 
Anton Vorontsov
email: [email-masked]
backup email: [email-masked]
irc://irc.freenode.net/bd2