On Thu, Dec 10, 2015 at 11:31:04AM +0200, Heikki Krogerus wrote:
> Hi Noam,
> 
> On Thu, Dec 10, 2015 at 07:26:42AM +0000, Noam Camus wrote:
> > >From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com] 
> > >Sent: Wednesday, December 09, 2015 3:20 PM
> > 
> > 
> > >> @@ -171,7 +174,8 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value)
> > >>  			if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> > >>  				return;
> > >>  			dw8250_force_idle(p);
> > >> -			writel(value, p->membase + (UART_LCR << p->regshift));
> > >> +			d->serial_out(value,
> > >> +				      p->membase + (UART_LCR << p->regshift));
> > >>  		}
> > 
> > >.. The way I see it, this is the only place where you would really need the
> > >new private serial_in/out hooks, so why not just check the >iotype here and
> > >based on that use writeb/writel/iowrite32be or what ever ..
> > 
> > In previous versions (V2) Greg dis-liked using iotype here this is why I added
> > the private serial_in/serial_out
> 
> I couldn't find that comment in the thread? All he said in his
> comment for this was that you should use real if condition instead of
> the ternary operator you had there (condition ? true : false).
> 
> Why would it not be acceptable? If it would really not be acceptable
> (which I don't believe) then you should simply duplicate the lcr
> checking to dw8250_seria_out32be like it is done now in
> dw8250_serial_out and dw8250_serial_out32, but there still is no need
> for the private serial_in/out hooks.
> 
> I'm attaching a diff that I think would work as a good starting point
> for you.

Now actually attaching the diff :).

-- 
heikki