Messages in this thread | | | Date | Mon, 14 Jan 2013 08:37:59 +0000 | From | "Jan Beulich" <> | Subject | Re: your patch "x86, 8042: Enable A20 using KBC to fix S3 resume on some MSI laptops" |
| |
>>> On 14.01.13 at 09:29, Ondrej Zary <linux@rainbow-software.org> wrote: > On Monday 14 January 2013, Jan Beulich wrote: >> Ondrej, >> >> I see two problems with this patch: For one, on a system without >> i8042 the code at the place it got inserted ought to incur a stall of >> 1s (50us * I8042_CTL_TIMEOUT [10000] * 2). I believe that this >> code should not be run before i8042_controller_check() completed >> successfully, but at the very least the second call to >> i8042_command() should be conditional upon the first being >> successful (effectively halving the stall). > > I believe that all PnP-capable systems without 8042 will exit with -ENODEV > after x86_platform.i8042_detect().
How that, with default_i8042_detect() being just "return 1;"?
> Old non-PnP systems usually have 8042.
Sure.
>> Second, considering that enabling A20 (even if just in a fake way), >> is a core system operation, I don't think it belongs into a driver >> that is only optionally present in the kernel. > > The first version of this patch added A20 enabling to early init code. But > that could be dangerous as it was run before any 8042 detection, possibly > breaking systems without 8042. I haven't found a better place for this.
I realize that the change would be more intrusive, but imo that's not an excuse for not doing it properly.
Jan
| |