Re: [PATCH] serial: tegra: Only print FIFO error message when an error occurs

[Jon Hunter <jonathanh@nvidia.com>]

On 30/06/2021 13:11, Thierry Reding wrote:
> On Wed, Jun 30, 2021 at 10:46:01AM +0100, Jon Hunter wrote:
>> The Tegra serial driver always prints an error message when enabling the
>> FIFO for devices that have support for checking the FIFO enable status.
>> Fix this by only display the error message, when an error occurs.
>>
>> Fixes: 222dcdff3405 ("serial: tegra: check for FIFO mode enabled status")
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
>> ---
>>  drivers/tty/serial/serial-tegra.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial-tegra.c b/drivers/tty/serial/serial-tegra.c
>> index 222032792d6c..cd481f7ba8eb 100644
>> --- a/drivers/tty/serial/serial-tegra.c
>> +++ b/drivers/tty/serial/serial-tegra.c
>> @@ -1045,9 +1045,10 @@ static int tegra_uart_hw_init(struct tegra_uart_port *tup)
>>  
>>  	if (tup->cdata->fifo_mode_enable_status) {
>>  		ret = tegra_uart_wait_fifo_mode_enabled(tup);
>> -		dev_err(tup->uport.dev, "FIFO mode not enabled\n");
>> -		if (ret < 0)
>> +		if (ret < 0) {
>> +			dev_err(tup->uport.dev, "FIFO mode not enabled\n");
> 
> The error message seems a bit confusing. I read this as meaning "FIFO
> mode was expected to be enabled but wasn't" whereas this really seems to
> mean that for some reason the FIFO enable timed out.
> 
> In the former case it sounds like a some configuration mismatch, while
> it's really something that went wrong during the process of enabling the
> FIFO mode.
> 
> So I wonder if this should perhaps be something like:
> 
> 	dev_err(tup->uport.dev, "FIFO mode enable timed out\n");
> 
> or something along those lines.

Yes good point. Maybe I should just ...

	dev_err(tup->uport.dev,
		"Failed to enable FIFO mode: %d\n", ret);

Then if it is updated to ever return anything other that -ETIMEDOUT we
are covered. I will send a V2.

Jon

-- 
nvpublic