[Uboot-stm32] [PATCH 4/4] console: add console_tstc_check helper function for CONSOLE_MUX

[Patrick DELAUNAY]

Hi Simon,

On 12/12/20 4:39 PM, Simon Glass wrote:
> Hi Patrick,
>
> On Thu, 3 Dec 2020 at 02:20, Patrick Delaunay <patrick.delaunay at st.com> wrote:
>> Add the helper function console_tstc_check() and replace the test
>> #if CONFIG_IS_ENABLED(CONSOLE_MUX) to a simple if to respect the
>> U-Boot coding rule.
>>
>> No functional change.
>>
>> Signed-off-by: Patrick Delaunay <patrick.delaunay at st.com>
>> ---
>>
>>   common/console.c | 36 ++++++++++++++++++++++++------------
>>   1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/common/console.c b/common/console.c
>> index 0b864444bb..c570cd88cf 100644
>> --- a/common/console.c
>> +++ b/common/console.c
>> @@ -248,6 +248,12 @@ static int console_getc(int file)
>>          return ret;
>>   }
>>
>> +/*  Upper layer may have already called tstc(): check the saved result */
>> +static bool console_tstc_check(void)
> This is checking if we actually have a device, right? I think
> has_tstcdev() would be better.

the variable tstcdev save the last stio device with indicated a pending 
character

(last stdio dev wich has result of tstc() != 0)

- in function console_tstc() called by tstc() API -

This device will be used for the next getc() request (then it reset also 
the tstcdev value).

So it is a "tstc" device but used "getc".

But OK to rename it to "console_has_tstcdev()" because for me

it is only a first cleanup step (see after).

>
> Also could you add a comment in the testcdev variable declaration as
> to what this variable actually is for?
>
OK


For information, I prepared a other patch, to be apply after this serie, 
to simply the fgetc() implementation

and have a common behavior with or without CONFIG_CONSOLE_MUX

This function "console_tstc_check()" will disappear, integrated in console_tstc(),
and the final result is more simple

int fgetc(int file)
{
	if (file < MAX_FILES) {
		/*
		 * Effectively poll for input wherever it may be available.
		 */
		for (;;) {
			WATCHDOG_RESET();
			if (console_tstc(file))
				return console_getc(file);
			/*
			 * If the watchdog must be rate-limited then it should
			 * already be handled in board-specific code.
			 */
			if (IS_ENABLED(CONFIG_WATCHDOG))
				udelay(1);
		}
	}
	return -1;
}

>> +{
>> +       return !!tstcdev;
>> +}
>> +
>>   static int console_tstc(int file)
>>   {
>>          int i, ret;
>> @@ -340,6 +346,11 @@ static inline int console_getc(int file)
>>          return stdio_devices[file]->getc(stdio_devices[file]);
>>   }
>>
>> +static bool console_tstc_check(void)
>> +{
>> +       return false;
>> +}
>> +
>> (...)


Regards

Patrick