On Thu, Jun 10, 2021 at 8:39 PM Maciej W. Rozycki <macro@xxxxxxxxxxx> wrote: > > Make sure only actual 8 bits of the IIR register are used in determining > the port type in `autoconfig'. > > The `serial_in' port accessor returns the `unsigned int' type, meaning > that with UPIO_AU, UPIO_MEM16, UPIO_MEM32, and UPIO_MEM32BE access types > more than 8 bits of data are returned, of which the high order bits will > often come from bus lines that are left floating in the data phase. For > example with the MIPS Malta board's CBUS UART, where the registers are > aligned on 8-byte boundaries and which uses 32-bit accesses, data as > follows is returned: > > YAMON> dump -32 0xbf000900 0x40 > > BF000900: 1F000942 1F000942 1F000900 1F000900 ...B...B........ > BF000910: 1F000901 1F000901 1F000900 1F000900 ................ > BF000920: 1F000900 1F000900 1F000960 1F000960 ...........`...` > BF000930: 1F000900 1F000900 1F0009FF 1F0009FF ................ > > YAMON> > > Evidently high-order 24 bits return values previously driven in the > address phase (the 3 highest order address bits used with the command > above are masked out in the simple virtual address mapping used here and > come out at zeros on the external bus), a common scenario with bus lines > left floating, due to bus capacitance. > > Consequently when the value of IIR, mapped at 0x1f000910, is retrieved > in `autoconfig', it comes out at 0x1f0009c1 and when it is right-shifted > by 6 and then assigned to 8-bit `scratch' variable, the value calculated > is 0x27, not one of 0, 1, 2, 3 expected in port type determination. > > Fix the issue then, by assigning the value returned from `serial_in' to > `scratch' first, which masks out 24 high-order bits retrieved, and only > then right-shift the resulting 8-bit data quantity, producing the value > of 3 in this case, as expected. Fix the same issue in `serial_dl_read'. > > The problem first appeared with Linux 2.6.9-rc3 which predates our repo > history, but the origin could be identified with the old MIPS/Linux repo > also at: <git://git.kernel.org/pub/scm/linux/kernel/git/ralf/linux.git> > as commit e0d2356c0777 ("Merge with Linux 2.6.9-rc3."), where code in > `serial_in' was updated with this case: > > + case UPIO_MEM32: > + return readl(up->port.membase + offset); > + > > which made it produce results outside the unsigned 8-bit range for the > first time, though obviously it is system dependent what actual values > appear in the high order bits retrieved and it may well have been zeros > in the relevant positions with the system the change originally was > intended for. It is at that point that code in `autoconf' should have > been updated accordingly, but clearly it was overlooked. > > Signed-off-by: Maciej W. Rozycki <macro@xxxxxxxxxxx> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > --- > drivers/tty/serial/8250/8250_port.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@xxxxxxxxx>