Re: Fwd: Issues with parity error handling in s3c2410 serial driver

[Guillermo Rodriguez <guille.rodriguez@gmail.com>]

El 08/05/2014 21:55, Greg Kroah-Hartman escribió:
> On Thu, May 08, 2014 at 01:31:10PM +0200, Guillermo Rodriguez Garcia wrote:
>> Hello all,
>>
>> I think I have found some issues with the handling of parity errors in the
>> s3c2410 serial driver (drivers/tty/serial/samsung.c)
>>
>> 1.
>> The driver defines S3C2410_UERSTAT_PARITY as 0x1000
>> (see http://lxr.free-electrons.com/source/drivers/tty/serial/samsung.c#L265)
>>
>> However, at least in the S3C2440, the correct value should be 0x02.
>> This matches the definition in include/linux/serial_s3c.h for the S3C2443 (see
>> http://lxr.free-electrons.com/source/include/linux/serial_s3c.h#L165)
>>
>> 2.
>> The driver currently contains code to detect parity errors, but this code is
>> guarded by the following check:
>>
>> if (unlikely(uerstat & S3C2410_UERSTAT_ANY)) {
>> ...
>> }
>>
>> And the definition of S3C2410_UERSTAT_ANY in include/linux/serial_s3c.h
>> does not include the parity bit.
>>
>> This means that the code that currently checks for parity errors in the serial
>> driver won't actually be run when a parity error occurs. I think that this is a
>> bug and that S3C2410_UERSTAT_ANY should also contain the parity bit
>>
>> 3.
>> The code that detects parity errors does not increment the parity error
>> counter in the serial_icounter_struct structure. Other errors (framing and
>> overrun) are accounted for -- it is just the parity error that is not traced
>> (see http://lxr.free-electrons.com/source/drivers/tty/serial/samsung.c#L277).
>>
>> This means that parity errors will not be reported via the TIOCGICOUNT
>> ioctl call.
>>
>>
>> Can someone comment on the above?
> 
> Can you make up a patch to resolve these and be sure to cc: the driver
> authors and this list?

I think it would be a good idea to split this in two patches. Issue 3/ above
is trivial to fix and this can get through easily.

For 1/ and 2/ I am sure that the problem is _not_ specific to the s3c2440
(the way it is now, parity errors are never checked since S3C2410_UERSTAT_ANY
only contains the overrun and framing bit masks). However I can only test on
the s3c2440. Shall I make a patch specifically for the s3c2440, or should I
try to make it generic? If the latter, I could use some help with testing on
other s3c SoCs.

Thank you,

Guillermo Rodriguez
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html