[PATCH] The of_register_spi_devices() was not called after registering the spi master.

[PATCH] The of_register_spi_devices() was not called after registering the spi master.

[stefano.babic-jEVQdWr+paajF4gvJNWmbkB+6BGkLq7r]

From: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>

Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
---
 drivers/spi/mpc52xx_psc_spi.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
index 25eda71..41f9921 100644
--- a/drivers/spi/mpc52xx_psc_spi.c
+++ b/drivers/spi/mpc52xx_psc_spi.c
@@ -18,6 +18,7 @@
 
 #if defined(CONFIG_PPC_MERGE)
 #include <linux/of_platform.h>
+#include <linux/of_spi.h>
 #else
 #include <linux/platform_device.h>
 #endif
@@ -439,8 +440,10 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	}
 
 	ret = spi_register_master(master);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(dev,"spi_register_master FAILED\n");
 		goto unreg_master;
+	}
 
 	return ret;
 
@@ -524,6 +527,8 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 	const u32 *regaddr_p;
 	u64 regaddr64, size64;
 	s16 id = -1;
+	struct spi_master *master;
+	int ret;
 
 	regaddr_p = of_get_address(op->node, 0, &size64, NULL);
 	if (!regaddr_p) {
@@ -545,8 +550,13 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 		id = *psc_nump + 1;
 	}
 
-	return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
+	ret=mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
 					irq_of_parse_and_map(op->node, 0), id);
+	if (ret==0)  {
+		master=spi_busnum_to_master(id);
+		of_register_spi_devices(master, op->node);
+	}
+	return(ret);
 }
 
 static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
-- 
1.5.4.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[David Brownell]

Could we get an ack on this from the PowerPC team?

This seems plausible, but OpenFirmware support isn't
widely understood outside PowerPC land.

- Dave

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Grant Likely]

On Wed, Sep 24, 2008 at 09:59:53PM -0700, David Brownell wrote:
> Could we get an ack on this from the PowerPC team?
> 
> This seems plausible, but OpenFirmware support isn't
> widely understood outside PowerPC land.
> 
> - Dave

Can you bounce this one to me?  I can't find an email with the same
subject in the spi-devel-general archives.

g.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[David Brownell]

On Wednesday 24 September 2008, Grant Likely wrote:
> On Wed, Sep 24, 2008 at 09:59:53PM -0700, David Brownell wrote:
> > Could we get an ack on this from the PowerPC team?
> > 
> > This seems plausible, but OpenFirmware support isn't
> > widely understood outside PowerPC land.
> > 
> > - Dave
> 
> Can you bounce this one to me?  I can't find an email with the same
> subject in the spi-devel-general archives.

I just sent it to you off-list.


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] The of_register_spi_devices() was not called after registering the spi master.

[sbabic-ynQEQJNshbs]

The of_register_spi_devices() was not called after registering the spi master.
It should be called for powerpc platforms which use the device tree to
populate the SPI bus.
    
Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] The of_register_spi_devices() was not called after registering the spi master.

[sbabic-ynQEQJNshbs]

From: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>

Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
---
 drivers/spi/mpc52xx_psc_spi.c |   16 +++++++++++++---
 1 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
index 25eda71..5d233e7 100644
--- a/drivers/spi/mpc52xx_psc_spi.c
+++ b/drivers/spi/mpc52xx_psc_spi.c
@@ -18,6 +18,7 @@
 
 #if defined(CONFIG_PPC_MERGE)
 #include <linux/of_platform.h>
+#include <linux/of_spi.h>
 #else
 #include <linux/platform_device.h>
 #endif
@@ -439,8 +440,10 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr,
 	}
 
 	ret = spi_register_master(master);
-	if (ret < 0)
+	if (ret < 0) {
+		dev_err(dev, "spi_register_master FAILED\n");
 		goto unreg_master;
+	}
 
 	return ret;
 
@@ -524,6 +527,8 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 	const u32 *regaddr_p;
 	u64 regaddr64, size64;
 	s16 id = -1;
+	struct spi_master *master;
+	int ret;
 
 	regaddr_p = of_get_address(op->node, 0, &size64, NULL);
 	if (!regaddr_p) {
@@ -545,8 +550,13 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
 		id = *psc_nump + 1;
 	}
 
