Re: [PATCH 4/7] serial: ns16550: add compatible entry for "ingenic, jz4740-uart"

[Antony Pavlov <antonynpavlov@gmail.com>]

On Mon, 24 Mar 2014 07:45:31 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> On Sat, Mar 22, 2014 at 03:29:54PM +0400, Antony Pavlov wrote:
> > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
> > ---
> >  drivers/serial/serial_ns16550.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
> > index aa0992f..7130871 100644
> > --- a/drivers/serial/serial_ns16550.c
> > +++ b/drivers/serial/serial_ns16550.c
> > @@ -257,6 +257,30 @@ static __maybe_unused struct ns16550_drvdata omap_drvdata = {
> >  	.linux_console_name = "ttyO",
> >  };
> >  
> > +#define JZ_UART_SHIFT	2
> > +
> > +static void jz_serial_reg_write(unsigned int val, unsigned long base,
> > +	unsigned char reg_offset)
> > +{
> > +	switch (reg_offset) {
> > +	case (fcr << JZ_UART_SHIFT):
> > +		val |= 0x10; /* Enable uart module */
> > +		break;
> 
> You could also do this by overwriting .init_port.

I'll do so.
 
> > +	case (ier << JZ_UART_SHIFT):
> > +		val |= (val & 0x4) << 2;
> 
> What does this bit do?

It's non-standard Ingenic-specific 'Receive Timeout Interrupt Enable' bit.
We can simply drop it as we use 'ns16550_write(cdev, 0x00, ier);' for ier initialization.

> 
> Putting driver logic into the register writes is not good. Where this
> leads to can be seen in drivers/mmc/host/sdhci-esdhc-imx.c in the
> kernel. Could we make fcrval overwritable in drvdata? Or just special
> case the fcr register access in ns16550_setbaudrate?

Please see v2 patchseries.

-- 
Best regards,
  Antony Pavlov

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox