lkml.org 
[lkml]   [2017]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] serial: Do not treat the IIR register as a bitfield
Date
Hey Andy,

On 30-03-17 11:56, Andy Shevchenko wrote:
> On Wed, 2017-03-29 at 20:44 +0200, Olliver Schinagl wrote:
>> It seems that at some point, someone made the assumption that the UART
>> Interrupt ID Register was a bitfield and started to check if certain
>> bits where set.
>>
>> Actually however the register contains interrupt ID's where only the
>> MSB
>> seems to be used singular and the rest share at least one bit. Thus
>> doing bitfield operations is wrong.
>>
>> This patch cleans up the serial_reg include file by ordering it and
>> replacing the UART_IIR_ID 'mask' with a proper mask for the register.
>> The OMAP uart appears to have used the two commonly 'reserved' bits 4
>> and 5 and thus get an UART_IIR_EXT_MASK for these two bits.
>>
>> This patch then goes over all UART_IIR_* users and changes the code
>> from
>> bitfield checking, to ID checking instead.
>
>
> Looking to implementation I would rather go with some helper like
>
> int serial_in_IIR(port, [additional mask])
> {
> return port->serial_in(port, UART_IIR) & (_IIR_MASK [| additional
> mask]);
> }

As I just wrote a simply static inline helper function in serial_core.h,
I just figured that the helper will only work for some of the calls. All
interrupt checks in xxx_serial_in() obviously can't rely on this. So do
you still want this helper function added for the other cases? Or have
all implementations do the masking manually?

And then, is iir = serial_port_in(up, UART_IIR) & UART_IIR_MASK;
preferred over splitting it over two lines, like I did?

Finally, why rename it to _IIR_MASK, I assume a typo here?

Olliver

>

\
 
 \ /
  Last update: 2017-03-31 15:56    [W:0.121 / U:1.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site