-	return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
-					irq_of_parse_and_map(op->node, 0), id);
+	if ((ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
+					irq_of_parse_and_map(op->node, 0), id)))
+		return ret;
+	master = spi_busnum_to_master(id);
+	of_register_spi_devices(master, op->node);
+
+	return 0;
 }
 
 static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
-- 
1.5.4.3


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Grant Likely]

On Fri, Sep 26, 2008 at 10:24:40AM +0200, sbabic-ynQEQJNshbs@public.gmane.org wrote:
> From: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>
> 
> Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>

One comment below.

> ---
>  drivers/spi/mpc52xx_psc_spi.c |   16 +++++++++++++---
>  1 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
> index 25eda71..5d233e7 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -545,8 +550,13 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
>  		id = *psc_nump + 1;
>  	}
>  
> -	return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> -					irq_of_parse_and_map(op->node, 0), id);
> +	if ((ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> +					irq_of_parse_and_map(op->node, 0), id)))
> +		return ret;

Preferred form is:

	ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
					irq_of_parse_and_map(op->node, 0), id);
	if (ret)
		return ret;

Burying the assignment inside the if() makes the code harder to read and
understand.

Otherwise, this looks right to me.

Thanks,
g.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Stefano Babic]

Grant Likely wrote:
> Preferred form is:
> 
> 	ret = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
> 					irq_of_parse_and_map(op->node, 0), id);
> 	if (ret)
> 		return ret;
> 
> Burying the assignment inside the if() makes the code harder to read and
> understand.

Thanks, understood. I will avoid in future to use this encryptic form ;)

I have another issue about this driver that I have found recently.
According to the MPC5200 manual the "tdfOnExit" flag must be set on the
last byte we want to send and the PSC controller holds SS low until the
flag is set.
However, this flag is set always on the last byte written into the FIFO,
independently if it is the last byte of the whole transfer:

                for (; send_at_once; sb++, send_at_once--) {
                        /* set EOF flag before the last word is sent */
                        if (send_at_once == 1)
                                out_8(&psc->ircr2, 0x01);

This generates spurious toggling of the SS signal that breaks the
protocol of some peripherals when we have to send more than 512 bytes.
(I have a peripheral that requires a start byte as first byte after SS
goes low).
IMHO this is a bug, because changing of the SS signal must be ruled only
by the cs_change field in the spi_transfer structure. However, it seems
nobody complains about this and probably it is not seen as a bug...

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office-ynQEQJNshbs@public.gmane.org
=====================================================================


-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

Re: [PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Grant Likely]

On Mon, Sep 29, 2008 at 05:04:47PM +0200, Stefano Babic wrote:
> I have another issue about this driver that I have found recently.
> According to the MPC5200 manual the "tdfOnExit" flag must be set on the
> last byte we want to send and the PSC controller holds SS low until the
> flag is set.
> However, this flag is set always on the last byte written into the FIFO,
> independently if it is the last byte of the whole transfer:
> 
>                 for (; send_at_once; sb++, send_at_once--) {
>                         /* set EOF flag before the last word is sent */
>                         if (send_at_once == 1)
>                                 out_8(&psc->ircr2, 0x01);
> 
> This generates spurious toggling of the SS signal that breaks the
> protocol of some peripherals when we have to send more than 512 bytes.
> (I have a peripheral that requires a start byte as first byte after SS
> goes low).
> IMHO this is a bug, because changing of the SS signal must be ruled only
> by the cs_change field in the spi_transfer structure. However, it seems
> nobody complains about this and probably it is not seen as a bug...

Yes, this is a bug.  Some boards do the SS with a GPIO instead of using
the SS pin and so probably see this problem.  It should be fixed.

g.

-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/

[PATCH] The of_register_spi_devices() was not called after registering the spi master.

[Stefano Babic]

The of_register_spi_devices() was not called after registering the spi
master.
It should be called for powerpc platforms which use the device tree to
populate the SPI bus.

Signed-off-by: Stefano Babic <sbabic-ynQEQJNshbs@public.gmane.org>



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/