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
