Re: [PATCH v1] serial: 8250_fintek: Print Fintek chip name

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Dec 14, 2020 at 02:14:45PM +0100, Flavio Suligoi wrote:
> At the moment, if a Fintek UART is detected, there is no
> printed information about this.
> The ttyS port is declared as a simple 16550A port, but,
> especially when we want to use the RS485 mode, it's
> very important understand if the Fintek UART is correctly
> detected as expected.
> 
> Signed-off-by: Flavio Suligoi <f.suligoi@asem.it>
> ---
>  drivers/tty/serial/8250/8250_fintek.c | 25 +++++++++++++++++++++++--
>  1 file changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_fintek.c b/drivers/tty/serial/8250/8250_fintek.c
> index 31c9e83ea3cb..ef2303cb5176 100644
> --- a/drivers/tty/serial/8250/8250_fintek.c
> +++ b/drivers/tty/serial/8250/8250_fintek.c
> @@ -97,6 +97,7 @@ struct fintek_8250 {
>  	u16 base_port;
>  	u8 index;
>  	u8 key;
> +	const char *chip_name;
>  };
>  
>  static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
> @@ -140,9 +141,11 @@ static void fintek_8250_exit_key(u16 base_port)
>  	release_region(base_port + ADDR_PORT, 2);
>  }
>  
> -static int fintek_8250_check_id(struct fintek_8250 *pdata)
> +static int fintek_8250_check_id(struct fintek_8250 *pdata,
> +				struct uart_8250_port *uart)
>  {
>  	u16 chip;
> +	const char *chip_name;
>  
>  	if (sio_read_reg(pdata, VENDOR_ID1) != VENDOR_ID1_VAL)
>  		return -ENODEV;
> @@ -155,17 +158,35 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
>  
>  	switch (chip) {
>  	case CHIP_ID_F81865:
> +		chip_name = "F81865";
> +		break;
>  	case CHIP_ID_F81866:
> +		chip_name = "F81866";
> +		break;
>  	case CHIP_ID_F81966:
> +		chip_name = "F81966";
> +		break;
>  	case CHIP_ID_F81216AD:
> +		chip_name = "F81216AD";
> +		break;
>  	case CHIP_ID_F81216H:
> +		chip_name = "F81216H";
> +		break;
>  	case CHIP_ID_F81216:
> +		chip_name = "F81216";
>  		break;
>  	default:
>  		return -ENODEV;
>  	}
>  
>  	pdata->pid = chip;
> +
> +	pr_info("%s%s%s Fintek %s\n",
> +		uart->port.dev ? dev_name(uart->port.dev) : "",
> +		uart->port.dev ? ": " : "",
> +		uart->port.name,
> +		chip_name);

Drivers, if all goes well, should not print anything to the kernel log.
This isn't ok.

And even if it was, dev_info() would be the correct thing to do...

thanks,

greg k-h