| Message ID | 1353576375-8560-1-git-send-email-andreas@gaisler.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On 2012-11-22 10:26, Andreas Larsson wrote: > I am looking into writing a driver for a core running on sparc that is mostly > but not entirely compatible with the cpu mode of spi-fsl-spi. I am thinking of > what could be the best approach for realizing this. Any comments on a preferred > approach in this situation? Any comments on this? A fair amount of rearranging and ifdefing would be needed to make the spi-fsl-spi driver workable outside an fsl and powerpc environment so I rather get some input before before trying to perfect a solution that could not be acceptable into mainline. Cheers, Andreas > These are two different approaches I see to solve the situation without too much > code duplication: > > Appproach A: Extend spi-fsl-spi and spi-fsl-lib to work outside of a FSL SOC > environtment and outside powerpc. This would require ifdefs for the driver to be > able to compile and work on sparc (or other platforms) - see patch draft at the > end. Everything that has to do with cpm and sysdev/fsl_soc.h needs to be within > ifdefs. Then the core in question could be added as another "mode" in the > spi-fsl-spi driver with core specific code embedded inside spi-fsl-spi.c just as > for the other existing modes. > > Approach B: Put the general and cpu mode specific functions from spi-fsl-spi in > spi-fsl-lib or in a new separate c file and use these from both spi-fsl-spi and > a new driver for the core in question. Some ifdefs would still be needed, but > most things should be able to be handled with function pointers in such a > solution. The question is where to move the general and cpu mode functions (but > not the cpm related ones that could not compile) > > Of course something in between A and B could be done as well, where all > functions stay in spi-fsl-spi.c but is exported so that they can be used with a > new driver. Then most ifdefs in fsl-spi-fsl would need to be in place for > compileability though. > > Of course a third option that leaves spi-fsl-* alone but results in a lot of > code duplication is: > > Approach C: Submit a totally separate driver for this core (with heavy copying > and pasting from spi-fsl-* as most of the functionality is the same as the cpu > mode parts of spi-fsl-spi). A lot of ugly code duplication. The only upside > compared to approach B is that there are mode register bits for this core that > conflicts with mode register bits in other mpc8xxx cores that are not currently > in use by the driver. If those would be used in the future in spi-fsl-spi, a > separate driver would not break for this core. > > Core specific things that is needed in any approach (but can be mostly isolated > from spi-fsl-* in approach B): > - Add entries to the register struct for additional registers > - Adding core-specific chipselect discovery/handling code as there might be > built in chip select capabilities in the core. > - Add core-specific code to handle driver matching, bus numbering, getting clock > frequency, tx/rx_shifting, and dealing with possible word-length limitations. > > > Below follows a draft patch on how spi-fsl-spi with friends could be made > functioning in cpu mode outside of a FSL SOC environment. Approach A pointed out > above would add the new mode on top of this for the needed core specific > things. Note the ugly define on in/out_be16/32 that would need to be done in a > proper way in a real patch. I didn't want to clutter the patch below with that > now instead showing the ifdefs clearly. > > Cheers, > Andreas Larsson > > --- > drivers/spi/Kconfig | 4 ++-- > drivers/spi/spi-fsl-lib.c | 10 ++++++++++ > drivers/spi/spi-fsl-lib.h | 8 ++++++++ > drivers/spi/spi-fsl-spi.c | 32 ++++++++++++++++++++++++++++---- > 4 files changed, 48 insertions(+), 6 deletions(-) > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > index 5b017af..8fbd698 100644 > --- a/drivers/spi/Kconfig > +++ b/drivers/spi/Kconfig > @@ -218,11 +218,11 @@ config SPI_MPC512x_PSC > > config SPI_FSL_LIB > tristate > - depends on FSL_SOC > + depends on OF > > config SPI_FSL_SPI > bool "Freescale SPI controller" > - depends on FSL_SOC > + depends on OF > select SPI_FSL_LIB > help > This enables using the Freescale SPI controllers in master mode. > diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c > index 1503574..0c9021d 100644 > --- a/drivers/spi/spi-fsl-lib.c > +++ b/drivers/spi/spi-fsl-lib.c > @@ -23,7 +23,9 @@ > #include <linux/mm.h> > #include <linux/of_platform.h> > #include <linux/spi/spi.h> > +#ifdef CONFIG_FSL_SOC > #include <sysdev/fsl_soc.h> > +#endif > > #include "spi-fsl-lib.h" > > @@ -208,6 +210,7 @@ int __devinit of_mpc8xxx_spi_probe(struct platform_device *ofdev) > /* Allocate bus num dynamically. */ > pdata->bus_num = -1; > > +#ifdef CONFIG_FSL_SOC > /* SPI controller is either clocked from QE or SoC clock. */ > pdata->sysclk = get_brgfreq(); > if (pdata->sysclk == -1) { > @@ -217,16 +220,23 @@ int __devinit of_mpc8xxx_spi_probe(struct platform_device *ofdev) > goto err; > } > } > +#else > + ret = of_property_read_u32(np, "clock-frequency", &pdata->sysclk); > + if (ret) > + goto err; > +#endif > > prop = of_get_property(np, "mode", NULL); > if (prop && !strcmp(prop, "cpu-qe")) > pdata->flags = SPI_QE_CPU_MODE; > +#ifdef CONFIG_FSL_SOC > else if (prop && !strcmp(prop, "qe")) > pdata->flags = SPI_CPM_MODE | SPI_QE; > else if (of_device_is_compatible(np, "fsl,cpm2-spi")) > pdata->flags = SPI_CPM_MODE | SPI_CPM2; > else if (of_device_is_compatible(np, "fsl,cpm1-spi")) > pdata->flags = SPI_CPM_MODE | SPI_CPM1; > +#endif > > return 0; > > diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h > index cbe881b..cd85170 100644 > --- a/drivers/spi/spi-fsl-lib.h > +++ b/drivers/spi/spi-fsl-lib.h > @@ -20,6 +20,12 @@ > > #include <asm/io.h> > > +/* Inline replacement or something like that in a real patch */ > +#define out_be32(reg, val) iowrite32be((val), (reg)) > +#define out_be16(reg, val) iowrite16be((val), (reg)) > +#define in_be32(reg) ioread32be((reg)) > +#define in_be16(reg) ioread16be((reg)) > + > /* SPI/eSPI Controller driver's private data. */ > struct mpc8xxx_spi { > struct device *dev; > @@ -34,8 +40,10 @@ struct mpc8xxx_spi { > > int subblock; > struct spi_pram __iomem *pram; > +#ifdef CONFIG_FSL_SOC > struct cpm_buf_desc __iomem *tx_bd; > struct cpm_buf_desc __iomem *rx_bd; > +#endif > > struct spi_transfer *xfer_in_progress; > > diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c > index 6a62934..f459416 100644 > --- a/drivers/spi/spi-fsl-spi.c > +++ b/drivers/spi/spi-fsl-spi.c > @@ -30,15 +30,20 @@ > #include <linux/mutex.h> > #include <linux/of.h> > #include <linux/of_platform.h> > +#include <linux/of_address.h> > +#include <linux/of_irq.h> > #include <linux/gpio.h> > #include <linux/of_gpio.h> > > +#ifdef CONFIG_FSL_SOC > #include <sysdev/fsl_soc.h> > #include <asm/cpm.h> > #include <asm/qe.h> > +#endif > > #include "spi-fsl-lib.h" > > +#ifdef CONFIG_FSL_SOC > /* CPM1 and CPM2 are mutually exclusive. */ > #ifdef CONFIG_CPM1 > #include <asm/cpm1.h> > @@ -47,6 +52,7 @@ > #include <asm/cpm2.h> > #define CPM_SPI_CMD mk_cr_cmd(CPM_CR_SPI_PAGE, CPM_CR_SPI_SBLOCK, 0, 0) > #endif > +#endif > > /* SPI Controller registers */ > struct fsl_spi_reg { > @@ -96,9 +102,11 @@ struct fsl_spi_reg { > #define SPI_PRAM_SIZE 0x100 > #define SPI_MRBLR ((unsigned int)PAGE_SIZE) > > +#ifdef CONFIG_FSL_SOC > static void *fsl_dummy_rx; > static DEFINE_MUTEX(fsl_dummy_rx_lock); > static int fsl_dummy_rx_refcnt; > +#endif > > static void fsl_spi_change_mode(struct spi_device *spi) > { > @@ -117,6 +125,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) > /* Turn off SPI unit prior changing mode */ > mpc8xxx_spi_write_reg(mode, cs->hw_mode & ~SPMODE_ENABLE); > > +#ifdef CONFIG_FSL_SOC > /* When in CPM mode, we need to reinit tx and rx. */ > if (mspi->flags & SPI_CPM_MODE) { > if (mspi->flags & SPI_QE) { > @@ -132,6 +141,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) > } > } > } > +#endif > mpc8xxx_spi_write_reg(mode, cs->hw_mode); > local_irq_restore(flags); > } > @@ -295,6 +305,7 @@ static int fsl_spi_setup_transfer(struct spi_device *spi, > return 0; > } > > +#ifdef CONFIG_FSL_SOC > static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) > { > struct cpm_buf_desc __iomem *tx_bd = mspi->tx_bd; > @@ -323,6 +334,9 @@ static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) > /* start transfer */ > mpc8xxx_spi_write_reg(®_base->command, SPCOM_STR); > } > +#else > +static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) { } > +#endif > > static int fsl_spi_cpm_bufs(struct mpc8xxx_spi *mspi, > struct spi_transfer *t, bool is_dma_mapped) > @@ -568,6 +582,7 @@ static int fsl_spi_setup(struct spi_device *spi) > return 0; > } > > +#ifdef CONFIG_FSL_SOC > static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) > { > u16 len; > @@ -591,6 +606,9 @@ static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) > else > complete(&mspi->done); > } > +#else > +static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) { } > +#endif > > static void fsl_spi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events) > { > @@ -646,6 +664,7 @@ static irqreturn_t fsl_spi_irq(s32 irq, void *context_data) > return ret; > } > > +#ifdef CONFIG_FSL_SOC > static void *fsl_spi_alloc_dummy_rx(void) > { > mutex_lock(&fsl_dummy_rx_lock); > @@ -836,6 +855,10 @@ static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) > cpm_muram_free(cpm_muram_offset(mspi->pram)); > fsl_spi_free_dummy_rx(); > } > +#else > +static int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi) { return -EINVAL; } > +static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) { } > +#endif > > static void fsl_spi_remove(struct mpc8xxx_spi *mspi) > { > @@ -1047,7 +1070,7 @@ static int __devinit of_fsl_spi_probe(struct platform_device *ofdev) > struct device_node *np = ofdev->dev.of_node; > struct spi_master *master; > struct resource mem; > - struct resource irq; > + int irq; > int ret = -ENOMEM; > > ret = of_mpc8xxx_spi_probe(ofdev); > @@ -1062,13 +1085,14 @@ static int __devinit of_fsl_spi_probe(struct platform_device *ofdev) > if (ret) > goto err; > > - ret = of_irq_to_resource(np, 0, &irq); > - if (!ret) { > + irq = irq_of_parse_and_map(np, 0); > + if (!irq) { > + dev_err(&ofdev->dev, "Could not get irq\n"); > ret = -EINVAL; > goto err; > } > > - master = fsl_spi_probe(dev, &mem, irq.start); > + master = fsl_spi_probe(dev, &mem, irq); > if (IS_ERR(master)) { > ret = PTR_ERR(master); > goto err; > -- > 1.7.0.4 > > ------------------------------------------------------------------------------ > Monitor your physical, virtual and cloud infrastructure from a single > web console. Get in-depth insight into apps, servers, databases, vmware, > SAP, cloud infrastructure, etc. Download 30-day Free Trial. > Pricing starts from $795 for 25 servers or applications! > http://p.sf.net/sfu/zoho_dev2dev_nov > _______________________________________________ > spi-devel-general mailing list > spi-devel-general@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/spi-devel-general > ------------------------------------------------------------------------------ LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial Remotely access PCs and mobile devices and provide instant support Improve your efficiency, and focus on delivering more value-add services Discover what IT Professionals Know. Rescue delivers http://p.sf.net/sfu/logmein_12329d2d
Andreas Larsson <andreas@gaisler.com> wrote on 2012/12/04 08:58:50: > > On 2012-11-22 10:26, Andreas Larsson wrote: > > I am looking into writing a driver for a core running on sparc that is mostly > > but not entirely compatible with the cpu mode of spi-fsl-spi. I am thinking of > > what could be the best approach for realizing this. Any comments on a preferred > > approach in this situation? > > Any comments on this? A fair amount of rearranging and ifdefing would be needed > to make the spi-fsl-spi driver workable outside an fsl and powerpc environment > so I rather get some input before before trying to perfect a solution that could > not be acceptable into mainline. I am far from kernel devlopment ATM but I do have one comment, how to ensure this new combined driver doesn't break on existing powerpc systems? Jocke > > Cheers, > Andreas > > > These are two different approaches I see to solve the situation without too much > > code duplication: > > > > Appproach A: Extend spi-fsl-spi and spi-fsl-lib to work outside of a FSL SOC > > environtment and outside powerpc. This would require ifdefs for the driver to be > > able to compile and work on sparc (or other platforms) - see patch draft at the > > end. Everything that has to do with cpm and sysdev/fsl_soc.h needs to be within > > ifdefs. Then the core in question could be added as another "mode" in the > > spi-fsl-spi driver with core specific code embedded inside spi-fsl-spi.c just as > > for the other existing modes. > > > > Approach B: Put the general and cpu mode specific functions from spi-fsl-spi in > > spi-fsl-lib or in a new separate c file and use these from both spi-fsl-spi and > > a new driver for the core in question. Some ifdefs would still be needed, but > > most things should be able to be handled with function pointers in such a > > solution. The question is where to move the general and cpu mode functions (but > > not the cpm related ones that could not compile) > > > > Of course something in between A and B could be done as well, where all > > functions stay in spi-fsl-spi.c but is exported so that they can be used with a > > new driver. Then most ifdefs in fsl-spi-fsl would need to be in place for > > compileability though. > > > > Of course a third option that leaves spi-fsl-* alone but results in a lot of > > code duplication is: > > > > Approach C: Submit a totally separate driver for this core (with heavy copying > > and pasting from spi-fsl-* as most of the functionality is the same as the cpu > > mode parts of spi-fsl-spi). A lot of ugly code duplication. The only upside > > compared to approach B is that there are mode register bits for this core that > > conflicts with mode register bits in other mpc8xxx cores that are not currently > > in use by the driver. If those would be used in the future in spi-fsl-spi, a > > separate driver would not break for this core. > > > > Core specific things that is needed in any approach (but can be mostly isolated > > from spi-fsl-* in approach B): > > - Add entries to the register struct for additional registers > > - Adding core-specific chipselect discovery/handling code as there might be > > built in chip select capabilities in the core. > > - Add core-specific code to handle driver matching, bus numbering, getting clock > > frequency, tx/rx_shifting, and dealing with possible word-length limitations. > > > > > > Below follows a draft patch on how spi-fsl-spi with friends could be made > > functioning in cpu mode outside of a FSL SOC environment. Approach A pointed out > > above would add the new mode on top of this for the needed core specific > > things. Note the ugly define on in/out_be16/32 that would need to be done in a > > proper way in a real patch. I didn't want to clutter the patch below with that > > now instead showing the ifdefs clearly. > > > > Cheers, > > Andreas Larsson > > > > --- > > drivers/spi/Kconfig | 4 ++-- > > drivers/spi/spi-fsl-lib.c | 10 ++++++++++ > > drivers/spi/spi-fsl-lib.h | 8 ++++++++ > > drivers/spi/spi-fsl-spi.c | 32 ++++++++++++++++++++++++++++---- > > 4 files changed, 48 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig > > index 5b017af..8fbd698 100644 > > --- a/drivers/spi/Kconfig > > +++ b/drivers/spi/Kconfig > > @@ -218,11 +218,11 @@ config SPI_MPC512x_PSC > > > > config SPI_FSL_LIB > > tristate > > - depends on FSL_SOC > > + depends on OF > > > > config SPI_FSL_SPI > > bool "Freescale SPI controller" > > - depends on FSL_SOC > > + depends on OF > > select SPI_FSL_LIB > > help > > This enables using the Freescale SPI controllers in master mode. > > diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c > > index 1503574..0c9021d 100644 > > --- a/drivers/spi/spi-fsl-lib.c > > +++ b/drivers/spi/spi-fsl-lib.c > > @@ -23,7 +23,9 @@ > > #include <linux/mm.h> > > #include <linux/of_platform.h> > > #include <linux/spi/spi.h> > > +#ifdef CONFIG_FSL_SOC > > #include <sysdev/fsl_soc.h> > > +#endif > > > > #include "spi-fsl-lib.h" > > > > @@ -208,6 +210,7 @@ int __devinit of_mpc8xxx_spi_probe(struct platform_device *ofdev) > > /* Allocate bus num dynamically. */ > > pdata->bus_num = -1; > > > > +#ifdef CONFIG_FSL_SOC > > /* SPI controller is either clocked from QE or SoC clock. */ > > pdata->sysclk = get_brgfreq(); > > if (pdata->sysclk == -1) { > > @@ -217,16 +220,23 @@ int __devinit of_mpc8xxx_spi_probe(struct > platform_device *ofdev) > > goto err; > > } > > } > > +#else > > + ret = of_property_read_u32(np, "clock-frequency", &pdata->sysclk); > > + if (ret) > > + goto err; > > +#endif > > > > prop = of_get_property(np, "mode", NULL); > > if (prop && !strcmp(prop, "cpu-qe")) > > pdata->flags = SPI_QE_CPU_MODE; > > +#ifdef CONFIG_FSL_SOC > > else if (prop && !strcmp(prop, "qe")) > > pdata->flags = SPI_CPM_MODE | SPI_QE; > > else if (of_device_is_compatible(np, "fsl,cpm2-spi")) > > pdata->flags = SPI_CPM_MODE | SPI_CPM2; > > else if (of_device_is_compatible(np, "fsl,cpm1-spi")) > > pdata->flags = SPI_CPM_MODE | SPI_CPM1; > > +#endif > > > > return 0; > > > > diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h > > index cbe881b..cd85170 100644 > > --- a/drivers/spi/spi-fsl-lib.h > > +++ b/drivers/spi/spi-fsl-lib.h > > @@ -20,6 +20,12 @@ > > > > #include <asm/io.h> > > > > +/* Inline replacement or something like that in a real patch */ > > +#define out_be32(reg, val) iowrite32be((val), (reg)) > > +#define out_be16(reg, val) iowrite16be((val), (reg)) > > +#define in_be32(reg) ioread32be((reg)) > > +#define in_be16(reg) ioread16be((reg)) > > + > > /* SPI/eSPI Controller driver's private data. */ > > struct mpc8xxx_spi { > > struct device *dev; > > @@ -34,8 +40,10 @@ struct mpc8xxx_spi { > > > > int subblock; > > struct spi_pram __iomem *pram; > > +#ifdef CONFIG_FSL_SOC > > struct cpm_buf_desc __iomem *tx_bd; > > struct cpm_buf_desc __iomem *rx_bd; > > +#endif > > > > struct spi_transfer *xfer_in_progress; > > > > diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c > > index 6a62934..f459416 100644 > > --- a/drivers/spi/spi-fsl-spi.c > > +++ b/drivers/spi/spi-fsl-spi.c > > @@ -30,15 +30,20 @@ > > #include <linux/mutex.h> > > #include <linux/of.h> > > #include <linux/of_platform.h> > > +#include <linux/of_address.h> > > +#include <linux/of_irq.h> > > #include <linux/gpio.h> > > #include <linux/of_gpio.h> > > > > +#ifdef CONFIG_FSL_SOC > > #include <sysdev/fsl_soc.h> > > #include <asm/cpm.h> > > #include <asm/qe.h> > > +#endif > > > > #include "spi-fsl-lib.h" > > > > +#ifdef CONFIG_FSL_SOC > > /* CPM1 and CPM2 are mutually exclusive. */ > > #ifdef CONFIG_CPM1 > > #include <asm/cpm1.h> > > @@ -47,6 +52,7 @@ > > #include <asm/cpm2.h> > > #define CPM_SPI_CMD mk_cr_cmd(CPM_CR_SPI_PAGE, CPM_CR_SPI_SBLOCK, 0, 0) > > #endif > > +#endif > > > > /* SPI Controller registers */ > > struct fsl_spi_reg { > > @@ -96,9 +102,11 @@ struct fsl_spi_reg { > > #define SPI_PRAM_SIZE 0x100 > > #define SPI_MRBLR ((unsigned int)PAGE_SIZE) > > > > +#ifdef CONFIG_FSL_SOC > > static void *fsl_dummy_rx; > > static DEFINE_MUTEX(fsl_dummy_rx_lock); > > static int fsl_dummy_rx_refcnt; > > +#endif > > > > static void fsl_spi_change_mode(struct spi_device *spi) > > { > > @@ -117,6 +125,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) > > /* Turn off SPI unit prior changing mode */ > > mpc8xxx_spi_write_reg(mode, cs->hw_mode & ~SPMODE_ENABLE); > > > > +#ifdef CONFIG_FSL_SOC > > /* When in CPM mode, we need to reinit tx and rx. */ > > if (mspi->flags & SPI_CPM_MODE) { > > if (mspi->flags & SPI_QE) { > > @@ -132,6 +141,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) > > } > > } > > } > > +#endif > > mpc8xxx_spi_write_reg(mode, cs->hw_mode); > > local_irq_restore(flags); > > } > > @@ -295,6 +305,7 @@ static int fsl_spi_setup_transfer(struct spi_device *spi, > > return 0; > > } > > > > +#ifdef CONFIG_FSL_SOC > > static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) > > { > > struct cpm_buf_desc __iomem *tx_bd = mspi->tx_bd; > > @@ -323,6 +334,9 @@ static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) > > /* start transfer */ > > mpc8xxx_spi_write_reg(®_base->command, SPCOM_STR); > > } > > +#else > > +static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) { } > > +#endif > > > > static int fsl_spi_cpm_bufs(struct mpc8xxx_spi *mspi, > > struct spi_transfer *t, bool is_dma_mapped) > > @@ -568,6 +582,7 @@ static int fsl_spi_setup(struct spi_device *spi) > > return 0; > > } > > > > +#ifdef CONFIG_FSL_SOC > > static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) > > { > > u16 len; > > @@ -591,6 +606,9 @@ static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) > > else > > complete(&mspi->done); > > } > > +#else > > +static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) { } > > +#endif > > > > static void fsl_spi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events) > > { > > @@ -646,6 +664,7 @@ static irqreturn_t fsl_spi_irq(s32 irq, void *context_data) > > return ret; > > } > > > > +#ifdef CONFIG_FSL_SOC > > static void *fsl_spi_alloc_dummy_rx(void) > > { > > mutex_lock(&fsl_dummy_rx_lock); > > @@ -836,6 +855,10 @@ static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) > > cpm_muram_free(cpm_muram_offset(mspi->pram)); > > fsl_spi_free_dummy_rx(); > > } > > +#else > > +static int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi) { return -EINVAL; } > > +static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) { } > > +#endif > > > > static void fsl_spi_remove(struct mpc8xxx_spi *mspi) > > { > > @@ -1047,7 +1070,7 @@ static int __devinit of_fsl_spi_probe(struct > platform_device *ofdev) > > struct device_node *np = ofdev->dev.of_node; > > struct spi_master *master; > > struct resource mem; > > - struct resource irq; > > + int irq; > > int ret = -ENOMEM; > > > > ret = of_mpc8xxx_spi_probe(ofdev); > > @@ -1062,13 +1085,14 @@ static int __devinit of_fsl_spi_probe(struct > platform_device *ofdev) > > if (ret) > > goto err; > > > > - ret = of_irq_to_resource(np, 0, &irq); > > - if (!ret) { > > + irq = irq_of_parse_and_map(np, 0); > > + if (!irq) { > > + dev_err(&ofdev->dev, "Could not get irq\n"); > > ret = -EINVAL; > > goto err; > > } > > > > - master = fsl_spi_probe(dev, &mem, irq.start); > > + master = fsl_spi_probe(dev, &mem, irq); > > if (IS_ERR(master)) { > > ret = PTR_ERR(master); > > goto err; > > -- > > 1.7.0.4 > > > > ------------------------------------------------------------------------------ > > Monitor your physical, virtual and cloud infrastructure from a single > > web console. Get in-depth insight into apps, servers, databases, vmware, > > SAP, cloud infrastructure, etc. Download 30-day Free Trial. > > Pricing starts from $795 for 25 servers or applications! > > http://p.sf.net/sfu/zoho_dev2dev_nov > > _______________________________________________ > > spi-devel-general mailing list > > spi-devel-general@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/spi-devel-general > > > ------------------------------------------------------------------------------ LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial Remotely access PCs and mobile devices and provide instant support Improve your efficiency, and focus on delivering more value-add services Discover what IT Professionals Know. Rescue delivers http://p.sf.net/sfu/logmein_12329d2d
On 2012-12-04 10:11, Joakim Tjernlund wrote: > Andreas Larsson <andreas@gaisler.com> wrote on 2012/12/04 08:58:50: >> Any comments on this? A fair amount of rearranging and ifdefing would >> be needed to make the spi-fsl-spi driver workable outside an fsl and >> powerpc environment so I rather get some input before before trying >> to perfect a solution that could not be acceptable into mainline. > > I am far from kernel devlopment ATM but I do have one comment, how to > ensure this new combined driver doesn't break on existing powerpc > systems? Of course it would be preferable if the future patch would be tested on the original system to be sure nothing breaks. Taking the "do everything inside the existing driver" approach could limit the risk of breaking anything by not moving things around. To get the driver to work outside an fsl environment ifdefs can be added in a way that the code would essentially be the same running on the original systems. Then, new branches that will never be taken on the original boards can handle the newly added mode. This might not produce the most elegant code in the end though. The easiest way to make sure that nothing is broken is of course if I submit a completely separate driver, the downside being the duplicate code/functionality. Cheers, Andreas ------------------------------------------------------------------------------ LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial Remotely access PCs and mobile devices and provide instant support Improve your efficiency, and focus on delivering more value-add services Discover what IT Professionals Know. Rescue delivers http://p.sf.net/sfu/logmein_12329d2d
On Thu, 22 Nov 2012 10:26:15 +0100, Andreas Larsson <andreas@gaisler.com> wrote: > I am looking into writing a driver for a core running on sparc that is mostly > but not entirely compatible with the cpu mode of spi-fsl-spi. I am thinking of > what could be the best approach for realizing this. Any comments on a preferred > approach in this situation? > > These are two different approaches I see to solve the situation without too much > code duplication: > > Appproach A: Extend spi-fsl-spi and spi-fsl-lib to work outside of a FSL SOC > environtment and outside powerpc. This would require ifdefs for the driver to be > able to compile and work on sparc (or other platforms) - see patch draft at the > end. Everything that has to do with cpm and sysdev/fsl_soc.h needs to be within > ifdefs. Then the core in question could be added as another "mode" in the > spi-fsl-spi driver with core specific code embedded inside spi-fsl-spi.c just as > for the other existing modes. >From what you've shown, I would support this approach. The proof is in what the final patch looks like though. g. ------------------------------------------------------------------------------ LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial Remotely access PCs and mobile devices and provide instant support Improve your efficiency, and focus on delivering more value-add services Discover what IT Professionals Know. Rescue delivers http://p.sf.net/sfu/logmein_12329d2d
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index 5b017af..8fbd698 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -218,11 +218,11 @@ config SPI_MPC512x_PSC config SPI_FSL_LIB tristate - depends on FSL_SOC + depends on OF config SPI_FSL_SPI bool "Freescale SPI controller" - depends on FSL_SOC + depends on OF select SPI_FSL_LIB help This enables using the Freescale SPI controllers in master mode. diff --git a/drivers/spi/spi-fsl-lib.c b/drivers/spi/spi-fsl-lib.c index 1503574..0c9021d 100644 --- a/drivers/spi/spi-fsl-lib.c +++ b/drivers/spi/spi-fsl-lib.c @@ -23,7 +23,9 @@ #include <linux/mm.h> #include <linux/of_platform.h> #include <linux/spi/spi.h> +#ifdef CONFIG_FSL_SOC #include <sysdev/fsl_soc.h> +#endif #include "spi-fsl-lib.h" @@ -208,6 +210,7 @@ int __devinit of_mpc8xxx_spi_probe(struct platform_device *ofdev) /* Allocate bus num dynamically. */ pdata->bus_num = -1; +#ifdef CONFIG_FSL_SOC /* SPI controller is either clocked from QE or SoC clock. */ pdata->sysclk = get_brgfreq(); if (pdata->sysclk == -1) { @@ -217,16 +220,23 @@ int __devinit of_mpc8xxx_spi_probe(struct platform_device *ofdev) goto err; } } +#else + ret = of_property_read_u32(np, "clock-frequency", &pdata->sysclk); + if (ret) + goto err; +#endif prop = of_get_property(np, "mode", NULL); if (prop && !strcmp(prop, "cpu-qe")) pdata->flags = SPI_QE_CPU_MODE; +#ifdef CONFIG_FSL_SOC else if (prop && !strcmp(prop, "qe")) pdata->flags = SPI_CPM_MODE | SPI_QE; else if (of_device_is_compatible(np, "fsl,cpm2-spi")) pdata->flags = SPI_CPM_MODE | SPI_CPM2; else if (of_device_is_compatible(np, "fsl,cpm1-spi")) pdata->flags = SPI_CPM_MODE | SPI_CPM1; +#endif return 0; diff --git a/drivers/spi/spi-fsl-lib.h b/drivers/spi/spi-fsl-lib.h index cbe881b..cd85170 100644 --- a/drivers/spi/spi-fsl-lib.h +++ b/drivers/spi/spi-fsl-lib.h @@ -20,6 +20,12 @@ #include <asm/io.h> +/* Inline replacement or something like that in a real patch */ +#define out_be32(reg, val) iowrite32be((val), (reg)) +#define out_be16(reg, val) iowrite16be((val), (reg)) +#define in_be32(reg) ioread32be((reg)) +#define in_be16(reg) ioread16be((reg)) + /* SPI/eSPI Controller driver's private data. */ struct mpc8xxx_spi { struct device *dev; @@ -34,8 +40,10 @@ struct mpc8xxx_spi { int subblock; struct spi_pram __iomem *pram; +#ifdef CONFIG_FSL_SOC struct cpm_buf_desc __iomem *tx_bd; struct cpm_buf_desc __iomem *rx_bd; +#endif struct spi_transfer *xfer_in_progress; diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c index 6a62934..f459416 100644 --- a/drivers/spi/spi-fsl-spi.c +++ b/drivers/spi/spi-fsl-spi.c @@ -30,15 +30,20 @@ #include <linux/mutex.h> #include <linux/of.h> #include <linux/of_platform.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> #include <linux/gpio.h> #include <linux/of_gpio.h> +#ifdef CONFIG_FSL_SOC #include <sysdev/fsl_soc.h> #include <asm/cpm.h> #include <asm/qe.h> +#endif #include "spi-fsl-lib.h" +#ifdef CONFIG_FSL_SOC /* CPM1 and CPM2 are mutually exclusive. */ #ifdef CONFIG_CPM1 #include <asm/cpm1.h> @@ -47,6 +52,7 @@ #include <asm/cpm2.h> #define CPM_SPI_CMD mk_cr_cmd(CPM_CR_SPI_PAGE, CPM_CR_SPI_SBLOCK, 0, 0) #endif +#endif /* SPI Controller registers */ struct fsl_spi_reg { @@ -96,9 +102,11 @@ struct fsl_spi_reg { #define SPI_PRAM_SIZE 0x100 #define SPI_MRBLR ((unsigned int)PAGE_SIZE) +#ifdef CONFIG_FSL_SOC static void *fsl_dummy_rx; static DEFINE_MUTEX(fsl_dummy_rx_lock); static int fsl_dummy_rx_refcnt; +#endif static void fsl_spi_change_mode(struct spi_device *spi) { @@ -117,6 +125,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) /* Turn off SPI unit prior changing mode */ mpc8xxx_spi_write_reg(mode, cs->hw_mode & ~SPMODE_ENABLE); +#ifdef CONFIG_FSL_SOC /* When in CPM mode, we need to reinit tx and rx. */ if (mspi->flags & SPI_CPM_MODE) { if (mspi->flags & SPI_QE) { @@ -132,6 +141,7 @@ static void fsl_spi_change_mode(struct spi_device *spi) } } } +#endif mpc8xxx_spi_write_reg(mode, cs->hw_mode); local_irq_restore(flags); } @@ -295,6 +305,7 @@ static int fsl_spi_setup_transfer(struct spi_device *spi, return 0; } +#ifdef CONFIG_FSL_SOC static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) { struct cpm_buf_desc __iomem *tx_bd = mspi->tx_bd; @@ -323,6 +334,9 @@ static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) /* start transfer */ mpc8xxx_spi_write_reg(®_base->command, SPCOM_STR); } +#else +static void fsl_spi_cpm_bufs_start(struct mpc8xxx_spi *mspi) { } +#endif static int fsl_spi_cpm_bufs(struct mpc8xxx_spi *mspi, struct spi_transfer *t, bool is_dma_mapped) @@ -568,6 +582,7 @@ static int fsl_spi_setup(struct spi_device *spi) return 0; } +#ifdef CONFIG_FSL_SOC static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) { u16 len; @@ -591,6 +606,9 @@ static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) else complete(&mspi->done); } +#else +static void fsl_spi_cpm_irq(struct mpc8xxx_spi *mspi, u32 events) { } +#endif static void fsl_spi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events) { @@ -646,6 +664,7 @@ static irqreturn_t fsl_spi_irq(s32 irq, void *context_data) return ret; } +#ifdef CONFIG_FSL_SOC static void *fsl_spi_alloc_dummy_rx(void) { mutex_lock(&fsl_dummy_rx_lock); @@ -836,6 +855,10 @@ static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) cpm_muram_free(cpm_muram_offset(mspi->pram)); fsl_spi_free_dummy_rx(); } +#else +static int fsl_spi_cpm_init(struct mpc8xxx_spi *mspi) { return -EINVAL; } +static void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) { } +#endif static void fsl_spi_remove(struct mpc8xxx_spi *mspi) { @@ -1047,7 +1070,7 @@ static int __devinit of_fsl_spi_probe(struct platform_device *ofdev) struct device_node *np = ofdev->dev.of_node; struct spi_master *master; struct resource mem; - struct resource irq; + int irq; int ret = -ENOMEM; ret = of_mpc8xxx_spi_probe(ofdev); @@ -1062,13 +1085,14 @@ static int __devinit of_fsl_spi_probe(struct platform_device *ofdev) if (ret) goto err; - ret = of_irq_to_resource(np, 0, &irq); - if (!ret) { + irq = irq_of_parse_and_map(np, 0); + if (!irq) { + dev_err(&ofdev->dev, "Could not get irq\n"); ret = -EINVAL; goto err; } - master = fsl_spi_probe(dev, &mem, irq.start); + master = fsl_spi_probe(dev, &mem, irq); if (IS_ERR(master)) { ret = PTR_ERR(master); goto err;