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

[Jagan Teki]

On 18 November 2014 14:42, Joakim Tjernlund
<joakim.tjernlund at transmode.se> wrote:
> Ping?
>
> Joakim Tjernlund/Transmode wrote on 2014/10/29 19:43:14:

Couldn't understand what discussion is going, does some issue in
driver or any plan to
write new code, please let me know - we can plan accordingly and kill
the thread.

>>
>> "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
>> > >
>

thanks!
-- 
Jagan.