Hi Simon and Stefan,
Sorry for my late answer (I just come back from my summer holiday)
> From: s...@google.com <s...@google.com> On Behalf Of Simon Glass
> Sent: mercredi 8 août 2018 11:56
>
> On 3 August 2018 at 05:38, Patrick Delaunay <patrick.delau...@st.com> wrote:
> > Add test to avoid access to rx buffer when this buffer is empty.
> > In this case directly call getc() function to avoid issue when tstc()
> > is not called.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delau...@st.com>
> > ---
> >
> > drivers/serial/serial-uclass.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/serial/serial-uclass.c
> > b/drivers/serial/serial-uclass.c index 321d23e..4121a37 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -228,6 +228,9 @@ static int _serial_getc(struct udevice *dev)
> > struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
> > char val;
> >
> > + if (upriv->rd_ptr == upriv->wr_ptr)
> > + return __serial_getc(dev);
> > +
> > val = upriv->buf[upriv->rd_ptr++];
> > upriv->rd_ptr %= CONFIG_SERIAL_RX_BUFFER_SIZE;
> >
> > --
> > 2.7.4
> >
>
> Reviewed-by: Simon Glass <s...@chromium.org>
>
> BTW I think the code in _serial_tstc() is wrong, or at least sub-optimal:
>
> /* Read all available chars into the RX buffer */ while (__serial_tstc(dev)) {
> upriv->buf[upriv->wr_ptr++] = __serial_getc(dev);
> upriv->wr_ptr %= CONFIG_SERIAL_RX_BUFFER_SIZE; }
>
> It should call serial_getc() until it returns -EAGAIN. There should be no
> need to
> call __serial_tstc() first,
This part had be added by Stefan Roese in SHA1
3ca7a06afb7cbc867b7861a8b9aada74e5bbb559
But I check again the code, and I think the code is correct.... but I agree it
is not optimal.
we can directly ops->getc(dev) and no more __serial_getc() or __serial_tstc() :
the behavior don't change but the access to serial driver is reduced.
When no char is available, ops->getc witll return -EAGAIN
static int _serial_tstc(struct udevice *dev)
{
struct dm_serial_ops *ops = serial_get_ops(dev);
struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
int err;
/* Read all available chars into the RX buffer */
while(1) {
err = ops->getc(dev);
if (err < 0)
break;
upriv->buf[upriv->wr_ptr++] = err;
upriv->wr_ptr %= CONFIG_SERIAL_RX_BUFFER_SIZE;
}
return upriv->rd_ptr != upriv->wr_ptr ? 1 : 0;
}
If you are OK I will push a other patchset for these otpimisation.
>
> Regards,
> Simon
Regards
Patrick
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
