> -----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 <mingkai...@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...@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. >
Make sense, I'll change it. Thanks, Mingkai _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
