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

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

[Anton Vorontsov]

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 arch/powerpc/sysdev/fsl_soc.c |   76 +++++++++++++++++++++++++++++++++++++++++
 arch/powerpc/sysdev/fsl_soc.h |    3 ++
 2 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 3289fab..0599851 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1125,3 +1125,79 @@ err:
 arch_initcall(cpm_smc_uart_of_init);
 
 #endif /* CONFIG_8xx */
+
+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);
+
+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++) {
+		int ret = 0;
+		const char *devid, *sysclk, *mode, *max_cs;
+		struct resource res[2];
+		struct platform_device *pdev = NULL;
+		struct fsl_spi_platform_data pdata = {
+			.activate_cs = fsl_spi_activate_cs,
+			.deactivate_cs = fsl_spi_deactivate_cs,
+		};
+
+		memset(res, 0, sizeof(res));
+
+		devid = of_get_property(np, "device-id", NULL);
+		sysclk = of_get_property(np, "sysclk", NULL);
+		mode = of_get_property(np, "mode", NULL);
+		max_cs = of_get_property(np, "max-chipselect", NULL);
+		if (!devid || !sysclk || !mode || !max_cs)
+			goto err;
+
+		pdata.bus_num = *(u32 *)devid;
+		pdata.sysclk = *(u32 *)sysclk;
+		pdata.max_chipselect = *(u32 *)max_cs;
+		if (!strcmp(mode, "qe"))
+			pdata.qe_mode = 1;
+
+		ret = of_address_to_resource(np, 0, &res[0]);
+		if (ret)
+			goto err;
+
+		res[1].start = res[2].end = irq_of_parse_and_map(np, 0);
+		if (res[1].start == NO_IRQ)
+			goto err;
+
+		res[1].name = "mpc83xx_spi";
+		res[1].flags = IORESOURCE_IRQ;;
+
+		pdev = platform_device_alloc("mpc83xx_spi", i);
+		if (!pdev)
+			goto err;
+
+		ret = platform_device_add_data(pdev, &pdata, sizeof(pdata));
+		if (ret)
+			goto unreg;
+
+		ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
+		if (ret)
+			goto unreg;
+
+		ret = platform_device_register(pdev);
+		if (ret)
+			goto unreg;
+
+		continue;
+unreg:
+		platform_device_del(pdev);
+err:
+		continue;
+	}
+
+	return 0;
+}
+
+arch_initcall(fsl_spi_of_init);
diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/fsl_soc.h
index 04e145b..c9de2a2 100644
--- a/arch/powerpc/sysdev/fsl_soc.h
+++ b/arch/powerpc/sysdev/fsl_soc.h
@@ -8,5 +8,8 @@ extern phys_addr_t get_immrbase(void);
 extern u32 get_brgfreq(void);
 extern u32 get_baudrate(void);
 
+extern void (*fsl_spi_activate_cs)(u8 cs, u8 polarity);
+extern void (*fsl_spi_deactivate_cs)(u8 cs, u8 polarity);
+
 #endif
 #endif
-- 
1.5.0.6

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

[Scott Wood]

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).

> +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.

Why not make the fsl spi driver an of_platform device?

-Scott

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

[Vitaly Bordug]

On Thu, 26 Jul 2007 11:45:08 -0500
Scott Wood <scottwood@freescale.com> 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).
> 
That is not exactly the point. 
I won't respond for Anton, but we already had such hooks justified in SPI case iirc.

> > +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.
> 
> Why not make the fsl spi driver an of_platform device?
> 
Because this particular thing is not ppc-only from what I recall.

> -Scott
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev


-- 
Sincerely, 
Vitaly

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

[Scott Wood]

Vitaly Bordug wrote:
> On Thu, 26 Jul 2007 11:45:08 -0500
> Scott Wood <scottwood@freescale.com> wrote:
>>Why not make the fsl spi driver an of_platform device?
> 
> Because this particular thing is not ppc-only from what I recall.

spi_mpc83xx.c isn't ppc-only?

Even if it weren't, the OF stuff has been moved out of arch, so a 
non-arch-specific driver can support both platform and of_platform 
devices directly.

-Scott

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

[Kumar Gala]

On Jul 26, 2007, at 12:05 PM, Vitaly Bordug wrote:

> On Thu, 26 Jul 2007 11:45:08 -0500
> Scott Wood <scottwood@freescale.com> 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).
>>
> That is not exactly the point.
> I won't respond for Anton, but we already had such hooks justified  
> in SPI case iirc.

