Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933459AbbLTQpn (ORCPT ); Sun, 20 Dec 2015 11:45:43 -0500 Received: from mail-yk0-f182.google.com ([209.85.160.182]:34015 "EHLO mail-yk0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932305AbbLTQpk (ORCPT ); Sun, 20 Dec 2015 11:45:40 -0500 MIME-Version: 1.0 In-Reply-To: <1449940707-17633-2-git-send-email-noamc@ezchip.com> References: <1449940707-17633-1-git-send-email-noamc@ezchip.com> <1449940707-17633-2-git-send-email-noamc@ezchip.com> Date: Sun, 20 Dec 2015 18:45:39 +0200 Message-ID: Subject: Re: [PATCH v9 1/3] serial: 8250_dw: Avoid serial_outx code duplicate with new dw8250_check_lcr() From: Andy Shevchenko To: Noam Camus Cc: "linux-serial@vger.kernel.org" , "linux-kernel@vger.kernel.org" , Greg Kroah-Hartman , "Krogerus, Heikki" , Andy Shevchenko , vgupta@synopsys.com, Alexey Brodkin , Jiri Slaby , Peter Hurley Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6687 Lines: 173 On Sat, Dec 12, 2015 at 7:18 PM, Noam Camus wrote: > From: Noam Camus > > With the help of Heikki we take common code that > makes sure LCR write wasn't ignored and put it in new function called > dw8250_check_lcr(). This function serves 3 serial_out routines: > dw8250_serial_out(), dw8250_serial_out32(), and dw8250_serial_outq(). > > This patch only brings better code reuse. > > Signed-off-by: Noam Camus > Cc: Heikki Krogerus > Cc: Andy Shevchenko All three fine by me: Acked-by: Andy Shevchenko > --- > drivers/tty/serial/8250/8250_dw.c | 91 +++++++++++++++++-------------------- > 1 files changed, 42 insertions(+), 49 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c > index a5d319e..ffe5e0c 100644 > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -95,25 +95,43 @@ static void dw8250_force_idle(struct uart_port *p) > (void)p->serial_in(p, UART_RX); > } > > -static void dw8250_serial_out(struct uart_port *p, int offset, int value) > +static void dw8250_check_lcr(struct uart_port *p, int value) > { > - writeb(value, p->membase + (offset << p->regshift)); > + void __iomem *offset = p->membase + (UART_LCR << p->regshift); > + int tries = 1000; > > /* Make sure LCR write wasn't ignored */ > - if (offset == UART_LCR) { > - int tries = 1000; > - while (tries--) { > - unsigned int lcr = p->serial_in(p, UART_LCR); > - if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) > - return; > - dw8250_force_idle(p); > - writeb(value, p->membase + (UART_LCR << p->regshift)); > - } > - /* > - * FIXME: this deadlocks if port->lock is already held > - * dev_err(p->dev, "Couldn't set LCR to %d\n", value); > - */ > + while (tries--) { > + unsigned int lcr = p->serial_in(p, UART_LCR); > + > + if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) > + return; > + > + dw8250_force_idle(p); > + > +#ifdef CONFIG_64BIT > + __raw_writeq(value & 0xff, offset); > +#else > + if (p->iotype == UPIO_MEM32) > + writel(value, offset); > + else > + writeb(value, offset); > +#endif > } > + /* > + * FIXME: this deadlocks if port->lock is already held > + * dev_err(p->dev, "Couldn't set LCR to %d\n", value); > + */ > +} > + > +static void dw8250_serial_out(struct uart_port *p, int offset, int value) > +{ > + struct dw8250_data *d = p->private_data; > + > + writeb(value, p->membase + (offset << p->regshift)); > + > + if (offset == UART_LCR && !d->uart_16550_compatible) > + dw8250_check_lcr(p, value); > } > > static unsigned int dw8250_serial_in(struct uart_port *p, int offset) > @@ -135,49 +153,26 @@ static unsigned int dw8250_serial_inq(struct uart_port *p, int offset) > > static void dw8250_serial_outq(struct uart_port *p, int offset, int value) > { > + struct dw8250_data *d = p->private_data; > + > value &= 0xff; > __raw_writeq(value, p->membase + (offset << p->regshift)); > /* Read back to ensure register write ordering. */ > __raw_readq(p->membase + (UART_LCR << p->regshift)); > > - /* Make sure LCR write wasn't ignored */ > - if (offset == UART_LCR) { > - int tries = 1000; > - while (tries--) { > - unsigned int lcr = p->serial_in(p, UART_LCR); > - if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) > - return; > - dw8250_force_idle(p); > - __raw_writeq(value & 0xff, > - p->membase + (UART_LCR << p->regshift)); > - } > - /* > - * FIXME: this deadlocks if port->lock is already held > - * dev_err(p->dev, "Couldn't set LCR to %d\n", value); > - */ > - } > + if (offset == UART_LCR && !d->uart_16550_compatible) > + dw8250_check_lcr(p, value); > } > #endif /* CONFIG_64BIT */ > > static void dw8250_serial_out32(struct uart_port *p, int offset, int value) > { > + struct dw8250_data *d = p->private_data; > + > writel(value, p->membase + (offset << p->regshift)); > > - /* Make sure LCR write wasn't ignored */ > - if (offset == UART_LCR) { > - int tries = 1000; > - while (tries--) { > - unsigned int lcr = p->serial_in(p, UART_LCR); > - if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) > - return; > - dw8250_force_idle(p); > - writel(value, p->membase + (UART_LCR << p->regshift)); > - } > - /* > - * FIXME: this deadlocks if port->lock is already held > - * dev_err(p->dev, "Couldn't set LCR to %d\n", value); > - */ > - } > + if (offset == UART_LCR && !d->uart_16550_compatible) > + dw8250_check_lcr(p, value); > } > > static unsigned int dw8250_serial_in32(struct uart_port *p, int offset) > @@ -463,10 +458,8 @@ static int dw8250_probe(struct platform_device *pdev) > dw8250_quirks(p, data); > > /* If the Busy Functionality is not implemented, don't handle it */ > - if (data->uart_16550_compatible) { > - p->serial_out = NULL; > + if (data->uart_16550_compatible) > p->handle_irq = NULL; > - } > > if (!data->skip_autocfg) > dw8250_setup_port(p); > -- > 1.7.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/