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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



09.07.2021 20:55, Michał Mirosław пишет:
> On Fri, Jul 09, 2021 at 12:38:07PM +0100, Jon Hunter wrote:
>>
>> On 09/07/2021 09:34, Jon Hunter wrote:
>>>
>>>
>>> On 08/07/2021 23:25, Michał Mirosław wrote:
>>>> On Wed, Jun 30, 2021 at 01:56:43PM +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 displaying the error message, only when an error occurs.
>>>>>
>>>>> Finally, update the error message to make it clear that enabling the
>>>>> FIFO failed and display the error code.
>>>> [...]
>>>>> @@ -1045,9 +1045,11 @@ 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,
>>>>> +				"Failed to enable FIFO mode: %d\n", ret);
>>>>
>>>> Could you change this to use %pe and ERR_PTR(ret)?
>>>
>>> Sorry, but it is not clear to me why this would be necessary in this case.
>>
>> I see so '%pe' prints the symbolic name of the error code. While that is
>> nice, it also looks a bit odd. Given that we simply print the error code
>> values in this driver, from looking at other prints, I prefer to keep it
>> as is for consistency.
> 
> It is a quite new facility of printk(), so I woudn't expect it to be
> present in older code. It saves a bit of time when you occasionally
> hit an error, so even incremental conversion seems beneficial for me.

It doesn't feel like a good approach to use ERR_PTR where it's not very
appropriate. I suppose printk could get a new specifier, like '%de' for
example, for the verbose integer error codes, couldn't it?



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux