Re: [ACPI] Re: [PATCH] PNPACPI: fix types when decoding ACPI resources [resend]

Bjorn Helgaas
2005-08-03T15:20:21Z

On Tuesday 02 August 2005 7:01 pm, Shaohua Li wrote:
> On Tue, 2005-08-02 at 09:55 -0600, Bjorn Helgaas wrote:
> > Any objections to the patch below? I posted it last Wednesday,
> > but haven't heard anything. Once we have this fix, 8250_pnp
> > should have sufficient functionality that we can get rid of
> > 8250_acpi.
> >
> > Use types that match the ACPI resource structures. Previously
> > the u64 value from an RSTYPE_ADDRESS64 was passed as an int,
> > which corrupts the value.
> >
> > This is one of the things that prevents 8250_pnp from working
> > on HP ia64 boxes. After 8250_pnp works, we will be able to
> > remove 8250_acpi.c.
> We might always use 'unsigned long'.

Do you have a reason for preferring 'unsigned long' over the
exact types used in the ACPI resource structures? I thought
it was useful to use the exact types, because then whatever
conversion needs to happen is all in one place.

In the existing code, there's implicit conversion when you
call "pnpacpi_parse_allocated_memresource(..., int mem, int len)"
and pass u64 values as "mem" and "len". You have to look both
at the call site and the called code. And gcc doesn't even
complain about this truncation.

But I guess it doesn't matter much either way.

> Did you have plan to remove other
> legacy acpi drivers?

No, I didn't -- which ones are you thinking about? Looking at
the callers of acpi_bus_register_driver(), I see:

arch/ia64/hp/common/sba_iommu.c
Probably can't be converted because it needs the
ACPI handle to extract a vendor-specific data
item from _CRS.

drivers/char/hpet.c
This probably should be converted to PNP. I'll
look into doing this.

Then of course, there are a bunch of things in drivers/acpi/
(battery, button, fan, ec, etc). I expect the reason they are
in drivers/acpi/ is because they need ACPI-specific functionality,
so they probably couldn't be converted to PNP.