Re: [PATCH] i2c: i801: Allow ACPI SystemIO OpRegion to conflict with PCI BAR

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, May 02, 2016 at 08:29:42AM -0700, Andy Lutomirski wrote:
> On Mon, May 2, 2016 at 3:12 AM, Mika Westerberg
> <mika.westerberg@xxxxxxxxxxxxxxx> wrote:
> > On Fri, Apr 29, 2016 at 06:13:52PM -0700, Andy Lutomirski wrote:
> >> A question, though: there's nothing that keeps i801_access from being
> >> called in between consecutive ACPI accesses.  Could this confuse the
> >> ASL code?  Would it be helpful if i801_access were to save away the
> >> old register state and restore it when it's done in the event that an
> >> opregion access has been seen so that the ASL-configured state doesn't
> >> get stomped on?
> >
> > Looking at those ASL methods of Lenovo Yoga 900 for example they seem to
> > initialize the hardware, do the transaction and cleanup in one go.
> > That's also what the i2c-i801.c driver is doing as far as I can say. So
> > in that sense they should not mess with each other.
> >
> 
> Is your locking actually sufficient to get that right?  You're taking
> acpi_lock, which is private to the driver, so you're only holding it
> during actual opregion access AFAICT.  That means that, if one thread
> is in the ACPI interpreter in one of these blocks and another thread
> is in the driver, they could still interleave their accesses.  Am I
> missing something?

No, you are right.

> > Of course this all breaks if the ASL code expects the state to survive
> > between transactions.
> >
> >> Also, what happens if i801_access happens while the i801 master is
> >> busy with an ASL-initiated transaction?  Will it wait for the
> >> transaction to finish?
> >
> > Yes, it should since ->acpi_lock is taken by i801_acpi_io_handler().
> 
> But i801_acpi_io_handler has no concept of a transaction AFAICT.

Indeed it only handles access one-by-one not by transaction. So if the
ASL code is in middle of a transaction and i2c-i801.c starts one as well
it will mess the registers.

Back to drawing board :-/
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux