Re: [PATCH v2] pinctrl: baytrail: Really serialize all register accesses

[Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>]

On Tue, Nov 19, 2019 at 06:09:17PM +0200, Mika Westerberg wrote:
> On Tue, Nov 19, 2019 at 04:46:41PM +0100, Hans de Goede wrote:
> > Commit 39ce8150a079 ("pinctrl: baytrail: Serialize all register access")
> > added a spinlock around all register accesses because:
> > 
> > "There is a hardware issue in Intel Baytrail where concurrent GPIO register
> >  access might result reads of 0xffffffff and writes might get dropped
> >  completely."
> > 
> > Testing has shown that this does not catch all cases, there are still
> > 2 problems remaining
> > 
> > 1) The original fix uses a spinlock per byt_gpio device / struct,
> > additional testing has shown that this is not sufficient concurent
> > accesses to 2 different GPIO banks also suffer from the same problem.
> > 
> > This commit fixes this by moving to a single global lock.
> > 
> > 2) The original fix did not add a lock around the register accesses in
> > the suspend/resume handling.
> > 
> > Since pinctrl-baytrail.c is using normal suspend/resume handlers,
> > interrupts are still enabled during suspend/resume handling. Nothing
> > should be using the GPIOs when they are being taken down, _but_ the
> > GPIOs themselves may still cause interrupts, which are likely to
> > use (read) the triggering GPIO. So we need to protect against
> > concurrent GPIO register accesses in the suspend/resume handlers too.
> > 
> > This commit fixes this by adding the missing spin_lock / unlock calls.
> > 
> > The 2 fixes together fix the Acer Switch 10 SW5-012 getting completely
> > confused after a suspend resume. The DSDT for this device has a bug
> > in its _LID method which reprograms the home and power button trigger-
> > flags requesting both high and low _level_ interrupts so the IRQs for
> > these 2 GPIOs continuously fire. This combined with the saving of
> > registers during suspend, triggers concurrent GPIO register accesses
> > resulting in saving 0xffffffff as pconf0 value during suspend and then
> > when restoring this on resume the pinmux settings get all messed up,
> > resulting in various I2C busses being stuck, the wifi no longer working
> > and often the tablet simply not coming out of suspend at all.
> > 
> > Cc: stable@xxxxxxxxxxxxxxx
> > Fixes: 39ce8150a079 ("pinctrl: baytrail: Serialize all register access")
> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> 
> Acked-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>

Pushed to my review and testing queue, thanks!

-- 
With Best Regards,
Andy Shevchenko