----- Original Message -----
> From: "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxxxxxxxx>
> To: "Aaron Sierra" <asierra@xxxxxxxxxxx>
> Sent: Monday, July 23, 2018 4:27:58 AM
> On Wed, 2018-07-18 at 18:35 -0500, Aaron Sierra wrote:
>
>> commit a21ae841664aae4c16452a216e26e3a3854fd14c
>> Author: Aaron Sierra <asierra@xxxxxxxxxxx>
>> Date: Wed Jul 18 17:56:37 2018 -0500
>>
>> serial: 8250_exar: Read INT0 from slave device, too
>>
>> My sleep wake-up refactoring in
>>
>> c7e1b4059075 ("tty: serial: exar: Relocate sleep wake-up
>> handling")
>>
>> did not account for devices with a slave device on the expansion
>> port.
>> This patch pokes the INT0 register in the slave device, if
>> present, in
>> order to ensure that MSI interrupts don't get permanently "stuck"
>> because of a sleep wake-up interrupt as described in the original
>> implementation:
>>
>> 2c0ac5b48a35 ("serial: exar: Fix stuck MSIs")
>>
>> Signed-off-by: Aaron Sierra <asierra@xxxxxxxxxxx>
>>
>> diff --git a/drivers/tty/serial/8250/8250_exar.c
>> b/drivers/tty/serial/8250/8250_exar.c
>> index 60b2b48..7c1880a 100644
>> --- a/drivers/tty/serial/8250/8250_exar.c
>> +++ b/drivers/tty/serial/8250/8250_exar.c
>> @@ -524,6 +524,10 @@ static irqreturn_t exar_misc_handler(int irq,
>> void *data)
>> /* Clear all PCI interrupts by reading INT0. No effect on IIR
>> */
>> ioread8(priv->virt + UART_EXAR_INT0);
>
> As I mentioned in previous message, this and below it better to be
> readb().
Hi Andy,
Why better? I've been under the impression that ioreadX() are the
preferred accessors for the general case due to type checking and
general utility. They are typically implemented around readX() anyway.
The virtual address is mapped from an MMIO BAR, so all accesses will be
MMIO in either case.
Here's my (possibly dated) primary motivator:
https://yarchive.net/comp/linux/io_space_accesses.html
Has something changed that I'm not aware of?
>> + /* Clear INT0 for Expansion Interface slave ports, too */
>> + if (priv->board->num_ports > 8)
>
>> + ioread8(priv->virt + (8 * 0x400) + UART_EXAR_INT0);
>
> 0x2000 is explicitly mentioned in the data sheet, so, I doubt we need to
> describe it as 8 * 0x400.
I was conflicted on that, thinking showing the logic behind that value
might have some benefit, but I agree that 0x2000 would be more readily
identifiable within the manual. I'll follow your recommendation in the
version that I submit.
-Aaron S.
>> return IRQ_HANDLED;
>> }
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
