On 03/11/2016 07:20 AM, Yegor Yefremov wrote: > On Fri, Mar 11, 2016 at 3:53 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote: >> On 03/11/2016 06:46 AM, Yegor Yefremov wrote: >>> Hi Peter, >>> >>> On Fri, Mar 11, 2016 at 3:34 PM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote: >>>> Hi Yegor, >>>> >>>> On 03/11/2016 05:46 AM, yegorslists@xxxxxxxxxxxxxx wrote: >>>>> From: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx> >>>>> >>>>> Introduce serial8250_out_MCR() and serial8250_in_MCR() routines, that >>>>> replace following calls: >>>>> >>>>> serial_out(port, UART_MCR, val) >>>>> serial_port_out(up, UART_MCR, val) >>>>> serial_in(port, UART_MCR) >>>> >>>> I assume that this patch would be the first in a series that adds >>>> mctrl gpio support to the 8250 driver (specifically to omap). >>>> >>>> A couple of comments. >>>> 1) I think the commit log should note the purpose of the abstraction; >>>> ie, to enable gpio modem control lines. >>> >>> I thought, it can be sent as stand-alone patch, as the new routines >>> clean up the code and make the access consistent regardless of gpio >>> feature, but I'll integrate the patch into series. >>> >>> I still have to implement the requested changes for mctrl_gpio itself. >>> >>>> 2) You'll want to rebase on top of Greg's tty-next branch of his tty tree >>>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git >>> >>> Will do. >> >> I mention that specifically because there is new RS485 support in >> tty-next that flips RTS via MCR writes so you'll want to catch those >> too. > > You're right. Now I see additional calls to read/write MCR, that were > not covered through my patch. > > Was this new RS485 RTS switching feature already tested on OMAP UARTs? I certainly hope so. > Yegor > >>>>> CC: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> >>>>> Signed-off-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx> >>>>> --- >>>>> drivers/tty/serial/8250/8250.h | 10 ++++++++++ >>>>> drivers/tty/serial/8250/8250_omap.c | 4 ++-- >>>>> drivers/tty/serial/8250/8250_port.c | 39 ++++++++++++++++++------------------- >>>>> 3 files changed, 31 insertions(+), 22 deletions(-) >>>>> >>>>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h >>>>> index d54dcd8..5469bef 100644 >>>>> --- a/drivers/tty/serial/8250/8250.h >>>>> +++ b/drivers/tty/serial/8250/8250.h >>>>> @@ -118,6 +118,16 @@ struct uart_8250_port *serial8250_get_port(int line); >>>>> void serial8250_rpm_get(struct uart_8250_port *p); >>>>> void serial8250_rpm_put(struct uart_8250_port *p); >>>>> >>>>> +static inline void serial8250_out_MCR(struct uart_8250_port *up, int value) >>>>> +{ >>>>> + serial_out(up, UART_MCR, value); >>>>> +} >>>>> + >>>>> +static inline int serial8250_in_MCR(struct uart_8250_port *up) >>>>> +{ >>>>> + return serial_in(up, UART_MCR); >>>>> +} >>>>> + >>>>> #if defined(__alpha__) && !defined(CONFIG_PCI) >>>>> /* >>>>> * Digital did something really horribly wrong with the OUT1 and OUT2 >>>>> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c >>>>> index a2c0734..bb431e3 100644 >>>>> --- a/drivers/tty/serial/8250/8250_omap.c >>>>> +++ b/drivers/tty/serial/8250/8250_omap.c >>>>> @@ -274,7 +274,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) >>>>> serial_out(up, UART_EFR, UART_EFR_ECB); >>>>> >>>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A); >>>>> - serial_out(up, UART_MCR, UART_MCR_TCRTLR); >>>>> + serial8250_out_MCR(up, UART_MCR_TCRTLR); >>>>> serial_out(up, UART_FCR, up->fcr); >>>>> >>>>> omap8250_update_scr(up, priv); >>>>> @@ -290,7 +290,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up) >>>>> serial_out(up, UART_LCR, 0); >>>>> >>>>> /* drop TCR + TLR access, we setup XON/XOFF later */ >>>>> - serial_out(up, UART_MCR, up->mcr); >>>>> + serial8250_out_MCR(up, up->mcr); >>>>> serial_out(up, UART_IER, up->ier); >>>>> >>>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B); >>>>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c >>>>> index 8d262bc..ad87495c 100644 >>>>> --- a/drivers/tty/serial/8250/8250_port.c >>>>> +++ b/drivers/tty/serial/8250/8250_port.c >>>>> @@ -701,10 +701,10 @@ static int size_fifo(struct uart_8250_port *up) >>>>> old_lcr = serial_in(up, UART_LCR); >>>>> serial_out(up, UART_LCR, 0); >>>>> old_fcr = serial_in(up, UART_FCR); >>>>> - old_mcr = serial_in(up, UART_MCR); >>>>> + old_mcr = serial8250_in_MCR(up); >>>>> serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO | >>>>> UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT); >>>>> - serial_out(up, UART_MCR, UART_MCR_LOOP); >>>>> + serial8250_out_MCR(up, UART_MCR_LOOP); >>>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A); >>>>> old_dl = serial_dl_read(up); >>>>> serial_dl_write(up, 0x0001); >>>>> @@ -716,7 +716,7 @@ static int size_fifo(struct uart_8250_port *up) >>>>> (count < 256); count++) >>>>> serial_in(up, UART_RX); >>>>> serial_out(up, UART_FCR, old_fcr); >>>>> - serial_out(up, UART_MCR, old_mcr); >>>>> + serial8250_out_MCR(up, old_mcr); >>>>> serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A); >>>>> serial_dl_write(up, old_dl); >>>>> serial_out(up, UART_LCR, old_lcr); >>>>> @@ -962,17 +962,17 @@ static void autoconfig_16550a(struct uart_8250_port *up) >>>>> * it's changed. If so, set baud_base in EXCR2 to 921600. -- dwmw2 >>>>> */ >>>>> serial_out(up, UART_LCR, 0); >>>>> - status1 = serial_in(up, UART_MCR); >>>>> + status1 = serial8250_in_MCR(up); >>>>> serial_out(up, UART_LCR, 0xE0); >>>>> status2 = serial_in(up, 0x02); /* EXCR1 */ >>>>> >>>>> if (!((status2 ^ status1) & UART_MCR_LOOP)) { >>>>> serial_out(up, UART_LCR, 0); >>>>> - serial_out(up, UART_MCR, status1 ^ UART_MCR_LOOP); >>>>> + serial8250_out_MCR(up, status1 ^ UART_MCR_LOOP); >>>>> serial_out(up, UART_LCR, 0xE0); >>>>> status2 = serial_in(up, 0x02); /* EXCR1 */ >>>>> serial_out(up, UART_LCR, 0); >>>>> - serial_out(up, UART_MCR, status1); >>>>> + serial8250_out_MCR(up, status1); >>>>> >>>>> if ((status2 ^ status1) & UART_MCR_LOOP) { >>>>> unsigned short quot; >>>>> @@ -1146,7 +1146,7 @@ static void autoconfig(struct uart_8250_port *up) >>>>> } >>>>> } >>>>> >>>>> - save_mcr = serial_in(up, UART_MCR); >>>>> + save_mcr = serial8250_in_MCR(up); >>>>> save_lcr = serial_in(up, UART_LCR); >>>>> >>>>> /* >>>>> @@ -1159,9 +1159,9 @@ static void autoconfig(struct uart_8250_port *up) >>>>> * that conflicts with COM 1-4 --- we hope! >>>>> */ >>>>> if (!(port->flags & UPF_SKIP_TEST)) { >>>>> - serial_out(up, UART_MCR, UART_MCR_LOOP | 0x0A); >>>>> + serial8250_out_MCR(up, UART_MCR_LOOP | 0x0A); >>>>> status1 = serial_in(up, UART_MSR) & 0xF0; >>>>> - serial_out(up, UART_MCR, save_mcr); >>>>> + serial8250_out_MCR(up, save_mcr); >>>>> if (status1 != 0x90) { >>>>> spin_unlock_irqrestore(&port->lock, flags); >>>>> DEBUG_AUTOCONF("LOOP test failed (%02x) ", >>>>> @@ -1227,7 +1227,7 @@ static void autoconfig(struct uart_8250_port *up) >>>>> if (port->type == PORT_RSA) >>>>> serial_out(up, UART_RSA_FRR, 0); >>>>> #endif >>>>> - serial_out(up, UART_MCR, save_mcr); >>>>> + serial8250_out_MCR(up, save_mcr); >>>>> serial8250_clear_fifos(up); >>>>> serial_in(up, UART_RX); >>>>> if (up->capabilities & UART_CAP_UUE) >>>>> @@ -1269,19 +1269,18 @@ static void autoconfig_irq(struct uart_8250_port *up) >>>>> >>>>> /* forget possible initially masked and pending IRQ */ >>>>> probe_irq_off(probe_irq_on()); >>>>> - save_mcr = serial_in(up, UART_MCR); >>>>> + save_mcr = serial8250_in_MCR(up); >>>>> save_ier = serial_in(up, UART_IER); >>>>> - serial_out(up, UART_MCR, UART_MCR_OUT1 | UART_MCR_OUT2); >>>>> + serial8250_out_MCR(up, UART_MCR_OUT1 | UART_MCR_OUT2); >>>>> >>>>> irqs = probe_irq_on(); >>>>> - serial_out(up, UART_MCR, 0); >>>>> + serial8250_out_MCR(up, 0); >>>>> udelay(10); >>>>> if (port->flags & UPF_FOURPORT) { >>>>> - serial_out(up, UART_MCR, >>>>> - UART_MCR_DTR | UART_MCR_RTS); >>>>> + serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS); >>>>> } else { >>>>> - serial_out(up, UART_MCR, >>>>> - UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2); >>>>> + serial8250_out_MCR(up, >>>>> + UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2); >>>>> } >>>>> serial_out(up, UART_IER, 0x0f); /* enable all intrs */ >>>>> serial_in(up, UART_LSR); >>>>> @@ -1292,7 +1291,7 @@ static void autoconfig_irq(struct uart_8250_port *up) >>>>> udelay(20); >>>>> irq = probe_irq_off(irqs); >>>>> >>>>> - serial_out(up, UART_MCR, save_mcr); >>>>> + serial8250_out_MCR(up, save_mcr); >>>>> serial_out(up, UART_IER, save_ier); >>>>> >>>>> if (port->flags & UPF_FOURPORT) >>>>> @@ -1701,7 +1700,7 @@ void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl) >>>>> >>>>> mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr; >>>>> >>>>> - serial_port_out(port, UART_MCR, mcr); >>>>> + serial8250_out_MCR(up, mcr); >>>>> } >>>>> EXPORT_SYMBOL_GPL(serial8250_do_set_mctrl); >>>>> >>>>> @@ -2848,7 +2847,7 @@ static void serial8250_console_restore(struct uart_8250_port *up) >>>>> >>>>> serial8250_set_divisor(port, baud, quot, frac); >>>>> serial_port_out(port, UART_LCR, up->lcr); >>>>> - serial_port_out(port, UART_MCR, UART_MCR_DTR | UART_MCR_RTS); >>>>> + serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS); >>>>> } >>>>> >>>>> /* >>>>> >>>> >> -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html