Re: [PATCH v3 05/10] serial: 8250: dw: Allow to use a fallback CPR value if not synthesized

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Miquel Raynal <miquel.raynal@bootlin.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Jiri Slaby <jirislaby@kernel.org>,
	linux-renesas-soc@vger.kernel.org, linux-serial@vger.kernel.org,
	Milan Stevanovic <milan.stevanovic@se.com>,
	Jimmy Lalande <jimmy.lalande@se.com>,
	Pascal Eberhard <pascal.eberhard@se.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Herve Codina <herve.codina@bootlin.com>,
	Clement Leger <clement.leger@bootlin.com>,
	Phil Edworthy <phil.edworthy@renesas.com>
Subject: Re: [PATCH v3 05/10] serial: 8250: dw: Allow to use a fallback CPR value if not synthesized
Date: Tue, 29 Mar 2022 18:49:52 +0300	[thread overview]
Message-ID: <YkMqoGIwEBVKtdXG@smile.fi.intel.com> (raw)
In-Reply-To: <20220329152430.756947-6-miquel.raynal@bootlin.com>

On Tue, Mar 29, 2022 at 05:24:25PM +0200, Miquel Raynal wrote:
> DW UART controllers can be synthesized without the CPR register.
> In this case, allow to the platform information to provide a CPR value.

> +	const struct dw8250_platform_data *pdata = device_get_match_data(p->dev);

Hmm... So, we will have two functions that do the same.
Perhaps you may store somewhere the pdata pointer (or contents) once
and call these functions at the time that the pdata is known to be set.
That's how usual pattern looks like. Calling device_get_match_data()
several times is unusual and potentially might be error prone.

...

>  	reg = dw8250_readl_ext(p, DW_UART_CPR);
> +	if (!reg) {

> +		if (pdata)
> +			reg = pdata->cpr;

If you store the content this becomes something like:

		reg = data->pdata.cpr;

But, please experiment a bit with it and choose the best option.

> +		dev_dbg(p->dev, "CPR is not available, using %x instead\n", reg);
> +	}

> +

No need of this blank line.

>  	if (!reg)
>  		return;

-- 
With Best Regards,
Andy Shevchenko