On Thu, 2019-05-02 at 07:31 +0200, Mario Six wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe.
> 
> 
> Hi Jagan and Jocke,
> 
> I'm back from vacation, so here's my answer:
> 
> On Mon, Apr 29, 2019 at 12:41 PM Jagan Teki <ja...@amarulasolutions.com> 
> wrote:
> > + Mario
> > 
> > On Mon, Apr 29, 2019 at 2:48 PM Joakim Tjernlund
> > <joakim.tjernl...@infinera.com> wrote:
> > > On Mon, 2019-04-29 at 01:58 +0530, Jagan Teki wrote:
> > > > From: Mario Six <mario....@gdsys.cc>
> > > > 
> > > > We do nothing in the loop if the "not empty" event was not detected. To
> > > > simplify the logic, check if this is the case, and skip the execution of
> > > > the loop early to reduce the nesting level and flag checking.
> > > 
> > > Looked at the driver to refresh memory and noticed:
> > > if (charSize == 32) {
> > >         /* Advance output buffer by 32 bits */
> > >         din += 4;
> > > }
> > > which suggests that only 32 bit char will increase the din ptr so does 
> > > other bitlens
> > > work for reading?
> Yes, It will work. When charSize < 32 in a loop execution, we necessarily also
> have numBlks == 0, since numBlks = DIV_ROUND_UP(bitlen, 32) and charSize =
> (bitlen >= 32 ? 32 : bitlen); in other words: we're at the very end of the 
> data
> to be sent/received and also the final loop execution, so we don't have to
> advance the pointer anymore.

Ahh, I see now.

But this over use of always 32 bits cause complexity/subtile bugs. The driver 
should use
the requested charsize(wordlen) throughout. As is now you cannot use the NF/NE 
flag as intended or
toggle LSB_FIRST
I think fsl_espi driver is worse though.

    Jocke
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to