| Message ID | 1456286163-5703-1-git-send-email-b18965@freescale.com |
|---|---|
| State | Accepted |
| Commit | c5917b4b054d60c6c495f06b0538afed39dfe343 |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Alison, On Wed, Feb 24, 2016 at 11:56 AM, Alison Wang <b18965@freescale.com> wrote: > In general, a carriage return needs to execute before a line feed. > The patch is to change serial DM driver based on this rule. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > --- > drivers/serial/serial-uclass.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c > index 1c447ff..f154eb1 100644 > --- a/drivers/serial/serial-uclass.c > +++ b/drivers/serial/serial-uclass.c > @@ -123,11 +123,12 @@ static void _serial_putc(struct udevice *dev, char ch) > struct dm_serial_ops *ops = serial_get_ops(dev); > int err; > > + if (ch == '\n') > + _serial_putc(dev, '\r'); > + > do { > err = ops->putc(dev, ch); > } while (err == -EAGAIN); > - if (ch == '\n') > - _serial_putc(dev, '\r'); > } > We should also clean up all DM serial driver to remove this handling in their driver. eg: serial_lpuart.c/serial_mxc.c Regards, Bin
Hi, Bin, > On Wed, Feb 24, 2016 at 11:56 AM, Alison Wang <b18965@freescale.com> > wrote: > > In general, a carriage return needs to execute before a line feed. > > The patch is to change serial DM driver based on this rule. > > > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > > --- > > drivers/serial/serial-uclass.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/serial/serial-uclass.c > > b/drivers/serial/serial-uclass.c index 1c447ff..f154eb1 100644 > > --- a/drivers/serial/serial-uclass.c > > +++ b/drivers/serial/serial-uclass.c > > @@ -123,11 +123,12 @@ static void _serial_putc(struct udevice *dev, > char ch) > > struct dm_serial_ops *ops = serial_get_ops(dev); > > int err; > > > > + if (ch == '\n') > > + _serial_putc(dev, '\r'); > > + > > do { > > err = ops->putc(dev, ch); > > } while (err == -EAGAIN); > > - if (ch == '\n') > > - _serial_putc(dev, '\r'); > > } > > > > We should also clean up all DM serial driver to remove this handling in > their driver. eg: serial_lpuart.c/serial_mxc.c > [Alison Wang] Thanks for your advice. Yes, I agree. This handling in particular driver is redundant. It seems only serial_lpuart.c and serial_arc.c has this handling. For serial_mxc.c, this handling is only in the non-DM .putc function, but not in the DM .putc function. Do you think it is necessary to change the non-DM .putc function? Best Regards, Alison Wang
Hi Alison, On Wed, Feb 24, 2016 at 3:35 PM, Huan Wang <alison.wang@nxp.com> wrote: > Hi, Bin, > >> On Wed, Feb 24, 2016 at 11:56 AM, Alison Wang <b18965@freescale.com> >> wrote: >> > In general, a carriage return needs to execute before a line feed. >> > The patch is to change serial DM driver based on this rule. >> > >> > Signed-off-by: Alison Wang <alison.wang@nxp.com> >> > --- >> > drivers/serial/serial-uclass.c | 5 +++-- >> > 1 file changed, 3 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/serial/serial-uclass.c >> > b/drivers/serial/serial-uclass.c index 1c447ff..f154eb1 100644 >> > --- a/drivers/serial/serial-uclass.c >> > +++ b/drivers/serial/serial-uclass.c >> > @@ -123,11 +123,12 @@ static void _serial_putc(struct udevice *dev, >> char ch) >> > struct dm_serial_ops *ops = serial_get_ops(dev); >> > int err; >> > >> > + if (ch == '\n') >> > + _serial_putc(dev, '\r'); >> > + >> > do { >> > err = ops->putc(dev, ch); >> > } while (err == -EAGAIN); >> > - if (ch == '\n') >> > - _serial_putc(dev, '\r'); >> > } >> > >> >> We should also clean up all DM serial driver to remove this handling in >> their driver. eg: serial_lpuart.c/serial_mxc.c >> > [Alison Wang] Thanks for your advice. Yes, I agree. This handling in > particular driver is redundant. > > It seems only serial_lpuart.c and serial_arc.c has this handling. For > serial_mxc.c, this handling is only in the non-DM .putc function, > but not in the DM .putc function. Do you think it is necessary to change > the non-DM .putc function? > No, we should keep the non-DM putc function as it is now. But this serial_mxc.c, its non-DM putc version has the same issue as the serial-uclass.c, so it should be fixed. The legacy serial_pxa.c, serial_s3c24x0.c and usbtty.c also have the same problem. Regards, Bin
Hi, Bin, > On Wed, Feb 24, 2016 at 3:35 PM, Huan Wang <alison.wang@nxp.com> wrote: > > Hi, Bin, > > > >> On Wed, Feb 24, 2016 at 11:56 AM, Alison Wang <b18965@freescale.com> > >> wrote: > >> > In general, a carriage return needs to execute before a line feed. > >> > The patch is to change serial DM driver based on this rule. > >> > > >> > Signed-off-by: Alison Wang <alison.wang@nxp.com> > >> > --- > >> > drivers/serial/serial-uclass.c | 5 +++-- > >> > 1 file changed, 3 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/drivers/serial/serial-uclass.c > >> > b/drivers/serial/serial-uclass.c index 1c447ff..f154eb1 100644 > >> > --- a/drivers/serial/serial-uclass.c > >> > +++ b/drivers/serial/serial-uclass.c > >> > @@ -123,11 +123,12 @@ static void _serial_putc(struct udevice *dev, > >> char ch) > >> > struct dm_serial_ops *ops = serial_get_ops(dev); > >> > int err; > >> > > >> > + if (ch == '\n') > >> > + _serial_putc(dev, '\r'); > >> > + > >> > do { > >> > err = ops->putc(dev, ch); > >> > } while (err == -EAGAIN); > >> > - if (ch == '\n') > >> > - _serial_putc(dev, '\r'); > >> > } > >> > > >> > >> We should also clean up all DM serial driver to remove this handling > >> in their driver. eg: serial_lpuart.c/serial_mxc.c > >> > > [Alison Wang] Thanks for your advice. Yes, I agree. This handling in > > particular driver is redundant. > > > > It seems only serial_lpuart.c and serial_arc.c has this handling. For > > serial_mxc.c, this handling is only in the non-DM .putc function, but > > not in the DM .putc function. Do you think it is necessary to change > > the non-DM .putc function? > > > > No, we should keep the non-DM putc function as it is now. But this > serial_mxc.c, its non-DM putc version has the same issue as the serial- > uclass.c, so it should be fixed. The legacy serial_pxa.c, > serial_s3c24x0.c and usbtty.c also have the same problem. > > [Alison Wang] Ok, I get your idea. I will send another patch to fix them. Thanks. Best Regards, Alison Wang
On Wed, Feb 24, 2016 at 11:56 AM, Alison Wang <b18965@freescale.com> wrote: > In general, a carriage return needs to execute before a line feed. > The patch is to change serial DM driver based on this rule. > > Signed-off-by: Alison Wang <alison.wang@nxp.com> > --- > drivers/serial/serial-uclass.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 1c447ff..f154eb1 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -123,11 +123,12 @@ static void _serial_putc(struct udevice *dev, char ch) struct dm_serial_ops *ops = serial_get_ops(dev); int err; + if (ch == '\n') + _serial_putc(dev, '\r'); + do { err = ops->putc(dev, ch); } while (err == -EAGAIN); - if (ch == '\n') - _serial_putc(dev, '\r'); } static void _serial_puts(struct udevice *dev, const char *str)
In general, a carriage return needs to execute before a line feed. The patch is to change serial DM driver based on this rule. Signed-off-by: Alison Wang <alison.wang@nxp.com> --- drivers/serial/serial-uclass.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)