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
_______________________________________________
spi-devel-general mailing list
spi-devel-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/spi-devel-general
