[U-Boot] [PATCH 3/7] eSPI: add eSPI controller support

[Hu Mingkai-B21284]

Hi Mike,

First truly sorry for the long time delay.
Please find comments inline.

> -----Original Message-----
> From: Mike Frysinger [mailto:vapier at gentoo.org] 
> Sent: Monday, March 16, 2009 6:06 PM
> To: Hu Mingkai-B21284
> Cc: u-boot at lists.denx.de; Jean-Christophe PLAGNIOL-VILLARD; 
> Fleming Andy-AFLEMING
> Subject: Re: [U-Boot] [PATCH 3/7] eSPI: add eSPI controller support
> 
> On Monday 16 March 2009 04:56:22 Hu Mingkai-B21284 wrote:
> > From: Mike Frysinger [mailto:vapier at gentoo.org]
> > > On Monday 16 March 2009 02:15:27 Jean-Christophe wrote:
> > > > On 10:52 Mon 16 Mar     , Mingkai Hu wrote:
> > > > > Add eSPI controller support under the SPI framework.
> > > > >
> > > > > Signed-off-by: Mingkai Hu <Mingkai.hu at freescale.com>
> > > > > ---
> > > > >  drivers/mtd/spi/spi_flash.c |   94 +++++++++++++++++-
> > > > >  drivers/spi/Makefile        |    1 +
> > > > >  drivers/spi/fsl_espi.c      |  232
> > > > > +++++++++++++++++++++++++++++++++++++++++++ include/spi.h
> > > > >
> > > > >   30 ++++++
> > > > >  4 files changed, 354 insertions(+), 3 deletions(-)  
> create mode
> > > > > 100644 drivers/spi/fsl_espi.c
> > > >
> > > > please use the spi framework, their is no need to touch the 
> > > > spi_flash support ti add and use the fsl_espi
> > >
> > > indeed ... i was so happy to see the spansion driver i 
> glossed over 
> > > this ...
> > > those spi_flash.c changes are almost assuredly wrong and indicate 
> > > you arent fitting into the framework correctly.
> >
> > There is difference between the SPI and eSPI chip select signal.
> > Generally, the SPI chip select is controlled by GPIO, so SW can 
> > control when to assert the chip select signal and when to stop the 
> > chip select signal.
> 
> that is incorrect.  the SPI framework does not care how the 
> controller manages the CS.  the Blackfin SPI controller for 
> example has dedicated CS's which the controller manages automatically.
> 
> > But the eSPI controller integrate the chip select into the 
> controller 
> > itself, it use the transfer length to control the duration 
> of the chip 
> > select signal.
> 
> so you must tell the controller ahead of time how many bytes 
> are going to be transferred ?  this sounds like a retarded 
> hardware implementation to me.  how exactly do you support 
> arbitrary transfer lengths ?
> 

IIRC every transaction byte length control every transaction.

> > The spi_flash_cmd_* function split the transfer into two 
> steps: first 
> > the command transfer and then the data transfer. At the first step, 
> > the transfer length is set to the command length, and when 
> transferred 
> > the command, the chip select will be negated, so the eSPI 
> controller 
> > thinks the transfer is cancelled and doesn't transfer the 
> data again.
> 
> the transfer state is passed in via the flags parameter.  
> this way the SPI controller knows when a transfer starts 
> (SPI_XFER_BEGIN) and when it ends (SPI_XFER_END).
> -mike
> 
As above, we can't use the start/end flags parameter.

Sorry again for the delayed response.

Many thanks,
Mingkai