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

["Michał Mirosław" <mirq-linux@rere.qmqm.pl>]

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.

Best Regards
Michał Mirosław