On Thursday, September 02, 2010 07:49:59 pm Huang Ying wrote: > On Fri, 2010-09-03 at 09:34 +0800, Bjorn Helgaas wrote: > > Hello, > > > > You recently added acpi_atomic_read() and acpi_atomic_write(). > > These are similar to acpi_read() and acpi_write(), but differ > > mainly in two ways: > > > > - The atomic ones can be used in interrupt context, because the > > ioremap() (which may block) can be done earlier by acpi_pre_map_gar() > > > > - The atomic ones do 64-bit accesses directly with readq() rather > > than splitting them into two 32-bit accesses as acpi_read() does. > > > > It's obvious to me that you need the first property (usable in > > interrupt context). Do you also rely on the second property > > (doing a single 64-bit access rather than two 32-bit accesses)? > > We don't rely on the 64-bit access. But I am not sure whether all > corresponding hardware is OK for splitting accessing. But maybe we can > fix it in the future if necessary. For example adding an > acpi_os_read_memory64. Good. Unless there's a spec requirement for 64-bit accesses in this area, I think it's better to do it the same way as acpi_read(), just so we're as consistent as possible. > > I'm curious because there's another path that performs memory > > accesses from interrupt context, using acpi_read() and > > acpi_os_read_memory(). The ACPI IRQ handler reads the PM1 > > Status register to learn the interrupt source. If this is > > memory mapped (not in I/O port space), we currently panic > > because ioremap() can't be called in interrupt context: > > > > http://marc.info/?l=linux-acpi&m=128094238920118&w=2 > > > > I wrote a patch that fixes this by premapping these areas so the > > ioremap() happens at boot-time rather than interrupt-time. This > > works fine, but it ends up looking very much like the atomic > > functions you added. I'd like to integrate them somehow rather > > than adding more code that does basically the same thing as your > > code. > > > > If your APEI code that uses acpi_atomic_read()/write() doesn't > > require the single 64-bit access, it might be possible to keep > > the premapping support, i.e., acpi_pre_map_gar(), change > > acpi_os_read_memory() to check the acpi_iomaps list first and > > only do the ioremap() if it doesn't find anything, remove > > acpi_atomic_read(), and just use acpi_read() instead. > > That is OK for me. Great, I'll experiment with this approach. Thanks a lot for the help! Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
