Re: [patch 4/11]makeing dock driver supports bay and battery hotplug

[Thomas Renninger <trenn@suse.de>]

Sorry for the late reply...


On Thursday 28 August 2008 04:03:58 Shaohua Li wrote:
> Making dock driver supports bay and battery hotplug. They are all
> regarded as dock, and unified handled.
>
> Signed-off-by: Shaohua Li <shaohua.li@intel.com>
> ---
...
> +
> +static int is_battery(acpi_handle handle)
> +{
> +	struct acpi_device_info *info;
> +	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> +	int ret = 1;
> +
> +	if (!ACPI_SUCCESS(acpi_get_object_info(handle, &buffer)))
> +		return 0;
> +	info = buffer.pointer;
> +	if (!(info->valid & ACPI_VALID_HID))
> +		ret = 0;
> +	else
> +		ret = !strcmp("PNP0C0A", info->hardware_id.value);
> +
> +	kfree(buffer.pointer);
Better get the device and then use:
const struct acpi_device_id battery_device_ids[] = {
        {"PNP0C0A", 0},
        {"", 0},
};
acpi_match_device_ids(struct acpi_device *device, battery_device_ids);
No need to allocate memory. This also matches batteries where the ID might be 
hidden in the CID list.
...

Something general:
I do not like the idea of this approach.
Battery and ATA device do get an eject notification?
Why can't they handle things themselves?
What is the gain to make battery and ata "dock" capable devices?
What if a PCI(e) device or whatever other device is also connected to the 
docking station. Do we want to add all devices attached to a dock station 
statically?

Would it make sense to add (if notifications are sent to these even this 
should not be needed):
  .eject
  .dock
functions. The dock driver can then go down the tree and call all children's 
eject functions.

I wonder how this can be solved in a more generic way.

Also this patchset mixes up sever fixes and dock design changes patches.
It would be great if the fixes can just go into test, not sure about the 
design change...

    Thomas