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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html