RE: [PATCH v2 2/6] eSPI: add eSPI controller support
> -Original Message-
> From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant
> Likely
> Sent: Tuesday, August 10, 2010 2:45 PM
> To: Hu Mingkai-B21284
> Cc: linuxppc-...@ozlabs.org; spi-devel-gene...@lists.sourceforge.net; Gala
> Kumar-B11780; Zang Roy-R61911
> Subject: Re: [PATCH v2 2/6] eSPI: add eSPI controller support
>
> On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu wrote:
> > 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
>
> I've not dug deep into this patch, but it seems pretty good. I did notice one
> thing below...
> [...]
>
> > diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h
> > index 774e1c8..0772c98 100644
> > --- a/drivers/spi/spi_fsl_lib.h
> > +++ b/drivers/spi/spi_fsl_lib.h
> > @@ -22,10 +22,12 @@
> > struct mpc8xxx_spi {
> > struct device *dev;
> > struct fsl_spi_reg __iomem *base;
> > + struct fsl_espi_reg __iomem *espi_base;
>
> There's got to be a cleaner way to do this. Rather than putting both pointers
> into mpc8xxx_spi, I suggest each driver use its own fsl_spi_priv and
> fsl_espi_priv wrapper structures that contain the controller specific
> properties.
>
Make sense, I'll change it.
Thanks,
Mingkai
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 2/6] eSPI: add eSPI controller support
On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu wrote:
> 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
I've not dug deep into this patch, but it seems pretty good. I did
notice one thing below...
[...]
> diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h
> index 774e1c8..0772c98 100644
> --- a/drivers/spi/spi_fsl_lib.h
> +++ b/drivers/spi/spi_fsl_lib.h
> @@ -22,10 +22,12 @@
> struct mpc8xxx_spi {
> struct device *dev;
> struct fsl_spi_reg __iomem *base;
> + struct fsl_espi_reg __iomem *espi_base;
There's got to be a cleaner way to do this. Rather than putting both
pointers into mpc8xxx_spi, I suggest each driver use its own
fsl_spi_priv and fsl_espi_priv wrapper structures that contain the
controller specific properties.
cheers,
g.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v2 2/6] 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
---
v2:
- Rename fsl_espi.c to spi_fsl_espi.c, also the config name
- Move register map definiton from spi_fsl_lib.c to spi_fsl_espi.c
- Break some funcions line in the arguments instead of the declaration
- Inconsistent whitespacing in the macro definition
- Init bits_per_word to 0 to eliminate the else clause
- Add brace for the else clause to match if clause
- Add chip name mpc8536 to the compatible value
- Drop the last entry's comma in the match table
- move module_init() immediately after the init fsl_espi_init() function
drivers/spi/Kconfig|9 +
drivers/spi/Makefile |1 +
drivers/spi/spi_fsl_espi.c | 633
drivers/spi/spi_fsl_lib.h |2 +
4 files changed, 645 insertions(+), 0 deletions(-)
create mode 100644 drivers/spi/spi_fsl_espi.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index cd7f13b..a379363 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -140,6 +140,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, P40xx uses this controller.
+
config SPI_GPIO
tristate "GPIO-based bitbanging SPI Master"
depends on GENERIC_GPIO
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index cf8d9be..dd86ba7 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_SPI_DW_MMIO) += dw_spi_mmio.o
obj-$(CONFIG_SPI_EP93XX) += ep93xx_spi.o
obj-$(CONFIG_SPI_FSL_LIB) += spi_fsl_lib.o
obj-$(CONFIG_SPI_FSL_SPI) += spi_fsl_spi.o
+obj-$(CONFIG_SPI_FSL_ESPI) += spi_fsl_espi.o
obj-$(CONFIG_SPI_GPIO) += spi_gpio.o
obj-$(CONFIG_SPI_IMX) += spi_imx.o
obj-$(CONFIG_SPI_LM70_LLP) += spi_lm70llp.o
diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
new file mode 100644
index 000..61987cf
--- /dev/null
+++ b/drivers/spi/spi_fsl_espi.c
@@ -0,0 +1,633 @@
+/*
+ * 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 "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 << 3
