Re: [ACPIHP PATCH part1 2/4] ACPIHP: introduce acpihp_slot driverto enumerate hotplug slots

[Jiang Liu]

On 11/06/2012 05:30 AM, Bjorn Helgaas wrote:
> On Sat, Nov 3, 2012 at 10:07 AM, Jiang Liu <liuj97@xxxxxxxxx> wrote:
>> An ACPI hotplug slot is an abstraction of receptacles, where a group of
>> system devices could be connected to. This patch implements the skeleton
>> of the ACPI system device hotplug slot enumerator. On loading, it scans
>> the whole ACPI namespace for hotplug slots and creates a device node for
>> each hotplug slot found. Every hotplug slot is associated with a device
>> class named acpihp_slot_class. Later hotplug drivers will register onto
>> acpihp_slot_class to manage all hotplug slots.
>>
>> Signed-off-by: Jiang Liu <jiang.liu@xxxxxxxxxx>
>> Signed-off-by: Gaohuai Han <hangaohuai@xxxxxxxxxx>
>> ---
>>  drivers/acpi/Kconfig          |   19 ++
>>  drivers/acpi/hotplug/Makefile |    3 +
>>  drivers/acpi/hotplug/slot.c   |  417 +++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 439 insertions(+)
>>  create mode 100644 drivers/acpi/hotplug/slot.c
>>
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 9577b23..af0aaf6 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -334,6 +334,25 @@ menuconfig ACPI_HOTPLUG
>>           If your hardware platform does not support system device dynamic
>>           reconfiguration at runtime, you need not to enable this option.
>>
>> +config ACPI_HOTPLUG_SLOT
>> +       tristate "System Device Hotplug Slot Enumerator"
> 
> I don't really believe in hotplug drivers being modules.  I think the
> core should support hotplug directly, and the decision to configure or
> not should be made at build-time.
Good suggestion, I will change it in next version.

>> +       depends on ACPI_HOTPLUG
>> +       default m
>> +       help
>> +         ACPI system device hotplug slot is an abstraction of ACPI based
>> +         system device dynamic reconfiguration control points. On load,
>> +         this driver enumerates system device hotplug slots by wakling the
>> +         ACPI namespace and provides platform specific methods to control
>> +         those hotplug slots.
>> +
>> +         By default, this driver detects system device hotplug slots by
>> +         checking avaliability of ACPI _EJ0 method. You may pass a module
>> +         parameter "fake_slot=0xf" to enable faking hotplug slots on
>> +         platforms without hardware dynamic reconfiguration capabilities.
>> +
>> +         To compile this driver as a module, choose M here:
>> +         the module will be called acpihp_slot.
>> +
> 
>> +static int __init acpihp_slot_generate_name(struct acpihp_slot *slot)
>> +{
>> +       int found = 0;
>> +       u32 child_types = 0;
>> +       unsigned long long uid;
>> +       struct acpihp_slot_id *slot_id;
>> +
>> +       /*
>> +        * Figure out slot type by checking types of ACPI devices which could
>> +        * be attached to the slot.
>> +        */
>> +       slot->type = acpihp_slot_get_type_self(slot);
>> +       if (slot->type == ACPIHP_SLOT_TYPE_UNKNOWN) {
>> +               acpi_walk_namespace(ACPI_TYPE_DEVICE, slot->handle,
>> +                               ACPI_UINT32_MAX, acpihp_slot_get_dev_type,
>> +                               NULL, NULL, (void **)&child_types);
>> +               acpi_walk_namespace(ACPI_TYPE_PROCESSOR, slot->handle,
>> +                               ACPI_UINT32_MAX, acpihp_slot_get_dev_type,
>> +                               NULL, NULL, (void **)&child_types);
>> +               slot->type = acpihp_slot_get_type_child(child_types);
>> +       }
> 
> If things can be hot-added below slot->handle, is there an ACPI
> requirement that there be *anything* in the existing namespace below
> slot->handle?  I'm not sure you can tell sort of things might be
> added.
To be compatible with old Windows, some BIOS will expose such a behavior.
It put the PCI root bus built in a physical processor directly under /_SB
instead of the module device for the processor. According to my observation,
new BIOSes have changed that behavior and build ACPI namespace according to
physical topology. 

>> +static int __init acpihp_slot_scan_slots(void)
>> +{
>> +       acpi_status status;
>> +
>> +       status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
>> +                                    ACPI_UINT32_MAX, acpihp_slot_scan,
>> +                                    NULL, NULL, NULL);
>> +       if (!ACPI_SUCCESS(status))
>> +               goto out_err;
>> +
>> +       status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
>> +                                    ACPI_UINT32_MAX, acpihp_slot_scan,
>> +                                    NULL, NULL, NULL);
> 
> Here's one reason I don't like this as a module: we have to walk the
> namespace again (twice, even).  What happens when you hot-add a node
> that itself *contains* another hot-pluggable receptacle?  Do you walk
> the namespace again, calling acpiphp_slot_scan() as needed?
Loading of dynamic ACPI table is an issue, it should be handled. But we have
left that on the TODO list for simplicity at first stage. When supporting 
that in future, we just need to scan the subtree starting from the slot.

Thanks
Gerry
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/