| Message ID | fa686aa40910311803m43504167s1ad802aecd2b4344@mail.gmail.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Grant Likely |
| Headers | show |
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
On Mon, Nov 2, 2009 at 6:14 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: >> 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? It has been discussed on-and-off on the mailing list and on IRC. There are 2 major problems that need to be solved before it can be done: 1. Figure out how to do OF-style driver probing with the platform bus. I could use the same heuristic as used by i2c & spi, but that approach isn't very scalable, and doesn't handle backwards compatibility very well. 2. Figure out the best way to extract platform data from the device tree without having a huge impact on the device drivers. The adapter code still driver specific, so it needs to be part of the device driver itself, but I want to establish a pattern which does not require major surgery to platform drivers in order to add OF support. >> - 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. Okay, I'll change this. Thanks for the feedback. g.
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) + 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