Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices()

[Wolfram Sang <w.sang@pengutronix.de>]

[-- Attachment #1.1: Type: text/plain, Size: 3131 bytes --]

Hi Grant,

> the patch referenced above is a little ugly.  Adding the call should

Agreed. I just referenced it to show there are more people wanting this
feature.

> be really simple.  I've drafted a patch to do only that step and
> attached it to this mail.  If this one works for you, then I'll merge
> it immediately into -next.

One minor comment, but works in general:

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

> Also, I'm resistant to changing the probe layout on this driver at
> this time.  With the work being done to generalize the OF support
> code, there is a strong possibility that of_platform will be
> deprecated in favor of going back to using the platform bus directly
> (just like how OF support works for i2c, spi, etc).  I'd rather not
> refactor the driver until I'm certain of the direction that things are
> going to go.

And this was possibly the best answer I could get \o/ Sounds really promising,
is there somewhere a discussion about how OF-generalization could happen?

> From 7629d40dc343ff216b752d5c68654dc9d30f0c91 Mon Sep 17 00:00:00 2001
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Sat, 31 Oct 2009 17:49:38 -0600
> Subject: [PATCH] spi/mpc5200: Register SPI devices described in device tree
> 
> Add call to of_register_spi_devices() to register SPI devices described
> in the OF device tree.
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>  drivers/spi/mpc52xx_psc_spi.c |    8 +++++++-
>  1 files changed, 7 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c
> index 1b74d5c..b445464 100644
> --- a/drivers/spi/mpc52xx_psc_spi.c
> +++ b/drivers/spi/mpc52xx_psc_spi.c
> @@ -17,6 +17,7 @@
>  #include <linux/errno.h>
>  #include <linux/interrupt.h>
>  #include <linux/of_platform.h>
> +#include <linux/of_spi.h>
>  #include <linux/workqueue.h>
>  #include <linux/completion.h>
>  #include <linux/io.h>
> @@ -464,6 +465,7 @@ static int __init mpc52xx_psc_spi_of_probe(struct of_device *op,
>  	const u32 *regaddr_p;
>  	u64 regaddr64, size64;
>  	s16 id = -1;
> +	int rc;
>  
>  	regaddr_p = of_get_address(op->node, 0, &size64, NULL);
>  	if (!regaddr_p) {
> @@ -485,8 +487,12 @@ 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,
> +	rc = mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64,
>  					irq_of_parse_and_map(op->node, 0), id);
> +	if (!rc)

A matter of taste, maybe: I'd prefer

	if (rc == 0)

as (!ptr) is often used for catching errors with pointers, but here it is the
'all went OK'-path.

> +		of_register_spi_devices(dev_get_drvdata(&op->dev), op->node);
> +
> +	return rc;
>  }
>  
>  static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op)
> -- 
> 1.6.3.3
> 

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 150 bytes --]

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev