[PATCH v2 2/6] eSPI: add eSPI controller support

Grant Likely grant.likely at secretlab.ca
Tue Aug 10 16:44:48 EST 2010


On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu at freescale.com> 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 <Mingkai.hu at freescale.com>

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.


More information about the Linuxppc-dev mailing list