----- Original Message ----- > From: "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxxxxxxxx> > To: "Aaron Sierra" <asierra@xxxxxxxxxxx> > Sent: Wednesday, July 18, 2018 9:38:46 AM > On Tue, 2018-07-17 at 11:12 +0300, Andy Shevchenko wrote: > > Aaron, while addressing your comment I noticed that your patch > (c7e1b4059075) actually might bring a regression in some cases. Andy, You may be right about that. I'll take a look. Please see my other comments below. > See below what I have as a patch in my WIP tree (https://bitbucket.org/a > ndy-shev/linux/branch/topic/uart/rpm) > > I wouldn't go further with 8250_exar until I hear from you about this. > Some clarifications would be nice to have. > > --- 8< --- 8< --- > > From 19e0b71fe8bd7b2562c05c993f0e60315f2a3205 Mon Sep 17 00:00:00 2001 > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Date: Tue, 17 Jul 2018 19:31:06 +0300 > Subject: [PATCH] serial: 8250_exar: Revert "Relocate sleep wake-up > handling" > > While benefits listed in the commit c7e1b4059075 sounds plausible, it > seems it breaks expansion board support. Do you have any evidence that this is causing breakage in the wild or is this speculation for the time being? > Unfortunately the data sheet doesn't show what happens to interrupts for > the slave UARTs. However, the description of INT0-INT3 register is > clearly points to the fact that the register has covered only 8 > channels, and it's unclear how the rest would behave. If there is a problem, I'm sure that there is a simple solution that wouldn't necessitate a full revert. Making sure to hit INT0 in the slave device's address space would likely be sufficient. Reverting to the previous behavior of hitting INT0 12 or 16 times *per interrupt* (depending on port count) seems very overkill to me. > By the way, pay attention that INT0-INT3 is actually 32-bit register and > ioread8() used in the mentioned patch is wrong, though might work, > by two reasons: > - register is 32-bit and we need a corresponding I/O accessor, > - readl() would be consistent with the rest of the code. Why do you say that INT0-INT3 is 32-bit (as if exclusively)? Exar's documentation for the XR17V358 describes them in 8-bit and 32-bit contexts in close proximity. They are tabulated as 8-bit registers: 0x80 - INT0 0x81 - INT1 0x82 - INT2 0x83 - INT3 And they are tabulated as a 32-bit register: 0x0080-0x0083 - INTERRUPT I believe my use of ioread8 is not wrong and is perfectly safe. However, if you *prefer* to use a 32-bit accessor, then that will impose no adverse performance impact. -Aaron S. > > Considering above revert the commit c7e1b4059075. > > Note, the sequential change would move IRQ handler to 8250_exar.c to fix > what is mentioned in Fixme comment. > > --- 8< --- 8< --- > > > -- > Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > Intel Finland Oy -- 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
