Re: [PATCH v1 1/2] serial: 8250_port: fix runtime PM use in __do_stop_tx_rs485()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 26, 2016 at 4:48 PM, Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> On Fri, 2016-08-26 at 16:51 +0300, Matwey V. Kornilov wrote:
>> 2016-08-26 15:49 GMT+03:00 Andy Shevchenko <andriy.shevchenko@xxxxxxxx
>> tel.com>:
>> >
>> > There are calls to serial8250_rpm_{get|put}() in
>> > __do_stop_tx_rs485() that are
>> > certainly placed in a wrong location. I dunno how it had been tested
>> > with
>> > runtime PM enabled because it is obvious "sleep in atomic context"
>> > error.
>> >
>> > Besides that serial8250_rpm_get() is called immediately after an IO
>> > just
>> > happened. It implies that the device is already powered on, see
>> > implementation
>> > of serial8250_em485_rts_after_send() and serial8250_clear_fifos()
>> > for the
>> > details.
>> >
>> > There is no bug have been seen due to, as I can guess, use of auto
>> > suspend mode
>> > when scheduled transaction to suspend is invoked quite lately than
>> > it's needed
>> > for a few writes to the port. It might be possible to trigger a
>> > warning if
>> > stop_tx_timer fires when device is suspended.
>> >
>> > Refactor the code to use runtime PM only in case of timer function.
>
>> > --- a/drivers/tty/serial/8250/8250_port.c
>> > +++ b/drivers/tty/serial/8250/8250_port.c
>> > @@ -1414,12 +1414,8 @@ static void __do_stop_tx_rs485(struct
>> > uart_8250_port *p)
>> >         if (!(p->port.rs485.flags & SER_RS485_RX_DURING_TX)) {
>> >                 serial8250_clear_fifos(p);
>> >
>> > -               serial8250_rpm_get(p);
>> > -
>> >                 p->ier |= UART_IER_RLSI | UART_IER_RDI;
>> >                 serial_port_out(&p->port, UART_IER, p->ier);
>> > -
>> > -               serial8250_rpm_put(p);
>>
>> This has been introduced by 0c66940d584d1aac92f6a78460dc0ba2efd3b7ba
>> Don't you mind add corresponding Fixes tag?
>
> I wouldn't mind, though I wonder if the fix doesn't break anything, so
> Tested-by tags will be appreciated.

I've compiled the kernel with this patch and tested RS485-HD
transmission. So far everything is working as expected. Though I
haven't touch anything PM related.

Tested-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>

>>
>> Fixes: 0c66940d584d ("tty/serial/8250: fix RS485 half-duplex RX")
>>
>> And I would like also
>>
>> Cc: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
>
> --
> Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Intel Finland Oy
--
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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux