On 2014/9/9 18:54, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
>>
>> +static struct acpi_table_madt *madt;
>> +static int read_madt;
>
> Pretty lousy file visible variable names.
>
> So we end up with two copies of the butt ugly
>
> if (!read_madt) {
> .....
> }
>
> stuff instead of creating a helper function which hides that and do
>
> madr = get_madt();
> if (!madt)
> return ...;
>
> at the call sites.
Thanks for suggestion, will improve in that way.
>
>> static int map_lapic_id(struct acpi_subtable_header *entry,
>> u32 acpi_id, int *apic_id)
>> {
>> @@ -64,8 +71,6 @@ static int map_lsapic_id(struct acpi_subtable_header *entry,
>> static int map_madt_entry(int type, u32 acpi_id)
>> {
>> unsigned long madt_end, entry;
>> - static struct acpi_table_madt *madt;
>> - static int read_madt;
>> int apic_id = -1;
>>
>> if (!read_madt) {
>> @@ -200,3 +205,90 @@ int acpi_get_cpuid(acpi_handle handle, int type, u32 acpi_id)
>> return acpi_map_cpuid(apic_id, acpi_id);
>> }
>> EXPORT_SYMBOL_GPL(acpi_get_cpuid);
>> +
>> +#ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
>> +static int map_ioapic_id(struct acpi_subtable_header *entry, u32 gsi_base,
>> + u64 *phys_addr, int *ioapic_id)
>> +{
>> + struct acpi_madt_io_apic *ioapic = (struct acpi_madt_io_apic *)entry;
>> +
>> + if (ioapic->global_irq_base != gsi_base)
>> + return 0;
>> +
>> + *phys_addr = ioapic->address;
>> + *ioapic_id = ioapic->id;
>> + return 1;
>> +}
>> +
>> +static int map_madt_ioapic_entry(u32 gsi_base, u64 *phys_addr)
>> +{
>> + struct acpi_subtable_header *hdr;
>> + unsigned long madt_end, entry;
>> + int apic_id = -1;
>> +
>> + if (!read_madt) {
>> + if (ACPI_FAILURE(acpi_get_table(ACPI_SIG_MADT, 0,
>> + (struct acpi_table_header **)&madt)))
>> + madt = NULL;
>> + read_madt++;
>> + }
>> +
>> + if (!madt)
>> + return apic_id;
>> +
>> + entry = (unsigned long)madt;
>> + madt_end = entry + madt->header.length;
>> +
>> + /* Parse all entries looking for a match. */
>> + entry += sizeof(struct acpi_table_madt);
>> + while (entry + sizeof(struct acpi_subtable_header) < madt_end) {
>> + hdr = (struct acpi_subtable_header *)entry;
>> + if (hdr->type == ACPI_MADT_TYPE_IO_APIC &&
>> + map_ioapic_id(hdr, gsi_base, phys_addr, &apic_id))
>> + break;
>> + else
>> + entry += hdr->length;
>> + }
>> +
>> + return apic_id;
>> +}
>> +
>> +static int map_mat_ioapic_entry(acpi_handle handle, u32 gsi_base,
>> + u64 *phys_addr)
>> +{
>> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>> + union acpi_object *obj;
>> + struct acpi_subtable_header *header;
>> + int apic_id = -1;
>> +
>> + if (ACPI_FAILURE(acpi_evaluate_object(handle, "_MAT", NULL, &buffer)))
>> + goto exit;
>> +
>> + if (!buffer.length || !buffer.pointer)
>> + goto exit;
>> +
>> + obj = buffer.pointer;
>> + if (obj->type != ACPI_TYPE_BUFFER ||
>> + obj->buffer.length < sizeof(struct acpi_subtable_header))
>> + goto exit;
>> +
>> + header = (struct acpi_subtable_header *)obj->buffer.pointer;
>> + if (header->type == ACPI_MADT_TYPE_IO_APIC)
>> + map_ioapic_id(header, gsi_base, phys_addr, &apic_id);
>> +
>> +exit:
>> + kfree(buffer.pointer);
>> + return apic_id;
>> +}
>
>
> This stuff really wants proper documentation. The lack of comments is
> just annoying.
Sure, will improve comments in next version.
Regards!
Gerry
>
>> +int acpi_get_ioapic_id(acpi_handle handle, u32 gsi_base, u64 *phys_addr)
>> +{
>> + int apic_id;
>> +
>> + apic_id = map_mat_ioapic_entry(handle, gsi_base, phys_addr);
>> + if (apic_id == -1)
>> + apic_id = map_madt_ioapic_entry(gsi_base, phys_addr);
>> +
>> + return apic_id;
>> +}
>
> Thanks,
>
> tglx
>
--
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
