Re: [PATCH v5 3/3] serial: sc16is7xx: add support for EXAR XR20M1172 UART

[Andy Shevchenko]

On Sat, Apr 20, 2024 at 09:22:06PM +0300, Konstantin Pugin wrote:
> From: Konstantin Pugin <ria.freelander@xxxxxxxxx>
> 
> XR20M1172 register set is mostly compatible with SC16IS762, but it has
> a support for additional division rates of UART with special DLD register.
> So, add handling this register by appropriate devicetree bindings.

..

>  	help
> -	  Core driver for NXP SC16IS7xx UARTs.
> +	  Core driver for NXP SC16IS7xx-compatible UARTs.

'-compatible' --> ' and compatible'

>  	  Supported ICs are:
> -
> -	    SC16IS740
> -	    SC16IS741
> -	    SC16IS750
> -	    SC16IS752
> -	    SC16IS760
> -	    SC16IS762
> +		NXP:
> +	    	SC16IS740
> +	    	SC16IS741
> +	    	SC16IS750
> +	    	SC16IS752
> +	    	SC16IS760
> +	    	SC16IS762

You broke the indentation (as it has mixed TABs and spaces).

> +		EXAR:
> +			XR20M1172

No need to rewrite all of them, just add your line as

	    XR20M1172 (Exar)

>  	  The driver supports both I2C and SPI interfaces.

(Note, this needs to be fixed, hence it justifies a new version of the patch.)

..

> +/*
> + * Divisor Fractional Register bits (EXAR extension)

Missing period at the end of the line.

> + * EXAR hardware is mostly compatible with SC16IS7XX, but supports additional feature:
> + * 4x and 8x divisor, instead of default 16x. It has a special register to program it.
> + * Bits 0 to 3 is fractional divisor, it used to set value of last 16 bits of
> + * uartclk * (16 / divisor) / baud, in case of default it will be uartclk / baud.
> + * Bits 4 and 5 used as switches, and should not be set to 1 simultaneously.
> + */

..

> +static bool sc16is7xx_has_dld(struct device *dev)
> +{
> +		struct sc16is7xx_port *s = dev_get_drvdata(dev);
> +
> +		if (s->devtype == &xr20m1172_devtype)
> +			return true;
> +		return false;

Besides what Jiri noticed, this has been indented one TAB too much.

> +}

-- 
With Best Regards,
Andy Shevchenko