Re: [PATCH 0/2] ACPI: Re-factor and remove ./drivers/acpi/atomicio.[ch]

["Rafael J. Wysocki" <rjw@sisk.pl>]

On Friday, October 28, 2011, Bjorn Helgaas wrote:
> On Thu, Oct 27, 2011 at 7:49 PM, Thomas Renninger <trenn@suse.de> wrote:
> > On Thursday 29 September 2011 23:59:08 Myron Stowe wrote:
> > ..
> >> Myron Stowe (2):
> >>       ACPI: Convert acpi_pre_map_gar()/acpi_atomic_read() and remove ./drivers/acpi/atomicio.[ch]
> >>       ACPI: Export interfaces for ioremapping/iounmapping ACPI registers
> >
> > Would be great to know whether these are going to be accepted.
> > If yes, this check should get removed as well:
> >
> > drivers/acpi/acpica/hwregs.c:
> > acpi_status
> > acpi_hw_validate_register(struct acpi_generic_address *reg,
> >                          u8 max_bit_width, u64 *address)
> > {
> > ...
> >        if (reg->bit_offset != 0) {
> >                ACPI_WARNING((AE_INFO,
> >                              "Unsupported register bit offset: 0x%X",
> >                              reg->bit_offset));
> >        }
> >
> > because APEI GAR declarations do use bit_offset != 0.
> 
> Half of this makes sense to me.  Myron's patch changes APEI from using
> acpi_atomic_read() (which doesn't call acpi_hw_validate_register()) to
> using acpi_read(), which *does* call it.  So after Myron's patch,
> we'll see warnings we didn't see before.
> 
> The part that doesn't make sense to me is just removing the warning.
> That warning looks to me like it's saying "oops, here's something we
> should support, but haven't implemented yet."  Wouldn't it be better
> to implement support for bit_offset in acpi_read() at the same time we
> remove the warning?  Then Myron could update his patch to drop the
> bit_offset support in __apei_exec_read_register() when converting to
> acpi_read().
> 
> If APEI uses bit_offset != 0, it's at least possible that other areas
> will use it in the future, and it'd be nicer to have all the support
> in acpi_read() rather than forcing APEI and others to each implement
> their own support for it.

Agreed.

Thanks,
Rafael