Re: assumptions in acpi drivers

[Bjorn Helgaas <bjorn.helgaas@hp.com>]

On Sat, 2009-10-10 at 14:16 +0100, Alan Jenkins wrote:

> > The current upstream version of this function also has the kfree
> > issue.  You're patching a different version, so it might be fixed
> > there already.  But it seems like all these hotkey drivers (and the
> > accelerometer driver) use the same pattern of assuming they'll only
> > see a single device, and then they make assumptions like "hotk ==
> > acpi_driver_data(device)".  That's just a bad example for people
> > to follow, and might not even be true for things like accelerometers
> > where you could imagine having more than one.
> 
> I've been thinking about this.  Frankly, I can't imagine a machine
> having multiple ACPI accelerometer devices with the same interface.
> It doesn't make sense to have multiple instances of any of the drivers
> under /platform/.  And many of them create platform devices with fixed
> names, so we can't pretend it would work.
> 
> Perhaps the best approach is to remove the references to
> device->driverdata, and consistently use "hotk" or equivalent.  The
> _core_ acpi devices would always provide correct examples of how to
> use driverdata.

I think the best pattern is something like this:

    struct acpi_device *singleton;

    xxx_add(struct acpi_device *device, ...)
    {
	if (singleton)
		return -EINVAL;

	xxx_data = kzalloc(...);
	xxx_data->foo = 0;

	device->driver_data = xxx_data;
	singleton = xxx_data;
	return 0;
    }

    xxx_remove(struct acpi_device *device)
    {
	kfree(acpi_driver_data(device));
    }

That way the assumption that there's only a single device is confined to
the add() method.  That makes the rest of the driver easier to read and
verify because it looks like every other ACPI driver.

Bjorn