Re: [RFC Patch 05/19] ACPI: Provide union for address_space64 and ext_address_space64

[Jiang Liu <jiang.liu@linux.intel.com>]

On 2015/1/22 10:32, Zheng, Lv wrote:
> Hi, Thomas and Jiang
> 
>> From: Jiang Liu [mailto:jiang.liu@linux.intel.com]
>> Sent: Thursday, January 08, 2015 10:33 AM
>>
>> From: Thomas Gleixner <tglx@linutronix.de>
>>
>> address_space64 and ext_address_space64 share substracts just at
>> different offsets. To unify the parsing functions implement the two
>> structs as unions of their substructs, so we can extract the shared
>> data.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Jiang Liu <jiang.liu@linux.intel.com>
>> ---
>>  include/acpi/acrestyp.h |   49 ++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 36 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h
>> index eb760ca0b2e0..307d5b2605c8 100644
>> --- a/include/acpi/acrestyp.h
>> +++ b/include/acpi/acrestyp.h
>> @@ -326,23 +326,46 @@ struct acpi_resource_address32 {
>>  	struct acpi_resource_source resource_source;
>>  };
>>
>> -struct acpi_resource_address64 {
>> -	ACPI_RESOURCE_ADDRESS_COMMON u64 granularity;
>> -	u64 minimum;
>> -	u64 maximum;
>> -	u64 translation_offset;
>> +#define ACPI_RESOURCE_ADDRESS64_COMMON \
>> +	u64 granularity;	       \
>> +	u64 minimum;		       \
>> +	u64 maximum;		       \
>> +	u64 translation_offset;	       \
>>  	u64 address_length;
>> -	struct acpi_resource_source resource_source;
>> +
>> +struct acpi_resource_address64_common {
>> +ACPI_RESOURCE_ADDRESS64_COMMON};
>> +
>> +struct acpi_resource_address64 {
>> +	union {
>> +		struct {
>> +			ACPI_RESOURCE_ADDRESS_COMMON
>> +			ACPI_RESOURCE_ADDRESS64_COMMON
>> +			struct acpi_resource_source resource_source;
>> +		};
> 
> This looks wrong to ACPICA upstream.
> 
>> +		struct {
>> +			struct acpi_resource_address base;
>> +			struct acpi_resource_address64_common addr;
>> +			struct acpi_resource_source resource_source;
>> +		} common;
>> +	};
>>  };
> 
> And this.
> Though anonymous structs/unions are now C11 standard, I still didn't see it used in the ACPICA upstream.
> It could be a problem if someone still compiles ACPICA using old compilers.
> 
>>
>>  struct acpi_resource_extended_address64 {
>> -	ACPI_RESOURCE_ADDRESS_COMMON u8 revision_ID;
>> -	u64 granularity;
>> -	u64 minimum;
>> -	u64 maximum;
>> -	u64 translation_offset;
>> -	u64 address_length;
>> -	u64 type_specific;
>> +	union {
>> +		struct {
>> +			ACPI_RESOURCE_ADDRESS_COMMON
>> +			u8 revision_ID;
>> +			ACPI_RESOURCE_ADDRESS64_COMMON
>> +			u64 type_specific;
>> +		};
> 
> Ditto.
> 
>> +		struct {
>> +			struct acpi_resource_address base;
>> +			u8 revision_ID;
>> +			struct acpi_resource_address64_common addr;
>> +			u64 type_specific;
>> +		} common;
>> +	};
> 
> Ditto.
> 
> I think what you want is the ability to access common.addr and common.base from different resource address64 types.
> So we can achieve this directly in the ACPICA upstream without using the union.
> 
> I tried this in the ACPICA upstream and the result is:
> https://github.com/zetalog/acpica/commit/0f4ed510
> Let me send its linuxized version after this email.
Hi Lv,
	Thanks for your great support:)
	What's the normal process to propagate this patch into
linux kernel? Or how long will it take? We have another hotplug
patch set which depends on this patch set, then depends on this
ACPICA core change.
Regards!
Gerry
> 
> Thanks and best regards
> -Lv
> 
>>  };
>>
>>  struct acpi_resource_extended_irq {
>> --
>> 1.7.10.4
>