All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated
@ 2018-08-03 11:38 Patrick Delaunay
  2018-08-03 11:38 ` [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer Patrick Delaunay
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Patrick Delaunay @ 2018-08-03 11:38 UTC (permalink / raw)
  To: u-boot


When I activate CONFIG_SERIAL_RX_BUFFER on my board without CONSOLE_MUX,
I have a strange behavior in console: a new prompt is displayed
continuously

I check the call stack

cread_line (common/cli_readline.c)
=> getcmd_getch (common/cli_readline.c)
==> fgetc (common/console.c)
===> console_tstc (common/console.c)
====> stdio_devices[file]->get()
=====> _serial_getc() (drivers/serial/serial-uclass.c)
       and the data reads from rx buffer but it is garbage
       as tstc is not called

PS: I have no issue when CONSOLE_MUX is activated because in this
    case the tstc() is always called in fgetc loop.

My first solution to update the rx buffer management,
to avoid the issue:

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)
		__serial_tstc(dev);

	if (upriv->rd_ptr == upriv->wr_ptr)
		return 0; /* error : no data to read */

	val = upriv->buf[upriv->rd_ptr++];
	upriv->rd_ptr %= CONFIG_SERIAL_RX_BUFFER_SIZE;

	return val;
}

With this first patch, I see that that WATCHDOG is not reloaded
and the getc() error value 0x0 wasn't correctly handle in cli;
the issue is alway present and the behavior change (getc function
is no more blocking): the caller needs to handle the error and
reload the watchdog.

To summarize, I see several issues in the current U-Boot code:
- No protection on the rx buffer in _serial_getc():
  the current code assumed that tstc() is alway called but
  it is dangerous
- error for getc() (read value = 0x0) is not handled in cread_line()
- in console.c, tstc() is not called when CONSOLE_MUX is not activated

In this patchset I try to solve all these issues by separate patch
but perhaps only the first correction on the rx buffer is really mandatory.

On my board stm32mp1 ev1 the issue is solved.



Patrick Delaunay (4):
  stm32mp1: activate serial rx buffer
  serial: protect access to serial rx buffer
  console: unify fgetc function when console MUX is deactivated
  cli: handle getch error

 common/cli_readline.c             | 4 ++++
 common/console.c                  | 9 +++++----
 configs/stm32mp15_basic_defconfig | 1 +
 drivers/serial/serial-uclass.c    | 3 +++
 4 files changed, 13 insertions(+), 4 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer
  2018-08-03 11:38 [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated Patrick Delaunay
@ 2018-08-03 11:38 ` Patrick Delaunay
  2018-09-11 12:24   ` [U-Boot] [U-Boot,1/4] " Tom Rini
  2018-08-03 11:38 ` [U-Boot] [PATCH 2/4] serial: protect access to " Patrick Delaunay
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Patrick Delaunay @ 2018-08-03 11:38 UTC (permalink / raw)
  To: u-boot

Activate the serial rx buffer.
Prepare console MUX activation with vidconsole, and avoid console
performance issue (missing character for copy-paste).

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 configs/stm32mp15_basic_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig
index c72a440..9792e49 100644
--- a/configs/stm32mp15_basic_defconfig
+++ b/configs/stm32mp15_basic_defconfig
@@ -41,4 +41,5 @@ CONFIG_DM_REGULATOR_FIXED=y
 CONFIG_DM_REGULATOR_GPIO=y
 CONFIG_DM_REGULATOR_STM32_VREFBUF=y
 CONFIG_DM_REGULATOR_STPMU1=y
