RE: [PATCH v3 3/7] eSPI: add eSPI controller support
> -Original Message-
> From: Anton Vorontsov [mailto:cbouatmai...@gmail.com]
> Sent: Friday, October 01, 2010 7:22 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-...@ozlabs.org; spi-devel-gene...@lists.sourceforge.net; linux-
> m...@lists.infradead.org; Gala Kumar-B11780
> Subject: Re: [PATCH v3 3/7] eSPI: add eSPI controller support
>
> Hello Mingkai,
>
> There are mostly cosmetic comments down below.
>
> > + /* spin until TX is done */
> > + while (((events = mpc8xxx_spi_read_reg(®_base->event))
> > + & SPIE_NF) == 0)
> > + cpu_relax();
>
> This is dangerous. There's a handy spin_event_timeout() in asm/delay.h.
>
When timeout, can I use return in the interrupt function directly like this?
if (!(events & SPIE_NF)) {
int ret;
/* spin until TX is done */
ret = spin_event_timeout(((events = mpc8xxx_spi_read_reg(
®_base->event)) & SPIE_NF) == 0, 1000, 0);
if (!ret) {
dev_err(mspi->dev, "tired waiting for SPIE_NF\n");
return;
}
}
> > +}
> > +
> > +static const struct of_device_id of_fsl_espi_match[] = {
> > + { .compatible = "fsl,mpc8536-espi" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, of_fsl_espi_match);
> > +
> > +static struct of_platform_driver fsl_espi_driver = {
> > + .driver = {
> > + .name = "fsl_espi",
> > + .owner = THIS_MODULE,
> > + .of_match_table = of_fsl_espi_match,
> > + },
> > + .probe = of_fsl_espi_probe,
> > + .remove = __devexit_p(of_fsl_espi_remove),
> > +};
> > +
> > +static int __init fsl_espi_init(void) {
> > + return of_register_platform_driver(&fsl_espi_driver);
> > +}
> > +module_init(fsl_espi_init);
> > +
> > +static void __exit fsl_espi_exit(void) {
> > + of_unregister_platform_driver(&fsl_espi_driver);
> > +}
> > +module_exit(fsl_espi_exit);
> > +
> > +MODULE_AUTHOR("Mingkai Hu");
> > +MODULE_DESCRIPTION("Enhanced Freescale SPI Driver");
>
> This sounds like that this is an enhanced version of the Freescale SPI driver,
> which it is not. ;-)
>
I quoted from the UM, maybe the enhancement is the controller takes over the
CS signal from the HW point view.
I changed all the other code according to your comments.
Thanks,
Mingkai
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 3/7] eSPI: add eSPI controller support
Hello Mingkai,
There are mostly cosmetic comments down below.
On Thu, Sep 30, 2010 at 04:00:42PM +0800, Mingkai Hu wrote:
[...]
> +/*
> + * Freescale eSPI controller driver.
> + *
> + * Copyright 2010 Freescale Semiconductor, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the
> + * Free Software Foundation; either version 2 of the License, or (at your
> + * option) any later version.
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
Please move the sysdev/ include after linux/.
Will make it a bit prettier. :-)
> +#include
> +#include
> +
> +#include "spi_fsl_lib.h"
[...]
> +static void fsl_espi_change_mode(struct spi_device *spi)
> +{
> + struct mpc8xxx_spi *mspi = spi_master_get_devdata(spi->master);
> + struct spi_mpc8xxx_cs *cs = spi->controller_state;
> + struct fsl_espi_reg *reg_base = (struct fsl_espi_reg *)mspi->reg_base;
No need for the type cast. The same for the rest of the code.
> + __be32 __iomem *mode;
> + __be32 __iomem *espi_mode = NULL;
> + u32 tmp;
> + unsigned long flags;
> +
> + espi_mode = ®_base->mode;
> + mode = ®_base->csmode[spi->chip_select];
Could save a few lines by turning this into initializers.
> +
> + /* Turn off IRQs locally to minimize time that SPI is disabled. */
> + local_irq_save(flags);
> +
> + /* Turn off SPI unit prior changing mode */
> + tmp = mpc8xxx_spi_read_reg(espi_mode);
> + mpc8xxx_spi_write_reg(espi_mode, tmp & ~SPMODE_ENABLE);
> + mpc8xxx_spi_write_reg(mode, cs->hw_mode);
> + mpc8xxx_spi_write_reg(espi_mode, tmp);
> +
> + local_irq_restore(flags);
> +}
> +
> +static u32 fsl_espi_tx_buf_lsb(struct mpc8xxx_spi *mpc8xxx_spi)
> +{
> + u32 data;
> + u16 data_h, data_l;
u16 data_h;
u16 data_l;
> +
No need for this empty line.
> + const u32 *tx = mpc8xxx_spi->tx;
<- Instead, add an empty line here.
> + if (!tx)
> + return 0;
> +
> + data = *tx++ << mpc8xxx_spi->tx_shift;
> + data_l = data & 0x;
> + data_h = (data >> 16) & 0x;
> + swab16s(&data_l);
> + swab16s(&data_h);
> + data = data_h | data_l;
> +
> + mpc8xxx_spi->tx = tx;
> + return data;
> +}
> +
> +static int fsl_espi_setup_transfer(struct spi_device *spi,
> + struct spi_transfer *t)
> +{
> + struct mpc8xxx_spi *mpc8xxx_spi;
> + int bits_per_word = 0;
> + u8 pm;
> + u32 hz = 0;
> + struct spi_mpc8xxx_cs *cs = spi->controller_state;
Stray tab.
> +
> + mpc8xxx_spi = spi_master_get_devdata(spi->master);
Could move this to the initializer.
> +
> + if (t) {
> + bits_per_word = t->bits_per_word;
> + hz = t->speed_hz;
> + }
> +
> + /* spi_transfer level calls that work per-word */
> + if (!bits_per_word)
> + bits_per_word = spi->bits_per_word;
> +
> + /* Make sure its a bit width we support [4..16] */
> + if ((bits_per_word < 4) || (bits_per_word > 16))
> + return -EINVAL;
> +
> + if (!hz)
> + hz = spi->max_speed_hz;
> +
> + cs->rx_shift = 0;
> + cs->tx_shift = 0;
> + cs->get_rx = mpc8xxx_spi_rx_buf_u32;
> + cs->get_tx = mpc8xxx_spi_tx_buf_u32;
> + if (bits_per_word <= 8) {
> + cs->rx_shift = 8 - bits_per_word;
> + } else if (bits_per_word <= 16) {
> + cs->rx_shift = 16 - bits_per_word;
> + if (spi->mode & SPI_LSB_FIRST)
> + cs->get_tx = fsl_espi_tx_buf_lsb;
> + } else
> + return -EINVAL;
} else {
}
> +
> + mpc8xxx_spi->rx_shift = cs->rx_shift;
> + mpc8xxx_spi->tx_shift = cs->tx_shift;
> + mpc8xxx_spi->get_rx = cs->get_rx;
> + mpc8xxx_spi->get_tx = cs->get_tx;
> +
> + bits_per_word = bits_per_word - 1;
> +
> + /* mask out bits we are going to set */
> + cs->hw_mode &= ~(CSMODE_LEN(0xF) | CSMODE_DIV16
> + | CSMODE_PM(0xF));
No need to break this statement.
> +
> + cs->hw_mode |= CSMODE_LEN(bits_per_word);
> +
> + if ((mpc8xxx_spi->spibrg / hz) > 64) {
> + cs->hw_mode |= CSMODE_DIV16;
> + pm = (mpc8xxx_spi->spibrg - 1) / (hz * 64) + 1;
> +
> + WARN_ONCE(pm > 16, "%s: Requested speed is too low: %d Hz. "
> + "Will use %d Hz instead.\n", dev_name(&spi->dev),
> + hz, mpc8xxx_spi->spibrg / 1024);
> + if (pm > 16)
> + pm = 16;
> + } else {
> + pm = (mpc8xxx_spi->spibrg - 1) / (hz * 4) + 1;
> + }
> + if (pm)
> + pm--;
> +
> + cs->hw_mode |= CSMODE_PM(pm);
> +
> + fsl_espi_change_mode(spi);
> + return 0;
> +}
> +
> +int fsl_espi_cpu_bufs(struct mpc8xxx_spi *mspi, struct spi_transfer *t,
>
[PATCH v3 3/7] eSPI: add eSPI controller support
Add eSPI controller support based on the library code spi_fsl_lib.c.
The eSPI controller is newer controller 85xx/Pxxx devices supported.
There're some differences comparing to the SPI controller:
1. Has different register map and different bit definition
So leave the code operated the register to the driver code, not
the common code.
2. Support 4 dedicated chip selects
The software can't controll the chip selects directly, The SPCOM[CS]
field is used to select which chip selects is used, and the
SPCOM[TRANLEN] field is set to tell the controller how long the CS
signal need to be asserted. So the driver doesn't need the chipselect
related function when transfering data, just set corresponding register
fields to controll the chipseclect.
3. Different Transmit/Receive FIFO access register behavior
For SPI controller, the Tx/Rx FIFO access register can hold only
one character regardless of the character length, but for eSPI
controller, the register can hold 4 or 2 characters according to
the character lengths. Access the Tx/Rx FIFO access register of the
eSPI controller will shift out/in 4/2 characters one time. For SPI
subsystem, the command and data are put into different transfers, so
we need to combine all the transfers to one transfer in order to pass
the transfer to eSPI controller.
Signed-off-by: Mingkai Hu
---
v3:
- Update to the latest kernel base.
drivers/spi/Kconfig|9 +
drivers/spi/Makefile |1 +
drivers/spi/spi_fsl_espi.c | 642
drivers/spi/spi_fsl_lib.h |1 +
4 files changed, 653 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/spi_fsl_espi.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 79ad06f..f6888af 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -195,6 +195,15 @@ config SPI_FSL_SPI
MPC83xx platform uses the controller in cpu mode or CPM/QE mode.
MPC8569 uses the controller in QE mode, MPC8610 in cpu mode.
+config SPI_FSL_ESPI
+ tristate "Freescale eSPI controller"
+ depends on FSL_SOC
+ select SPI_FSL_LIB
+ help
+ This enables using the Freescale eSPI controllers in master mode.
+ From MPC8536, 85xx platform uses the controller, and all P10xx,
+ P20xx, P30xx,P40xx, P50xx uses this controller.
+
config SPI_OMAP_UWIRE
tristate "OMAP1 MicroWire"
depends on ARCH_OMAP1
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 7974c21..833d17e 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -35,6 +35,7 @@ obj-$(CONFIG_SPI_MPC512x_PSC) += mpc512x_psc_spi.o
obj-$(CONFIG_SPI_MPC52xx_PSC) += mpc52xx_psc_spi.o
obj-$(CONFIG_SPI_MPC52xx) += mpc52xx_spi.o
obj-$(CONFIG_SPI_FSL_LIB) += spi_fsl_lib.o
+obj-$(CONFIG_SPI_FSL_ESPI) += spi_fsl_espi.o
obj-$(CONFIG_SPI_FSL_SPI) += spi_fsl_spi.o
obj-$(CONFIG_SPI_PPC4xx) += spi_ppc4xx.o
obj-$(CONFIG_SPI_S3C24XX_GPIO) += spi_s3c24xx_gpio.o
diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
new file mode 100644
index 000..be98148
--- /dev/null
+++ b/drivers/spi/spi_fsl_espi.c
@@ -0,0 +1,642 @@
+/*
+ * Freescale eSPI controller driver.
+ *
+ * Copyright 2010 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "spi_fsl_lib.h"
+
+/* eSPI Controller registers */
+struct fsl_espi_reg {
+ __be32 mode;/* 0x000 - eSPI mode register */
+ __be32 event; /* 0x004 - eSPI event register */
+ __be32 mask;/* 0x008 - eSPI mask register */
+ __be32 command; /* 0x00c - eSPI command register */
+ __be32 transmit;/* 0x010 - eSPI transmit FIFO access register*/
+ __be32 receive; /* 0x014 - eSPI receive FIFO access register*/
+ u8 res[8]; /* 0x018 - 0x01c reserved */
+ __be32 csmode[4]; /* 0x020 - 0x02c eSPI cs mode register */
+};
+
+/* eSPI Controller mode register definitions */
+#define SPMODE_ENABLE (1 << 31)
+#define SPMODE_LOOP(1 << 30)
+#define SPMODE_TXTHR(x)((x) << 8)
+#define SPMODE_RXTHR(x)((x) << 0)
+
+/* eSPI Controller CS mode register definitions */
+#define CSMODE_CI_INACTIVEHIGH (1 << 31)
+#define CSMODE_CP_BEGIN_EDGECLK(1 << 30)
+#define CSMODE_REV (1 << 29)
+#define CSMODE_DIV16 (1 << 28)
+#define CSMODE_PM(x) ((x) << 24)
+#define CSMODE_POL_1 (1 << 20)
+#def
