Re: [PATCH] serial: 8250_dw: Fix LCR workaround regression

[Tim Kryger]

On Wed, Dec 11, 2013 at 6:00 AM, Heikki Krogerus
<heikki.krogerus@xxxxxxxxxxxxxxx> wrote:
> Hi,
>
> On Tue, Dec 10, 2013 at 10:28:04PM +0000, James Hogan wrote:
>> From: James Hogan <james.hogan@xxxxxxxxxx>
>>
>> Commit c49436b657d0 (serial: 8250_dw: Improve unwritable LCR workaround)
>> caused a regression. It added a check that the LCR was written properly
>> to detect and workaround the busy quirk, but the behaviour of bit 5
>> (UART_LCR_SPAR) differs between IP versions 3.00a and 3.14c per the
>> docs. On older versions this caused the check to fail and it would
>> repeatedly force idle and rewrite the LCR register, causing delays and
>> preventing any input from serial being received.
>>
>> This is fixed by masking out UART_LCR_SPAR before making the comparison.
>>
>> Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> Cc: Jiri Slaby <jslaby@xxxxxxx>
>> Cc: Tim Kryger <tim.kryger@xxxxxxxxxx>
>> Cc: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
>> Cc: Matt Porter <matt.porter@xxxxxxxxxx>
>> Cc: Markus Mayer <markus.mayer@xxxxxxxxxx>
>> Cc: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
>> Tested-by: Tim Kryger <tim.kryger@xxxxxxxxxx>
>> Tested-by: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
>
> I tested this with HW without the busy functionality
> (UART_16550_COMPATIBLE == yes). No problems here. FWIW:
>
> Tested-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>

Greg, can you consider taking this patch as a regression fix for 3.13?

Just in case you want a little more background beyond what is in the
commit message...

Statically declared DW UARTs will execute autoconfig_16550a during
probe which attempts to set the SPAR bit of LCR.  Some old versions of
the DW IP don't implement the stick parity feature and for these
versions the SPAR bit always reads back as zero. Since these writes
aren't fully accepted by the hardware, they trigger the workaround
which interprets the partially rejected write to mean the UART was
busy.  A number of attempts are made to idle the UART and re-write the
LCR but eventually the workaround gives up and prints a warning.  DT
declared DW UARTs don't run autoconfig and don't see this issue as far
as I know.

Thanks,
Tim Kryger
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/