No they are used for runtime chip select handling.  The SPI  
controller is able to handle multiple devices, however the chipselect  
is not specified by SPI and thus it left to board code.

>>> +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.
>>
>> Why not make the fsl spi driver an of_platform device?
>>
> Because this particular thing is not ppc-only from what I recall.

The device node name already existed before this patch.

- k

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

[Kumar Gala]

On Jul 26, 2007, at 8:56 AM, Anton Vorontsov wrote:

> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  arch/powerpc/sysdev/fsl_soc.c |   76 ++++++++++++++++++++++++++++++ 
> +++++++++++
>  arch/powerpc/sysdev/fsl_soc.h |    3 ++
>  2 files changed, 79 insertions(+), 0 deletions(-)

There should be 2nd patch with updates to documentation for the SPI  
device node.

>
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> fsl_soc.c
> index 3289fab..0599851 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -1125,3 +1125,79 @@ err:
>  arch_initcall(cpm_smc_uart_of_init);
>
>  #endif /* CONFIG_8xx */
> +
> +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);
> +

I don't think we should do it this way.  We shouldn't arch_initcall()  
scan for fsl_spi, but have the board code call a function and pass  
some things into it.

> +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++) {
> +		int ret = 0;
> +		const char *devid, *sysclk, *mode, *max_cs;
> +		struct resource res[2];
> +		struct platform_device *pdev = NULL;
> +		struct fsl_spi_platform_data pdata = {
> +			.activate_cs = fsl_spi_activate_cs,
> +			.deactivate_cs = fsl_spi_deactivate_cs,
> +		};
> +
> +		memset(res, 0, sizeof(res));
> +
> +		devid = of_get_property(np, "device-id", NULL);
> +		sysclk = of_get_property(np, "sysclk", NULL);
> +		mode = of_get_property(np, "mode", NULL);
> +		max_cs = of_get_property(np, "max-chipselect", NULL);
> +		if (!devid || !sysclk || !mode || !max_cs)
> +			goto err;
> +
> +		pdata.bus_num = *(u32 *)devid;
> +		pdata.sysclk = *(u32 *)sysclk;
> +		pdata.max_chipselect = *(u32 *)max_cs;
> +		if (!strcmp(mode, "qe"))
> +			pdata.qe_mode = 1;
> +
> +		ret = of_address_to_resource(np, 0, &res[0]);
> +		if (ret)
> +			goto err;
> +
> +		res[1].start = res[2].end = irq_of_parse_and_map(np, 0);
> +		if (res[1].start == NO_IRQ)
> +			goto err;
> +
> +		res[1].name = "mpc83xx_spi";
> +		res[1].flags = IORESOURCE_IRQ;;
> +
> +		pdev = platform_device_alloc("mpc83xx_spi", i);
> +		if (!pdev)
> +			goto err;
> +
> +		ret = platform_device_add_data(pdev, &pdata, sizeof(pdata));
> +		if (ret)
> +			goto unreg;
> +
> +		ret = platform_device_add_resources(pdev, res, ARRAY_SIZE(res));
> +		if (ret)
> +			goto unreg;
> +
> +		ret = platform_device_register(pdev);
> +		if (ret)
> +			goto unreg;
> +
> +		continue;
> +unreg:
> +		platform_device_del(pdev);
> +err:
> +		continue;
> +	}
> +
> +	return 0;
> +}
> +
> +arch_initcall(fsl_spi_of_init);
> diff --git a/arch/powerpc/sysdev/fsl_soc.h b/arch/powerpc/sysdev/ 
> fsl_soc.h
> index 04e145b..c9de2a2 100644
> --- a/arch/powerpc/sysdev/fsl_soc.h
> +++ b/arch/powerpc/sysdev/fsl_soc.h
> @@ -8,5 +8,8 @@ extern phys_addr_t get_immrbase(void);
>  extern u32 get_brgfreq(void);
>  extern u32 get_baudrate(void);
>
> +extern void (*fsl_spi_activate_cs)(u8 cs, u8 polarity);
> +extern void (*fsl_spi_deactivate_cs)(u8 cs, u8 polarity);
> +
>  #endif
>  #endif
> -- 
> 1.5.0.6
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

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

[Anton Vorontsov]

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: cbou@mail.ru
backup email: ya-cbou@yandex.ru
irc://irc.freenode.net/bd2