* [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() @ 2009-10-30 19:44 Wolfram Sang 2009-10-31 5:32 ` Stephen Rothwell 2009-11-01 1:03 ` Grant Likely 0 siblings, 2 replies; 9+ messages in thread From: Wolfram Sang @ 2009-10-30 19:44 UTC (permalink / raw) To: spi-devel-general; +Cc: Kári Davíðsson, linuxppc-dev This driver has a generic part for the probe/remove routines which probably used to be called from a platform driver and an of_platform driver. Meanwhile, the driver is of_platform only, so there is no need for the generic part anymore. Unifying probe/remove finally enables us to call of_register_spi_devices(), so spi-device nodes from the device tree will be parsed. This was also mentioned by: http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073273.html On the way, some patterns have been changed to current best practices (like using '!ptr' and 'struct resource'). Also, an older, yet uncommented, patch from me has been incorporated to improve the checks if the selected PSC is valid: http://patchwork.ozlabs.org/patch/36780/ Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> Cc: Kári Davíðsson <kari.davidsson@marel.com> Cc: Grant Likely <grant.likely@secretlab.ca> --- drivers/spi/mpc52xx_psc_spi.c | 110 +++++++++++++++++++--------------------- 1 files changed, 52 insertions(+), 58 deletions(-) diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c index 1b74d5c..3fa8c78 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> @@ -27,6 +28,7 @@ #include <asm/mpc52xx.h> #include <asm/mpc52xx_psc.h> +#define DRIVER_NAME "mpc52xx-psc-spi" #define MCLK 20000000 /* PSC port MClk in hz */ struct mpc52xx_psc_spi { @@ -358,32 +360,54 @@ static irqreturn_t mpc52xx_psc_spi_isr(int irq, void *dev_id) return IRQ_NONE; } -/* bus_num is used only for the case dev->platform_data == NULL */ -static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, - u32 size, unsigned int irq, s16 bus_num) +static int __init mpc52xx_psc_spi_of_probe(struct of_device *op, + const struct of_device_id *match) { - struct fsl_spi_platform_data *pdata = dev->platform_data; + struct fsl_spi_platform_data *pdata = op->dev.platform_data; struct mpc52xx_psc_spi *mps; struct spi_master *master; + struct resource mem; + s16 id = -1; int ret; - master = spi_alloc_master(dev, sizeof *mps); - if (master == NULL) + /* get PSC id (only 1,2,3,6 can do SPI) */ + if (!pdata) { + const u32 *psc_nump; + + psc_nump = of_get_property(op->node, "cell-index", NULL); + if (!psc_nump || (*psc_nump > 2 && *psc_nump != 5)) { + dev_err(&op->dev, "PSC can't do SPI\n"); + return -EINVAL; + } + id = *psc_nump + 1; + } + + ret = of_address_to_resource(op->node, 0, &mem); + if (ret) + return ret; + + master = spi_alloc_master(&op->dev, sizeof *mps); + if (!master) return -ENOMEM; - dev_set_drvdata(dev, master); + if (!request_mem_region(mem.start, resource_size(&mem), DRIVER_NAME)) { + ret = -ENOMEM; + goto free_master; + } + + dev_set_drvdata(&op->dev, master); mps = spi_master_get_devdata(master); /* the spi->mode bits understood by this driver: */ master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST; - mps->irq = irq; - if (pdata == NULL) { - dev_warn(dev, "probe called without platform data, no " + mps->irq = irq_of_parse_and_map(op->node, 0); + if (!pdata) { + dev_info(&op->dev, "probe called without platform data, no " "cs_control function will be called\n"); mps->cs_control = NULL; mps->sysclk = 0; - master->bus_num = bus_num; + master->bus_num = id; master->num_chipselect = 255; } else { mps->cs_control = pdata->cs_control; @@ -395,19 +419,18 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, master->transfer = mpc52xx_psc_spi_transfer; master->cleanup = mpc52xx_psc_spi_cleanup; - mps->psc = ioremap(regaddr, size); + mps->psc = ioremap(mem.start, resource_size(&mem)); if (!mps->psc) { - dev_err(dev, "could not ioremap I/O port range\n"); + dev_err(&op->dev, "could not ioremap I/O port range\n"); ret = -EFAULT; - goto free_master; + goto free_unmap; } /* On the 5200, fifo regs are immediately ajacent to the psc regs */ mps->fifo = ((void __iomem *)mps->psc) + sizeof(struct mpc52xx_psc); - ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, "mpc52xx-psc-spi", - mps); + ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, DRIVER_NAME, mps); if (ret) - goto free_master; + goto free_unmap; ret = mpc52xx_psc_spi_port_config(master->bus_num, mps); if (ret < 0) @@ -429,24 +452,29 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, if (ret < 0) goto unreg_master; + of_register_spi_devices(master, op->node); + return ret; unreg_master: destroy_workqueue(mps->workqueue); free_irq: free_irq(mps->irq, mps); -free_master: +free_unmap: if (mps->psc) iounmap(mps->psc); + release_mem_region(mem.start, resource_size(&mem)); +free_master: spi_master_put(master); return ret; } -static int __exit mpc52xx_psc_spi_do_remove(struct device *dev) +static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op) { - struct spi_master *master = dev_get_drvdata(dev); + struct spi_master *master = dev_get_drvdata(&op->dev); struct mpc52xx_psc_spi *mps = spi_master_get_devdata(master); + struct resource mem; flush_workqueue(mps->workqueue); destroy_workqueue(mps->workqueue); @@ -454,46 +482,12 @@ static int __exit mpc52xx_psc_spi_do_remove(struct device *dev) free_irq(mps->irq, mps); if (mps->psc) iounmap(mps->psc); + if (of_address_to_resource(op->node, 0, &mem) == 0) + release_mem_region(mem.start, resource_size(&mem)); return 0; } -static int __init mpc52xx_psc_spi_of_probe(struct of_device *op, - const struct of_device_id *match) -{ - const u32 *regaddr_p; - u64 regaddr64, size64; - s16 id = -1; - - regaddr_p = of_get_address(op->node, 0, &size64, NULL); - if (!regaddr_p) { - printk(KERN_ERR "Invalid PSC address\n"); - return -EINVAL; - } - regaddr64 = of_translate_address(op->node, regaddr_p); - - /* get PSC id (1..6, used by port_config) */ - if (op->dev.platform_data == NULL) { - const u32 *psc_nump; - - psc_nump = of_get_property(op->node, "cell-index", NULL); - if (!psc_nump || *psc_nump > 5) { - printk(KERN_ERR "mpc52xx_psc_spi: Device node %s has invalid " - "cell-index property\n", op->node->full_name); - return -EINVAL; - } - 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); -} - -static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op) -{ - return mpc52xx_psc_spi_do_remove(&op->dev); -} - static struct of_device_id mpc52xx_psc_spi_of_match[] = { { .compatible = "fsl,mpc5200-psc-spi", }, { .compatible = "mpc5200-psc-spi", }, /* old */ @@ -504,12 +498,12 @@ MODULE_DEVICE_TABLE(of, mpc52xx_psc_spi_of_match); static struct of_platform_driver mpc52xx_psc_spi_of_driver = { .owner = THIS_MODULE, - .name = "mpc52xx-psc-spi", + .name = DRIVER_NAME, .match_table = mpc52xx_psc_spi_of_match, .probe = mpc52xx_psc_spi_of_probe, .remove = __exit_p(mpc52xx_psc_spi_of_remove), .driver = { - .name = "mpc52xx-psc-spi", + .name = DRIVER_NAME, .owner = THIS_MODULE, }, }; -- 1.6.3.3 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() 2009-10-30 19:44 [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() Wolfram Sang @ 2009-10-31 5:32 ` Stephen Rothwell 2009-10-31 9:03 ` Wolfram Sang 2009-11-01 1:03 ` Grant Likely 1 sibling, 1 reply; 9+ messages in thread From: Stephen Rothwell @ 2009-10-31 5:32 UTC (permalink / raw) To: Wolfram Sang Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev [-- Attachment #1.1: Type: text/plain, Size: 901 bytes --] Hi Wolfram, On Fri, 30 Oct 2009 20:44:12 +0100 Wolfram Sang <w.sang@pengutronix.de> wrote: > > static struct of_platform_driver mpc52xx_psc_spi_of_driver = { > .owner = THIS_MODULE, > - .name = "mpc52xx-psc-spi", > + .name = DRIVER_NAME, You no longer need to set either owner or name in the of_platform driver, just in the included struct driver (as is done below), so you could just remove the above two lines. > .match_table = mpc52xx_psc_spi_of_match, > .probe = mpc52xx_psc_spi_of_probe, > .remove = __exit_p(mpc52xx_psc_spi_of_remove), > .driver = { > - .name = "mpc52xx-psc-spi", > + .name = DRIVER_NAME, > .owner = THIS_MODULE, > }, > }; I am hoping that we can remove the owner and name fields from struct of_platform_driver sometime. -- Cheers, Stephen Rothwell sfr@canb.auug.org.au http://www.canb.auug.org.au/~sfr/ [-- Attachment #1.2: Type: application/pgp-signature, Size: 198 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() 2009-10-31 5:32 ` Stephen Rothwell @ 2009-10-31 9:03 ` Wolfram Sang 0 siblings, 0 replies; 9+ messages in thread From: Wolfram Sang @ 2009-10-31 9:03 UTC (permalink / raw) To: Stephen Rothwell Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev [-- Attachment #1.1: Type: text/plain, Size: 988 bytes --] Hi Stephen, > > static struct of_platform_driver mpc52xx_psc_spi_of_driver = { > > .owner = THIS_MODULE, > > - .name = "mpc52xx-psc-spi", > > + .name = DRIVER_NAME, > > You no longer need to set either owner or name in the of_platform driver, > just in the included struct driver (as is done below), so you could just > remove the above two lines. Okay, thanks. Will drop it. > > > .match_table = mpc52xx_psc_spi_of_match, > > .probe = mpc52xx_psc_spi_of_probe, > > .remove = __exit_p(mpc52xx_psc_spi_of_remove), > > .driver = { > > - .name = "mpc52xx-psc-spi", > > + .name = DRIVER_NAME, > > .owner = THIS_MODULE, > > }, > > }; > > I am hoping that we can remove the owner and name fields from struct > of_platform_driver sometime. Sounds good! :) 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() 2009-10-30 19:44 [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() Wolfram Sang 2009-10-31 5:32 ` Stephen Rothwell @ 2009-11-01 1:03 ` Grant Likely 2009-11-02 13:14 ` Wolfram Sang 1 sibling, 1 reply; 9+ messages in thread From: Grant Likely @ 2009-11-01 1:03 UTC (permalink / raw) To: Wolfram Sang Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 10450 bytes --] Hi Wolfram, Thanks for this work. Comments below. On Fri, Oct 30, 2009 at 1:44 PM, Wolfram Sang <w.sang@pengutronix.de> wrote: > This driver has a generic part for the probe/remove routines which probably > used to be called from a platform driver and an of_platform driver. Meanwhile, > the driver is of_platform only, so there is no need for the generic part > anymore. Unifying probe/remove finally enables us to call > of_register_spi_devices(), so spi-device nodes from the device tree will be > parsed. This was also mentioned by: > http://lists.ozlabs.org/pipermail/linuxppc-dev/2009-June/073273.html I haven't really been happy with any of the patches that added the of_register_spi_devices() call, but I never got around to dealing with it properly, or even replying. I found Jon's patch too invasive, and the patch referenced above is a little ugly. Adding the call should 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. > On the way, some patterns have been changed to current best practices (like > using '!ptr' and 'struct resource'). Also, an older, yet uncommented, patch > from me has been incorporated to improve the checks if the selected PSC is > valid: > > http://patchwork.ozlabs.org/patch/36780/ There are at least 3 discrete changes here. I prefer for each to be provided as a separate patch so I can cherry pick the ones that are ready. I'll go back and comment on the patch you referenced above patch right now. 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. Cheers, g. > > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de> > Cc: Kári Davíðsson <kari.davidsson@marel.com> > Cc: Grant Likely <grant.likely@secretlab.ca> > --- > drivers/spi/mpc52xx_psc_spi.c | 110 +++++++++++++++++++--------------------- > 1 files changed, 52 insertions(+), 58 deletions(-) > > diff --git a/drivers/spi/mpc52xx_psc_spi.c b/drivers/spi/mpc52xx_psc_spi.c > index 1b74d5c..3fa8c78 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> > @@ -27,6 +28,7 @@ > #include <asm/mpc52xx.h> > #include <asm/mpc52xx_psc.h> > > +#define DRIVER_NAME "mpc52xx-psc-spi" > #define MCLK 20000000 /* PSC port MClk in hz */ > > struct mpc52xx_psc_spi { > @@ -358,32 +360,54 @@ static irqreturn_t mpc52xx_psc_spi_isr(int irq, void *dev_id) > return IRQ_NONE; > } > > -/* bus_num is used only for the case dev->platform_data == NULL */ > -static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, > - u32 size, unsigned int irq, s16 bus_num) > +static int __init mpc52xx_psc_spi_of_probe(struct of_device *op, > + const struct of_device_id *match) > { > - struct fsl_spi_platform_data *pdata = dev->platform_data; > + struct fsl_spi_platform_data *pdata = op->dev.platform_data; > struct mpc52xx_psc_spi *mps; > struct spi_master *master; > + struct resource mem; > + s16 id = -1; > int ret; > > - master = spi_alloc_master(dev, sizeof *mps); > - if (master == NULL) > + /* get PSC id (only 1,2,3,6 can do SPI) */ > + if (!pdata) { > + const u32 *psc_nump; > + > + psc_nump = of_get_property(op->node, "cell-index", NULL); > + if (!psc_nump || (*psc_nump > 2 && *psc_nump != 5)) { > + dev_err(&op->dev, "PSC can't do SPI\n"); > + return -EINVAL; > + } > + id = *psc_nump + 1; > + } > + > + ret = of_address_to_resource(op->node, 0, &mem); > + if (ret) > + return ret; > + > + master = spi_alloc_master(&op->dev, sizeof *mps); > + if (!master) > return -ENOMEM; > > - dev_set_drvdata(dev, master); > + if (!request_mem_region(mem.start, resource_size(&mem), DRIVER_NAME)) { > + ret = -ENOMEM; > + goto free_master; > + } > + > + dev_set_drvdata(&op->dev, master); > mps = spi_master_get_devdata(master); > > /* the spi->mode bits understood by this driver: */ > master->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_LSB_FIRST; > > - mps->irq = irq; > - if (pdata == NULL) { > - dev_warn(dev, "probe called without platform data, no " > + mps->irq = irq_of_parse_and_map(op->node, 0); > + if (!pdata) { > + dev_info(&op->dev, "probe called without platform data, no " > "cs_control function will be called\n"); > mps->cs_control = NULL; > mps->sysclk = 0; > - master->bus_num = bus_num; > + master->bus_num = id; > master->num_chipselect = 255; > } else { > mps->cs_control = pdata->cs_control; > @@ -395,19 +419,18 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, > master->transfer = mpc52xx_psc_spi_transfer; > master->cleanup = mpc52xx_psc_spi_cleanup; > > - mps->psc = ioremap(regaddr, size); > + mps->psc = ioremap(mem.start, resource_size(&mem)); > if (!mps->psc) { > - dev_err(dev, "could not ioremap I/O port range\n"); > + dev_err(&op->dev, "could not ioremap I/O port range\n"); > ret = -EFAULT; > - goto free_master; > + goto free_unmap; > } > /* On the 5200, fifo regs are immediately ajacent to the psc regs */ > mps->fifo = ((void __iomem *)mps->psc) + sizeof(struct mpc52xx_psc); > > - ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, "mpc52xx-psc-spi", > - mps); > + ret = request_irq(mps->irq, mpc52xx_psc_spi_isr, 0, DRIVER_NAME, mps); > if (ret) > - goto free_master; > + goto free_unmap; > > ret = mpc52xx_psc_spi_port_config(master->bus_num, mps); > if (ret < 0) > @@ -429,24 +452,29 @@ static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, > if (ret < 0) > goto unreg_master; > > + of_register_spi_devices(master, op->node); > + > return ret; > > unreg_master: > destroy_workqueue(mps->workqueue); > free_irq: > free_irq(mps->irq, mps); > -free_master: > +free_unmap: > if (mps->psc) > iounmap(mps->psc); > + release_mem_region(mem.start, resource_size(&mem)); > +free_master: > spi_master_put(master); > > return ret; > } > > -static int __exit mpc52xx_psc_spi_do_remove(struct device *dev) > +static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op) > { > - struct spi_master *master = dev_get_drvdata(dev); > + struct spi_master *master = dev_get_drvdata(&op->dev); > struct mpc52xx_psc_spi *mps = spi_master_get_devdata(master); > + struct resource mem; > > flush_workqueue(mps->workqueue); > destroy_workqueue(mps->workqueue); > @@ -454,46 +482,12 @@ static int __exit mpc52xx_psc_spi_do_remove(struct device *dev) > free_irq(mps->irq, mps); > if (mps->psc) > iounmap(mps->psc); > + if (of_address_to_resource(op->node, 0, &mem) == 0) > + release_mem_region(mem.start, resource_size(&mem)); > > return 0; > } > > -static int __init mpc52xx_psc_spi_of_probe(struct of_device *op, > - const struct of_device_id *match) > -{ > - const u32 *regaddr_p; > - u64 regaddr64, size64; > - s16 id = -1; > - > - regaddr_p = of_get_address(op->node, 0, &size64, NULL); > - if (!regaddr_p) { > - printk(KERN_ERR "Invalid PSC address\n"); > - return -EINVAL; > - } > - regaddr64 = of_translate_address(op->node, regaddr_p); > - > - /* get PSC id (1..6, used by port_config) */ > - if (op->dev.platform_data == NULL) { > - const u32 *psc_nump; > - > - psc_nump = of_get_property(op->node, "cell-index", NULL); > - if (!psc_nump || *psc_nump > 5) { > - printk(KERN_ERR "mpc52xx_psc_spi: Device node %s has invalid " > - "cell-index property\n", op->node->full_name); > - return -EINVAL; > - } > - 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); > -} > - > -static int __exit mpc52xx_psc_spi_of_remove(struct of_device *op) > -{ > - return mpc52xx_psc_spi_do_remove(&op->dev); > -} > - > static struct of_device_id mpc52xx_psc_spi_of_match[] = { > { .compatible = "fsl,mpc5200-psc-spi", }, > { .compatible = "mpc5200-psc-spi", }, /* old */ > @@ -504,12 +498,12 @@ MODULE_DEVICE_TABLE(of, mpc52xx_psc_spi_of_match); > > static struct of_platform_driver mpc52xx_psc_spi_of_driver = { > .owner = THIS_MODULE, > - .name = "mpc52xx-psc-spi", > + .name = DRIVER_NAME, > .match_table = mpc52xx_psc_spi_of_match, > .probe = mpc52xx_psc_spi_of_probe, > .remove = __exit_p(mpc52xx_psc_spi_of_remove), > .driver = { > - .name = "mpc52xx-psc-spi", > + .name = DRIVER_NAME, > .owner = THIS_MODULE, > }, > }; > -- > 1.6.3.3 > > -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. [-- Attachment #2: 0001-spi-mpc5200-Register-SPI-devices-described-in-device.patch --] [-- Type: text/x-patch, Size: 1601 bytes --] 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 [-- Attachment #3: Type: text/plain, Size: 150 bytes --] _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() 2009-11-01 1:03 ` Grant Likely @ 2009-11-02 13:14 ` Wolfram Sang 2009-11-02 16:08 ` Grant Likely 0 siblings, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2009-11-02 13:14 UTC (permalink / raw) To: Grant Likely Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev [-- 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() 2009-11-02 13:14 ` Wolfram Sang @ 2009-11-02 16:08 ` Grant Likely [not found] ` <fa686aa40911020808n73a898ack5fb66933188555da-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Grant Likely @ 2009-11-02 16:08 UTC (permalink / raw) To: Wolfram Sang Cc: Kári Davíðsson, spi-devel-general, linuxppc-dev 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. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <fa686aa40911020808n73a898ack5fb66933188555da-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* [PATCH] mpc52xx-psc-spi: Enumerate child nodes in the OF tree [not found] ` <fa686aa40911020808n73a898ack5fb66933188555da-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2009-11-03 11:43 ` Kári Davíðsson [not found] ` <4AF01751.1090806-zCUwNCi8n9MAvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Kári Davíðsson @ 2009-11-03 11:43 UTC (permalink / raw) Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f [-- Attachment #1: Type: text/plain, Size: 214 bytes --] Hello, I need to apply the following patch in order to get the mpc52xx-psc-spi driver to instantiate my spi device driver. This patch is based on 2.6.29 but should apply cleanly to the most recent kernel. rg kd [-- Attachment #2: spi.patch --] [-- Type: text/x-diff, Size: 1218 bytes --] Index: drivers/spi/mpc52xx_psc_spi.c =================================================================== --- drivers/spi/mpc52xx_psc_spi.c (revision 692) +++ drivers/spi/mpc52xx_psc_spi.c (working copy) @@ -21,6 +21,7 @@ #include <linux/io.h> #include <linux/delay.h> #include <linux/spi/spi.h> +#include <linux/of_spi.h> #include <linux/fsl_devices.h> #include <asm/mpc52xx.h> @@ -370,9 +371,10 @@ } /* bus_num is used only for the case dev->platform_data == NULL */ -static int __init mpc52xx_psc_spi_do_probe(struct device *dev, u32 regaddr, +static int __init mpc52xx_psc_spi_do_probe(struct of_device *op, u32 regaddr, u32 size, unsigned int irq, s16 bus_num) { + struct device * dev = &op->dev; struct fsl_spi_platform_data *pdata = dev->platform_data; struct mpc52xx_psc_spi *mps; struct spi_master *master; @@ -439,6 +441,8 @@ if (ret < 0) goto unreg_master; + of_register_spi_devices(master, op->node); + return ret; unreg_master: @@ -495,7 +499,7 @@ id = *psc_nump + 1; } - return mpc52xx_psc_spi_do_probe(&op->dev, (u32)regaddr64, (u32)size64, + return mpc52xx_psc_spi_do_probe(op, (u32)regaddr64, (u32)size64, irq_of_parse_and_map(op->node, 0), id); } [-- Attachment #3: Type: text/plain, Size: 399 bytes --] ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference [-- Attachment #4: Type: text/plain, Size: 210 bytes --] _______________________________________________ spi-devel-general mailing list spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/spi-devel-general ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <4AF01751.1090806-zCUwNCi8n9MAvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] mpc52xx-psc-spi: Enumerate child nodes in the OF tree [not found] ` <4AF01751.1090806-zCUwNCi8n9MAvxtiuMwx3w@public.gmane.org> @ 2009-11-03 13:05 ` Wolfram Sang [not found] ` <20091103130515.GS3571-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> 0 siblings, 1 reply; 9+ messages in thread From: Wolfram Sang @ 2009-11-03 13:05 UTC (permalink / raw) To: Kári Davíðsson Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f [-- Attachment #1.1: Type: text/plain, Size: 378 bytes --] Hi, > I need to apply the following patch in order to get the mpc52xx-psc-spi driver > to instantiate my spi device driver. Check the patch Grant sent recently. I think this will go mainline next merge-window. -- 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: 399 bytes --] ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference [-- Attachment #3: Type: text/plain, Size: 210 bytes --] _______________________________________________ spi-devel-general mailing list spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/spi-devel-general ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <20091103130515.GS3571-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>]
* Re: [PATCH] mpc52xx-psc-spi: Enumerate child nodes in the OF tree [not found] ` <20091103130515.GS3571-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> @ 2009-11-03 13:47 ` Grant Likely 0 siblings, 0 replies; 9+ messages in thread From: Grant Likely @ 2009-11-03 13:47 UTC (permalink / raw) To: Wolfram Sang; +Cc: spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Tue, Nov 3, 2009 at 6:05 AM, Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> wrote: > Hi, > >> I need to apply the following patch in order to get the mpc52xx-psc-spi driver >> to instantiate my spi device driver. > > Check the patch Grant sent recently. I think this will go mainline next > merge-window. Yes, I've got a patch that does the same thing in my -next queue. g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ------------------------------------------------------------------------------ Come build with us! The BlackBerry(R) Developer Conference in SF, CA is the only developer event you need to attend this year. Jumpstart your developing skills, take BlackBerry mobile applications to market and stay ahead of the curve. Join us from November 9 - 12, 2009. Register now! http://p.sf.net/sfu/devconference ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-11-03 13:47 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-30 19:44 [PATCH] mpc52xx-psc-spi: refactor probe and remove to make use of of_register_spi_devices() Wolfram Sang
2009-10-31 5:32 ` Stephen Rothwell
2009-10-31 9:03 ` Wolfram Sang
2009-11-01 1:03 ` Grant Likely
2009-11-02 13:14 ` Wolfram Sang
2009-11-02 16:08 ` Grant Likely
[not found] ` <fa686aa40911020808n73a898ack5fb66933188555da-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2009-11-03 11:43 ` [PATCH] mpc52xx-psc-spi: Enumerate child nodes in the OF tree Kári Davíðsson
[not found] ` <4AF01751.1090806-zCUwNCi8n9MAvxtiuMwx3w@public.gmane.org>
2009-11-03 13:05 ` Wolfram Sang
[not found] ` <20091103130515.GS3571-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2009-11-03 13:47 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).