[U-Boot] u-boot, fsl_espi.c driver

[Joakim Tjernlund]

Ping?

Joakim Tjernlund/Transmode wrote on 2014/10/29 19:43:14:
> 
> "Mingkai.Hu at freescale.com" <Mingkai.Hu at freescale.com> wrote on 
2014/10/28 12:17:24:
> > 
> > Hi Joakim and York,
> 
> Hi yourself, been travelling or a few days.
> 
> > 
> > I apologize for the delayed response and thanks for your catch up, 
Joakim
> > In order to get a higher transfer speed, the eSPI controller was 
designed to transfer multiple bytes at one transaction which is not comply 
with the SPI framework and the CS can't be inactive until one transfer 
finished, so we need to combine the multiple transfers into one transfer, 
that's the reason why there are more memory copy back and forth.
> 
> hmm, can't make sense of this. If you can memcpy from/to memory you can 
also write/read the SPI FIFO
> Also writing 4 bytes to the SPI FIFO at a time will not but you much if 
anything at all. It just
> makes it impossible to use NF flag etc.
> Speed will come from keeping the SPI FIFO non empty and not copying 
memory back and fourth.
> CS can be controlled within the SPI framework already.
> 
> > 
> > The driver has been tested on SPI flash and the transfer length larger 
than max_trans_len should be handled but half-duplex 
> was not handled. Is it possible to provide patch to make it workable for 
different devices?
> 
> Not as the driver is written today.
> 
> > 
> > 0x0b is the FAST READ command. The logic here is to move the pointer 
of receive buffer to receive new data when reading from NOR flash and the 
reading length is larger than the max_trans_len.
> 
> What FAST READ command? Sounds like it is connected to the FLASH rather 
than SPI? if so that
> need to go from the SPI driver and moved into some board specific addon.
> 
> > 
> > Thanks,
> > Mingkai
> > 
> > ________________________________________
> > From: Sun York-R58495
> > Sent: Thursday, October 23, 2014 6:14 AM
> > To: jagan Teki; Hu Mingkai-B21284
> > Cc: Joakim Tjernlund; Hou Zhiqiang-B48286; u-boot at lists.denx.de
> > Subject: Re: u-boot, fsl_espi.c driver
> > 
> > Jagan and Mingkai,
> > 
> > Would you take a look when you have a chance?
> > 
> > York
> > 
> > 
> > On 10/22/2014 03:12 PM, Joakim Tjernlund wrote:
> > > York Sun <yorksun at freescale.com> wrote on 2014/10/09 20:06:31:
> > >>
> > >> Dear Joakim,
> > >>
> > >> On 10/09/2014 10:43 AM, Joakim Tjernlund wrote:
> > >>> York Sun <yorksun at freescale.com> wrote on 2014/10/09 18:25:40:
> > >>>>
> > >>>> Dear Joakim,
> > >>>>
> > >>>> Thanks for raising a concern.
> > >>>>
> > >>>> It's not fair to blame the last person who submitted a patch. We 
are
> > > all
> > >>> working
> > >>>
> > >>> No of course not, I just noticed you guys had been in there and
> > > patched up
> > >>> some problem
> > >>> so I hoped you would be interested to fix the remaining problems. 
This
> > >
> > >>> driver should never
> > >>> have been committed in the first place.
> > >>>
> > >>>> to make it better as an opensource comminuty. You have done a 
good
> > > job
> > >>> to hack
> > >>>> it to work. Would it be nicer if you can submit this or improved
> > > patch
> > >>> to u-boot
> > >>>> community for further review and testing, after putting informing
> > > commit
> > >>>> message? The mailing list address is CC'ed.
> > >>>
> > >>> Main problem with this driver is that TX does not work for:
> > >>>  len > max_tran_len (nor does RX)
> > >>>  does not TX the last odd bytes(len % 4 != 0)
> > >>>  Does not work with TX only(RX buf == NULL)
> > >>>
> > >>> Silently ignores SPI_LSB_FIRST
> > >>>
> > >>> On top of that it uses malloc all over and copies data back and 
forth
> > > for
> > >>> no good
> > >>> reason, image a big SPI transfer with many MB(like my FPGA load).
> > >>>
> > >>> I am not in a good position fix this properly as my FPGA is TX 
only so
> > > I
> > >>> cannot test
> > >>> RX at all(which is broken by my hack)
> > >>>
> > >>> Finally, just to illustrate the merits of this driver, after
> > > transmission
> > >>> is done
> > >>> there is:
> > >>>  if (*buffer == 0x0b) {
> > >>>                                 data_in += tran_len;
> > >>>                                 data_len -= tran_len;
> > >>>                                 *(int *)buffer += tran_len;
> > >>>                         }
> > >>> what is the magic 0x0b test all about?
> > >>>
> > >>
> > >> Thanks for the report. Driver maintainer (CC'ed) will take a look.
> > >>
> > >> York
> > >
> > > No reaction from maintainers, I don't think this driver is still
> > > maintained.
> > >
> > >    Jocke
> > >