[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20151210103338.GE4884@kuha.fi.intel.com>
Date: Thu, 10 Dec 2015 12:33:38 +0200
From: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To: Noam Camus <noamc@...hip.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"jslaby@...e.com" <jslaby@...e.com>,
"peter@...leysoftware.com" <peter@...leysoftware.com>,
"fransklaver@...il.com" <fransklaver@...il.com>,
"Alexey.Brodkin@...opsys.com" <Alexey.Brodkin@...opsys.com>,
"vgupta@...opsys.com" <vgupta@...opsys.com>
Subject: Re: [PATCH-v8] serial: 8250_dw: Add support for big-endian MMIO
accesses
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@...ux.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
View attachment "dw8250_check_lcr.diff" of type "text/plain" (2913 bytes)
Powered by blists - more mailing lists