+CONFIG_SERIAL_RX_BUFFER=y
 CONFIG_STM32_SERIAL=y
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-08-03 11:38 [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated Patrick Delaunay
  2018-08-03 11:38 ` [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer Patrick Delaunay
@ 2018-08-03 11:38 ` Patrick Delaunay
  2018-08-08  9:56   ` Simon Glass
  2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
  2018-08-03 11:38 ` [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated Patrick Delaunay
  2018-08-03 11:38 ` [U-Boot] [PATCH 4/4] cli: handle getch error Patrick Delaunay
  3 siblings, 2 replies; 17+ messages in thread
From: Patrick Delaunay @ 2018-08-03 11:38 UTC (permalink / raw)
  To: u-boot

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.delaunay@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

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated
  2018-08-03 11:38 [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated Patrick Delaunay
  2018-08-03 11:38 ` [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer Patrick Delaunay
  2018-08-03 11:38 ` [U-Boot] [PATCH 2/4] serial: protect access to " Patrick Delaunay
@ 2018-08-03 11:38 ` Patrick Delaunay
  2018-08-08  9:55   ` Simon Glass
  2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
  2018-08-03 11:38 ` [U-Boot] [PATCH 4/4] cli: handle getch error Patrick Delaunay
  3 siblings, 2 replies; 17+ messages in thread
From: Patrick Delaunay @ 2018-08-03 11:38 UTC (permalink / raw)
  To: u-boot

Unify the fgetc function when MUX is activated or not:
- always call tstc() : it is the normal behavior expected
  by serial uclass (call tstc then getc) and that avoids
  issue when SERIAL_RX_BUFFER is activated
- reload WATCHDOG in the char waiting loop

This patch allow to have the same behavior when CONSOLE_MUX is activated
or not and avoid regression when this feature is deactivated.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 common/console.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/common/console.c b/common/console.c
index 7aa58d0..9a94f32 100644
--- a/common/console.c
+++ b/common/console.c
@@ -311,12 +311,12 @@ int serial_printf(const char *fmt, ...)
 int fgetc(int file)
 {
 	if (file < MAX_FILES) {
-#if CONFIG_IS_ENABLED(CONSOLE_MUX)
 		/*
 		 * Effectively poll for input wherever it may be available.
 		 */
 		for (;;) {
 			WATCHDOG_RESET();
+#if CONFIG_IS_ENABLED(CONSOLE_MUX)
 			/*
 			 * Upper layer may have already called tstc() so
 			 * check for that first.
@@ -324,6 +324,10 @@ int fgetc(int file)
 			if (tstcdev != NULL)
 				return console_getc(file);
 			console_tstc(file);
+#else
+			if (console_tstc(file))
+				return console_getc(file);
+#endif
 #ifdef CONFIG_WATCHDOG
 			/*
 			 * If the watchdog must be rate-limited then it should
@@ -332,9 +336,6 @@ int fgetc(int file)
 			 udelay(1);
 #endif
 		}
-#else
-		return console_getc(file);
-#endif
 	}
 
 	return -1;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 4/4] cli: handle getch error
  2018-08-03 11:38 [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated Patrick Delaunay
                   ` (2 preceding siblings ...)
  2018-08-03 11:38 ` [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated Patrick Delaunay
@ 2018-08-03 11:38 ` Patrick Delaunay
  2018-09-11 12:24   ` [U-Boot] [U-Boot,4/4] " Tom Rini
  3 siblings, 1 reply; 17+ messages in thread
From: Patrick Delaunay @ 2018-08-03 11:38 UTC (permalink / raw)
  To: u-boot

Handle getch error (when getch return 0x0) to avoid display issue
in the console.

Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
---

 common/cli_readline.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/common/cli_readline.c b/common/cli_readline.c
index 60a232b..99b6317 100644
--- a/common/cli_readline.c
+++ b/common/cli_readline.c
@@ -273,6 +273,10 @@ static int cread_line(const char *const prompt, char *buf, unsigned int *len,
 
 		ichar = getcmd_getch();
 
+		/* ichar=0x0 when error occurs in U-Boot getc */
+		if (!ichar)
+			continue;
+
 		if ((ichar == '\n') || (ichar == '\r')) {
 			putc('\n');
 			break;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated
  2018-08-03 11:38 ` [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated Patrick Delaunay
@ 2018-08-08  9:55   ` Simon Glass
  2018-09-03 14:56     ` Patrick DELAUNAY
  2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
  1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2018-08-08  9:55 UTC (permalink / raw)
  To: u-boot

On 3 August 2018 at 05:38, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> Unify the fgetc function when MUX is activated or not:
> - always call tstc() : it is the normal behavior expected
>   by serial uclass (call tstc then getc) and that avoids
>   issue when SERIAL_RX_BUFFER is activated
> - reload WATCHDOG in the char waiting loop
>
> This patch allow to have the same behavior when CONSOLE_MUX is activated
> or not and avoid regression when this feature is deactivated.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> ---
>
>  common/console.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

I'm not sure how to test this with the various cases...

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-08-03 11:38 ` [U-Boot] [PATCH 2/4] serial: protect access to " Patrick Delaunay
@ 2018-08-08  9:56   ` Simon Glass
  2018-09-03 13:35     ` Patrick DELAUNAY
  2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
  1 sibling, 1 reply; 17+ messages in thread
From: Simon Glass @ 2018-08-08  9:56 UTC (permalink / raw)
  To: u-boot

On 3 August 2018 at 05:38, Patrick Delaunay <patrick.delaunay@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.delaunay@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 <sjg@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,

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-08-08  9:56   ` Simon Glass
@ 2018-09-03 13:35     ` Patrick DELAUNAY
  2018-09-03 14:03       ` Stefan
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick DELAUNAY @ 2018-09-03 13:35 UTC (permalink / raw)
  To: u-boot

Hi Simon and Stefan,

Sorry for my late answer (I just come back from my summer holiday)

> From: sjg at google.com <sjg@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.delaunay@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.delaunay@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 <sjg@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-09-03 13:35     ` Patrick DELAUNAY
@ 2018-09-03 14:03       ` Stefan
  2018-09-04  7:56         ` Patrick DELAUNAY
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan @ 2018-09-03 14:03 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On 03.09.2018 15:35, Patrick DELAUNAY wrote:
> Hi Simon and Stefan,
> 
> Sorry for my late answer (I just come back from my summer holiday)
> 
>> From: sjg at google.com <sjg@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.delaunay@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.delaunay@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 <sjg@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....

I really hope so. I did test this implementation quite heavily at
that time.

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

I'm not 100% sure, if this new implementation is "better". Lets
compare the code:

Current version:
static int _serial_tstc(struct udevice *dev)
{
	struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);

	/* 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;
	}

	return upriv->rd_ptr != upriv->wr_ptr ? 1 : 0;
}

New version:
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;
}

The new version has more code and will most likely produce
a larger binary. You are removing the calls to the __foo()
functions - making the call chain a bit smaller. But this
will only affect the performance which is most likely negligible
in this case.

I any case, I don't object against any "optimizations" here.
But please make sure to test any changes very thorough - please
also on platforms with limited RX FIFO sizes.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated
  2018-08-08  9:55   ` Simon Glass
@ 2018-09-03 14:56     ` Patrick DELAUNAY
  0 siblings, 0 replies; 17+ messages in thread
From: Patrick DELAUNAY @ 2018-09-03 14:56 UTC (permalink / raw)
  To: u-boot

Hi Simon,

> From: sjg at google.com <sjg@google.com> On Behalf Of Simon Glass
> Sent: mercredi 8 août 2018 11:56
> To: Patrick DELAUNAY <patrick.delaunay@st.com>
> On 3 August 2018 at 05:38, Patrick Delaunay <patrick.delaunay@st.com> wrote:
> > Unify the fgetc function when MUX is activated or not:
> > - always call tstc() : it is the normal behavior expected
> >   by serial uclass (call tstc then getc) and that avoids
> >   issue when SERIAL_RX_BUFFER is activated
> > - reload WATCHDOG in the char waiting loop
> >
> > This patch allow to have the same behavior when CONSOLE_MUX is
> > activated or not and avoid regression when this feature is deactivated.
> >
> > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> > ---
> >
> >  common/console.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I'm not sure how to test this with the various cases...

Yes it is a difficully and I don't know the process in this case,
but at least the behavior (testc() function call) will be shared.

If this change seens too risky, it can be dropped as only the PATCH 1/4 is mandatory to solve my issue.

Regards, Patrick

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-09-03 14:03       ` Stefan
@ 2018-09-04  7:56         ` Patrick DELAUNAY
  2018-09-04 12:07           ` Stefan
  0 siblings, 1 reply; 17+ messages in thread
From: Patrick DELAUNAY @ 2018-09-04  7:56 UTC (permalink / raw)
  To: u-boot

Hi Stefan

> From: Stefan <sr@denx.de>
> Sent: lundi 3 septembre 2018 16:03
> 
> Hi Patrick,
> 
> On 03.09.2018 15:35, Patrick DELAUNAY wrote:
> > Hi Simon and Stefan,
> >
> > Sorry for my late answer (I just come back from my summer holiday)
> >
> >> From: sjg at google.com <sjg@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.delaunay@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.delaunay@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 <sjg@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....
> 
> I really hope so. I did test this implementation quite heavily at that time.
> 
> > 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.
> 
> I'm not 100% sure, if this new implementation is "better". Lets compare the
> code:
> 
> Current version:
> static int _serial_tstc(struct udevice *dev) {
> 	struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
> 
> 	/* 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;
> 	}
> 
> 	return upriv->rd_ptr != upriv->wr_ptr ? 1 : 0; }
> 
> New version:
> 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; }
> 
> The new version has more code and will most likely produce a larger binary. You
> are removing the calls to the __foo() functions - making the call chain a bit
> smaller. But this will only affect the performance which is most likely negligible
> in this case.

Yes, perhaps a larger code but no more  "pending"  ops call of serial driver.
I only  directly use getc ops => that avoid one access to HW register I think.

Than can improve the execution time, but I agree that seems marginal in most the case.

> I any case, I don't object against any "optimizations" here.
> But please make sure to test any changes very thorough - please also on
> platforms with limited RX FIFO sizes.

Unfortunately I have no platform with limited FIFO size, So I don't known how test it deeply.

And the impact depends also how is implemented the serial driver (gets and pending ops can use several HW register access and is depending on the access time to the IP).

But if you and Simon think that "optimization" is not needed, you can forget this proposal because I think also the gain should be very low.
This proposal is only a reaction on the Simon comment (at least sub-optimal)

> Thanks,
> Stefan

Regards
Patrick 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-09-04  7:56         ` Patrick DELAUNAY
@ 2018-09-04 12:07           ` Stefan
  2018-09-06  8:09             ` Patrick DELAUNAY
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan @ 2018-09-04 12:07 UTC (permalink / raw)
  To: u-boot

Hi Patrick,

On 04.09.2018 09:56, Patrick DELAUNAY wrote:
> Hi Stefan
> 
>> From: Stefan <sr@denx.de>
>> Sent: lundi 3 septembre 2018 16:03
>>
>> Hi Patrick,
>>
>> On 03.09.2018 15:35, Patrick DELAUNAY wrote:
>>> Hi Simon and Stefan,
>>>
>>> Sorry for my late answer (I just come back from my summer holiday)
>>>
>>>> From: sjg at google.com <sjg@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.delaunay@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.delaunay@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 <sjg@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....
>>
>> I really hope so. I did test this implementation quite heavily at that time.
>>
>>> 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.
>>
>> I'm not 100% sure, if this new implementation is "better". Lets compare the
>> code:
>>
>> Current version:
>> static int _serial_tstc(struct udevice *dev) {
>> 	struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
>>
>> 	/* 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;
>> 	}
>>
>> 	return upriv->rd_ptr != upriv->wr_ptr ? 1 : 0; }
>>
>> New version:
>> 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; }
>>
>> The new version has more code and will most likely produce a larger binary. You
>> are removing the calls to the __foo() functions - making the call chain a bit
>> smaller. But this will only affect the performance which is most likely negligible
>> in this case.
> 
> Yes, perhaps a larger code but no more  "pending"  ops call of serial driver.
> I only  directly use getc ops => that avoid one access to HW register I think.
> 
> Than can improve the execution time, but I agree that seems marginal in most the case.
> 
>> I any case, I don't object against any "optimizations" here.
>> But please make sure to test any changes very thorough - please also on
>> platforms with limited RX FIFO sizes.
> 
> Unfortunately I have no platform with limited FIFO size, So I don't known how test it deeply.
> 
> And the impact depends also how is implemented the serial driver (gets and pending ops can use several HW register access and is depending on the access time to the IP).
> 
> But if you and Simon think that "optimization" is not needed, you can forget this proposal because I think also the gain should be very low.
> This proposal is only a reaction on the Simon comment (at least sub-optimal)

I would prefer not to change this code without any real need
(bug fix etc). As the current code has undergone many tests
and has proven stable - at least for me in all my test cases.
And I can't test any changes very easily, as the platform setup
will take a while for me.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/4] serial: protect access to serial rx buffer
  2018-09-04 12:07           ` Stefan
@ 2018-09-06  8:09             ` Patrick DELAUNAY
  0 siblings, 0 replies; 17+ messages in thread
From: Patrick DELAUNAY @ 2018-09-06  8:09 UTC (permalink / raw)
  To: u-boot

Hi Stefan,

> From: Stefan <sr@denx.de>
> Sent: mardi 4 septembre 2018 14:08
> 
> Hi Patrick,
> 
> On 04.09.2018 09:56, Patrick DELAUNAY wrote:
> > Hi Stefan
> >
> >> From: Stefan <sr@denx.de>
> >> Sent: lundi 3 septembre 2018 16:03
> >>
> >> Hi Patrick,
> >>
> >> On 03.09.2018 15:35, Patrick DELAUNAY wrote:
> >>> Hi Simon and Stefan,
> >>>
> >>> Sorry for my late answer (I just come back from my summer holiday)
> >>>
> >>>> From: sjg at google.com <sjg@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.delaunay@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.delaunay@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 <sjg@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....
> >>
> >> I really hope so. I did test this implementation quite heavily at that time.
> >>
> >>> 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.
> >>
> >> I'm not 100% sure, if this new implementation is "better". Lets
> >> compare the
> >> code:
> >>
> >> Current version:
> >> static int _serial_tstc(struct udevice *dev) {
> >> 	struct serial_dev_priv *upriv = dev_get_uclass_priv(dev);
> >>
> >> 	/* 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;
> >> 	}
> >>
> >> 	return upriv->rd_ptr != upriv->wr_ptr ? 1 : 0; }
> >>
> >> New version:
> >> 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; }
> >>
> >> The new version has more code and will most likely produce a larger
> >> binary. You are removing the calls to the __foo() functions - making
> >> the call chain a bit smaller. But this will only affect the
> >> performance which is most likely negligible in this case.
> >
> > Yes, perhaps a larger code but no more  "pending"  ops call of serial driver.
> > I only  directly use getc ops => that avoid one access to HW register I think.
> >
> > Than can improve the execution time, but I agree that seems marginal in most
> the case.
> >
> >> I any case, I don't object against any "optimizations" here.
> >> But please make sure to test any changes very thorough - please also
> >> on platforms with limited RX FIFO sizes.
> >
> > Unfortunately I have no platform with limited FIFO size, So I don't known how
> test it deeply.
> >
> > And the impact depends also how is implemented the serial driver (gets and
> pending ops can use several HW register access and is depending on the access
> time to the IP).
> >
> > But if you and Simon think that "optimization" is not needed, you can forget
> this proposal because I think also the gain should be very low.
> > This proposal is only a reaction on the Simon comment (at least
> > sub-optimal)
> 
> I would prefer not to change this code without any real need (bug fix etc). As the
> current code has undergone many tests and has proven stable - at least for me
> in all my test cases.
> And I can't test any changes very easily, as the platform setup will take a while
> for me.

Ok thanks for the feedback, so I forget my "optimization" proposal.
But the fist patch to protect  the serial rx buffer access is needed 
(the initial patch reviewed by Simon).

> 
> Thanks,
> Stefan

Regards 
Patrick

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [U-Boot,1/4] stm32mp1: activate serial rx buffer
  2018-08-03 11:38 ` [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer Patrick Delaunay
@ 2018-09-11 12:24   ` Tom Rini
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-09-11 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 03, 2018 at 01:38:42PM +0200, Patrick Delaunay wrote:

> Activate the serial rx buffer.
> Prepare console MUX activation with vidconsole, and avoid console
> performance issue (missing character for copy-paste).
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180911/c759c703/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [U-Boot, 2/4] serial: protect access to serial rx buffer
  2018-08-03 11:38 ` [U-Boot] [PATCH 2/4] serial: protect access to " Patrick Delaunay
  2018-08-08  9:56   ` Simon Glass
@ 2018-09-11 12:24   ` Tom Rini
  1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-09-11 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 03, 2018 at 01:38:43PM +0200, Patrick Delaunay 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.delaunay@st.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180911/ec64cd2f/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [U-Boot, 3/4] console: unify fgetc function when console MUX is deactivated
  2018-08-03 11:38 ` [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated Patrick Delaunay
  2018-08-08  9:55   ` Simon Glass
@ 2018-09-11 12:24   ` Tom Rini
  1 sibling, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-09-11 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 03, 2018 at 01:38:44PM +0200, Patrick Delaunay wrote:

> Unify the fgetc function when MUX is activated or not:
> - always call tstc() : it is the normal behavior expected
>   by serial uclass (call tstc then getc) and that avoids
>   issue when SERIAL_RX_BUFFER is activated
> - reload WATCHDOG in the char waiting loop
> 
> This patch allow to have the same behavior when CONSOLE_MUX is activated
> or not and avoid regression when this feature is deactivated.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180911/0e8cd078/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [U-Boot,4/4] cli: handle getch error
  2018-08-03 11:38 ` [U-Boot] [PATCH 4/4] cli: handle getch error Patrick Delaunay
@ 2018-09-11 12:24   ` Tom Rini
  0 siblings, 0 replies; 17+ messages in thread
From: Tom Rini @ 2018-09-11 12:24 UTC (permalink / raw)
  To: u-boot

On Fri, Aug 03, 2018 at 01:38:45PM +0200, Patrick Delaunay wrote:

> Handle getch error (when getch return 0x0) to avoid display issue
> in the console.
> 
> Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180911/107eb1bb/attachment.sig>

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2018-09-11 12:24 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-03 11:38 [U-Boot] [PATCH 0/4] Solve issue with serial rx buffer when MUX is deactivated Patrick Delaunay
2018-08-03 11:38 ` [U-Boot] [PATCH 1/4] stm32mp1: activate serial rx buffer Patrick Delaunay
2018-09-11 12:24   ` [U-Boot] [U-Boot,1/4] " Tom Rini
2018-08-03 11:38 ` [U-Boot] [PATCH 2/4] serial: protect access to " Patrick Delaunay
2018-08-08  9:56   ` Simon Glass
2018-09-03 13:35     ` Patrick DELAUNAY
2018-09-03 14:03       ` Stefan
2018-09-04  7:56         ` Patrick DELAUNAY
2018-09-04 12:07           ` Stefan
2018-09-06  8:09             ` Patrick DELAUNAY
2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
2018-08-03 11:38 ` [U-Boot] [PATCH 3/4] console: unify fgetc function when console MUX is deactivated Patrick Delaunay
2018-08-08  9:55   ` Simon Glass
2018-09-03 14:56     ` Patrick DELAUNAY
2018-09-11 12:24   ` [U-Boot] [U-Boot, " Tom Rini
2018-08-03 11:38 ` [U-Boot] [PATCH 4/4] cli: handle getch error Patrick Delaunay
2018-09-11 12:24   ` [U-Boot] [U-Boot,4/4] " Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.