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
--
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
