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

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Thu, Apr 18, 2024 at 08:06:07PM +0300, Konstantin Pugin wrote:
> From: Konstantin Pugin <ria.freelander@gmail.com>
> 
> 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.

...

>  /* Special Register set: Only if ((LCR[7] == 1) && (LCR != 0xBF)) */
>  #define SC16IS7XX_DLL_REG		(0x00) /* Divisor Latch Low */
>  #define SC16IS7XX_DLH_REG		(0x01) /* Divisor Latch High */
> +#define XR20M117X_DLD_REG		(0x02) /* Divisor Fractional Register (only on EXAR chips) */

The comment in the parentheses is not needed anymore as it's implied by
the namespace.

...

> +#define XR20M117X_DLD_16X		0
> +#define XR20M117X_DLD_DIV(m)	((m) & GENMASK(3, 0))

Seems like one too little TABs in between.

> +#define XR20M117X_DLD_8X		BIT(4)
> +#define XR20M117X_DLD_4X		BIT(5)

...

>  	char	name[10];
>  	int	nr_gpio;
>  	int	nr_uart;

> +	bool has_dld;

Not needed. See below.

...

> +	bool has_dld = s->devtype->has_dld;

So, you can check against devtype itself:

	s->devtype == &xr20m1172_devtype;

> +static const struct sc16is7xx_devtype = {

...

> +	if (has_dld && DIV_ROUND_CLOSEST(clk, baud) < 16)
> +		divisor = 1 << (fls(DIV_ROUND_CLOSEST(clk, baud)) - 1);

BIT() ?

-- 
With Best Regards,
Andy Shevchenko