* Re: [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions
[not found] ` <1378477486-8758-5-git-send-email-tianyu.lan@intel.com>
@ 2013-09-06 15:36 ` Bjorn Helgaas
2013-09-06 16:01 ` Lan Tianyu
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2013-09-06 15:36 UTC (permalink / raw)
To: Lan Tianyu
Cc: Len Brown, Rafael J. Wysocki, Yinghai Lu,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Luck, linux-ia64@vger.kernel.org
[+cc Tony, linux-ia64]
On Fri, Sep 6, 2013 at 8:24 AM, Lan Tianyu <tianyu.lan@intel.com> wrote:
> Using ACPI resource functions to convert ACPI resource to generic resource
> instead of resource_to_addr(). Remove resource_to_addr().
>
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
> arch/x86/pci/acpi.c | 81 ++++++++---------------------------------------------
> 1 file changed, 12 insertions(+), 69 deletions(-)
Please make corresponding changes to arch/ia64/pci/pci.c so that these
paths remain as similar as possible. There's quite a bit of
similarity between this x86 and ia64 code, and it would be nice to
unify them more when possible.
> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
> index d641897..d4f85a1 100644
> --- a/arch/x86/pci/acpi.c
> +++ b/arch/x86/pci/acpi.c
> @@ -219,62 +219,15 @@ static void teardown_mcfg_map(struct pci_root_info *info)
> #endif
>
> static acpi_status
> -resource_to_addr(struct acpi_resource *resource,
> - struct acpi_resource_address64 *addr)
> -{
> - acpi_status status;
> - struct acpi_resource_memory24 *memory24;
> - struct acpi_resource_memory32 *memory32;
> - struct acpi_resource_fixed_memory32 *fixed_memory32;
> -
> - memset(addr, 0, sizeof(*addr));
> - switch (resource->type) {
> - case ACPI_RESOURCE_TYPE_MEMORY24:
> - memory24 = &resource->data.memory24;
> - addr->resource_type = ACPI_MEMORY_RANGE;
> - addr->minimum = memory24->minimum;
> - addr->address_length = memory24->address_length;
> - addr->maximum = addr->minimum + addr->address_length - 1;
> - return AE_OK;
> - case ACPI_RESOURCE_TYPE_MEMORY32:
> - memory32 = &resource->data.memory32;
> - addr->resource_type = ACPI_MEMORY_RANGE;
> - addr->minimum = memory32->minimum;
> - addr->address_length = memory32->address_length;
> - addr->maximum = addr->minimum + addr->address_length - 1;
> - return AE_OK;
> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> - fixed_memory32 = &resource->data.fixed_memory32;
> - addr->resource_type = ACPI_MEMORY_RANGE;
> - addr->minimum = fixed_memory32->address;
> - addr->address_length = fixed_memory32->address_length;
> - addr->maximum = addr->minimum + addr->address_length - 1;
> - return AE_OK;
> - case ACPI_RESOURCE_TYPE_ADDRESS16:
> - case ACPI_RESOURCE_TYPE_ADDRESS32:
> - case ACPI_RESOURCE_TYPE_ADDRESS64:
> - status = acpi_resource_to_address64(resource, addr);
> - if (ACPI_SUCCESS(status) &&
> - (addr->resource_type = ACPI_MEMORY_RANGE ||
> - addr->resource_type = ACPI_IO_RANGE) &&
> - addr->address_length > 0) {
> - return AE_OK;
> - }
> - break;
> - }
> - return AE_ERROR;
> -}
> -
> -static acpi_status
> count_resource(struct acpi_resource *acpi_res, void *data)
> {
> struct pci_root_info *info = data;
> - struct acpi_resource_address64 addr;
> - acpi_status status;
> + struct resource res;
>
> - status = resource_to_addr(acpi_res, &addr);
> - if (ACPI_SUCCESS(status))
> + if (acpi_dev_resource_address_space(acpi_res, &res)
> + || acpi_dev_resource_memory(acpi_res, &res))
> info->res_num++;
> +
> return AE_OK;
> }
>
> @@ -282,27 +235,18 @@ static acpi_status
> setup_resource(struct acpi_resource *acpi_res, void *data)
> {
> struct pci_root_info *info = data;
> - struct resource *res;
> + struct resource *res = &info->res[info->res_num];
> struct acpi_resource_address64 addr;
> - acpi_status status;
> - unsigned long flags;
> u64 start, orig_end, end;
>
> - status = resource_to_addr(acpi_res, &addr);
> - if (!ACPI_SUCCESS(status))
> - return AE_OK;
> + memset(&addr, 0x00, sizeof(addr));
>
> - if (addr.resource_type = ACPI_MEMORY_RANGE) {
> - flags = IORESOURCE_MEM;
> - if (addr.info.mem.caching = ACPI_PREFETCHABLE_MEMORY)
> - flags |= IORESOURCE_PREFETCH;
> - } else if (addr.resource_type = ACPI_IO_RANGE) {
> - flags = IORESOURCE_IO;
> - } else
> + if (!(acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
> + || acpi_dev_resource_memory(acpi_res, res)))
> return AE_OK;
>
> - start = addr.minimum + addr.translation_offset;
> - orig_end = end = addr.maximum + addr.translation_offset;
> + start = res->start;
> + orig_end = end = res->end;
>
> /* Exclude non-addressable range or non-addressable portion of range */
> end = min(end, (u64)iomem_resource.end);
> @@ -310,6 +254,7 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> dev_info(&info->bridge->dev,
> "host bridge window [%#llx-%#llx] "
> "(ignored, not CPU addressable)\n", start, orig_end);
> + memset(&info->res[info->res_num], 0x00, sizeof(*res));
> return AE_OK;
> } else if (orig_end != end) {
> dev_info(&info->bridge->dev,
> @@ -318,11 +263,9 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
> start, orig_end, end + 1, orig_end);
> }
>
> - res = &info->res[info->res_num];
> res->name = info->name;
> - res->flags = flags;
> - res->start = start;
> res->end = end;
> +
> info->res_offset[info->res_num] = addr.translation_offset;
> info->res_num++;
>
> --
> 1.8.2.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions
2013-09-06 15:36 ` [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions Bjorn Helgaas
@ 2013-09-06 16:01 ` Lan Tianyu
2013-09-06 16:10 ` Bjorn Helgaas
0 siblings, 1 reply; 4+ messages in thread
From: Lan Tianyu @ 2013-09-06 16:01 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Len Brown, Rafael J. Wysocki, Yinghai Lu,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Luck, linux-ia64@vger.kernel.org
On 09/06/2013 11:36 AM, Bjorn Helgaas wrote:
> [+cc Tony, linux-ia64]
>
Hi Bjorn:
Thanks for review.
> On Fri, Sep 6, 2013 at 8:24 AM, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> Using ACPI resource functions to convert ACPI resource to generic resource
>> instead of resource_to_addr(). Remove resource_to_addr().
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>> arch/x86/pci/acpi.c | 81 ++++++++---------------------------------------------
>> 1 file changed, 12 insertions(+), 69 deletions(-)
>
> Please make corresponding changes to arch/ia64/pci/pci.c so that these
> paths remain as similar as possible. There's quite a bit of
> similarity between this x86 and ia64 code, and it would be nice to
> unify them more when possible.
>
OK. Actually, I have such plan. I will do that if there is no objection
on this patchset.
>> diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
>> index d641897..d4f85a1 100644
>> --- a/arch/x86/pci/acpi.c
>> +++ b/arch/x86/pci/acpi.c
>> @@ -219,62 +219,15 @@ static void teardown_mcfg_map(struct pci_root_info *info)
>> #endif
>>
>> static acpi_status
>> -resource_to_addr(struct acpi_resource *resource,
>> - struct acpi_resource_address64 *addr)
>> -{
>> - acpi_status status;
>> - struct acpi_resource_memory24 *memory24;
>> - struct acpi_resource_memory32 *memory32;
>> - struct acpi_resource_fixed_memory32 *fixed_memory32;
>> -
>> - memset(addr, 0, sizeof(*addr));
>> - switch (resource->type) {
>> - case ACPI_RESOURCE_TYPE_MEMORY24:
>> - memory24 = &resource->data.memory24;
>> - addr->resource_type = ACPI_MEMORY_RANGE;
>> - addr->minimum = memory24->minimum;
>> - addr->address_length = memory24->address_length;
>> - addr->maximum = addr->minimum + addr->address_length - 1;
>> - return AE_OK;
>> - case ACPI_RESOURCE_TYPE_MEMORY32:
>> - memory32 = &resource->data.memory32;
>> - addr->resource_type = ACPI_MEMORY_RANGE;
>> - addr->minimum = memory32->minimum;
>> - addr->address_length = memory32->address_length;
>> - addr->maximum = addr->minimum + addr->address_length - 1;
>> - return AE_OK;
>> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
>> - fixed_memory32 = &resource->data.fixed_memory32;
>> - addr->resource_type = ACPI_MEMORY_RANGE;
>> - addr->minimum = fixed_memory32->address;
>> - addr->address_length = fixed_memory32->address_length;
>> - addr->maximum = addr->minimum + addr->address_length - 1;
>> - return AE_OK;
>> - case ACPI_RESOURCE_TYPE_ADDRESS16:
>> - case ACPI_RESOURCE_TYPE_ADDRESS32:
>> - case ACPI_RESOURCE_TYPE_ADDRESS64:
>> - status = acpi_resource_to_address64(resource, addr);
>> - if (ACPI_SUCCESS(status) &&
>> - (addr->resource_type = ACPI_MEMORY_RANGE ||
>> - addr->resource_type = ACPI_IO_RANGE) &&
>> - addr->address_length > 0) {
>> - return AE_OK;
>> - }
>> - break;
>> - }
>> - return AE_ERROR;
>> -}
>> -
>> -static acpi_status
>> count_resource(struct acpi_resource *acpi_res, void *data)
>> {
>> struct pci_root_info *info = data;
>> - struct acpi_resource_address64 addr;
>> - acpi_status status;
>> + struct resource res;
>>
>> - status = resource_to_addr(acpi_res, &addr);
>> - if (ACPI_SUCCESS(status))
>> + if (acpi_dev_resource_address_space(acpi_res, &res)
>> + || acpi_dev_resource_memory(acpi_res, &res))
>> info->res_num++;
>> +
>> return AE_OK;
>> }
>>
>> @@ -282,27 +235,18 @@ static acpi_status
>> setup_resource(struct acpi_resource *acpi_res, void *data)
>> {
>> struct pci_root_info *info = data;
>> - struct resource *res;
>> + struct resource *res = &info->res[info->res_num];
>> struct acpi_resource_address64 addr;
>> - acpi_status status;
>> - unsigned long flags;
>> u64 start, orig_end, end;
>>
>> - status = resource_to_addr(acpi_res, &addr);
>> - if (!ACPI_SUCCESS(status))
>> - return AE_OK;
>> + memset(&addr, 0x00, sizeof(addr));
>>
>> - if (addr.resource_type = ACPI_MEMORY_RANGE) {
>> - flags = IORESOURCE_MEM;
>> - if (addr.info.mem.caching = ACPI_PREFETCHABLE_MEMORY)
>> - flags |= IORESOURCE_PREFETCH;
>> - } else if (addr.resource_type = ACPI_IO_RANGE) {
>> - flags = IORESOURCE_IO;
>> - } else
>> + if (!(acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
>> + || acpi_dev_resource_memory(acpi_res, res)))
>> return AE_OK;
>>
>> - start = addr.minimum + addr.translation_offset;
>> - orig_end = end = addr.maximum + addr.translation_offset;
>> + start = res->start;
>> + orig_end = end = res->end;
>>
>> /* Exclude non-addressable range or non-addressable portion of range */
>> end = min(end, (u64)iomem_resource.end);
>> @@ -310,6 +254,7 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>> dev_info(&info->bridge->dev,
>> "host bridge window [%#llx-%#llx] "
>> "(ignored, not CPU addressable)\n", start, orig_end);
>> + memset(&info->res[info->res_num], 0x00, sizeof(*res));
>> return AE_OK;
>> } else if (orig_end != end) {
>> dev_info(&info->bridge->dev,
>> @@ -318,11 +263,9 @@ setup_resource(struct acpi_resource *acpi_res, void *data)
>> start, orig_end, end + 1, orig_end);
>> }
>>
>> - res = &info->res[info->res_num];
>> res->name = info->name;
>> - res->flags = flags;
>> - res->start = start;
>> res->end = end;
>> +
>> info->res_offset[info->res_num] = addr.translation_offset;
>> info->res_num++;
>>
>> --
>> 1.8.2.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions
2013-09-06 16:01 ` Lan Tianyu
@ 2013-09-06 16:10 ` Bjorn Helgaas
2013-09-06 16:35 ` Lan Tianyu
0 siblings, 1 reply; 4+ messages in thread
From: Bjorn Helgaas @ 2013-09-06 16:10 UTC (permalink / raw)
To: Lan Tianyu
Cc: Len Brown, Rafael J. Wysocki, Yinghai Lu,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Luck, linux-ia64@vger.kernel.org
On Fri, Sep 6, 2013 at 10:01 AM, Lan Tianyu <tianyu.lan@intel.com> wrote:
> On 09/06/2013 11:36 AM, Bjorn Helgaas wrote:
>> Please make corresponding changes to arch/ia64/pci/pci.c so that these
>> paths remain as similar as possible. There's quite a bit of
>> similarity between this x86 and ia64 code, and it would be nice to
>> unify them more when possible.
>>
>
> OK. Actually, I have such plan. I will do that if there is no objection on
> this patchset.
Great, I'm glad to hear that! I'm not sure whether you mean "after
this patchset is accepted" or "as part of this patchset if it seems a
reasonable path." I vote for the latter, because if we put in the
parts people care about, i.e., x86, the rest seems to never happen.
That's not surprising; whose manager will approve extra time to work
on an arch that's not on their critical path? But in my opinion,
doing just x86 is only doing half the job, and we have to do the whole
thing if we want to keep Linux maintainable in the future.
Bjorn
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions
2013-09-06 16:10 ` Bjorn Helgaas
@ 2013-09-06 16:35 ` Lan Tianyu
0 siblings, 0 replies; 4+ messages in thread
From: Lan Tianyu @ 2013-09-06 16:35 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Len Brown, Rafael J. Wysocki, Yinghai Lu,
linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
Tony Luck, linux-ia64@vger.kernel.org
On 09/06/2013 12:10 PM, Bjorn Helgaas wrote:
> On Fri, Sep 6, 2013 at 10:01 AM, Lan Tianyu <tianyu.lan@intel.com> wrote:
>> On 09/06/2013 11:36 AM, Bjorn Helgaas wrote:
>
>>> Please make corresponding changes to arch/ia64/pci/pci.c so that these
>>> paths remain as similar as possible. There's quite a bit of
>>> similarity between this x86 and ia64 code, and it would be nice to
>>> unify them more when possible.
>>>
>>
>> OK. Actually, I have such plan. I will do that if there is no objection on
>> this patchset.
>
> Great, I'm glad to hear that! I'm not sure whether you mean "after
> this patchset is accepted" or "as part of this patchset if it seems a
> reasonable path." I vote for the latter, because if we put in the
> parts people care about, i.e., x86, the rest seems to never happen.
> That's not surprising; whose manager will approve extra time to work
> on an arch that's not on their critical path? But in my opinion,
> doing just x86 is only doing half the job, and we have to do the whole
> thing if we want to keep Linux maintainable in the future.
I mean the later. :).
Yes, Linux maintainable is very important.
My plan is to find all such cases of converting ACPI resource to generic
resource but not using ACPI resource function and rework them.
>
> Bjorn
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-06 16:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1378477486-8758-1-git-send-email-tianyu.lan@intel.com>
[not found] ` <1378477486-8758-5-git-send-email-tianyu.lan@intel.com>
2013-09-06 15:36 ` [RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI resource functions Bjorn Helgaas
2013-09-06 16:01 ` Lan Tianyu
2013-09-06 16:10 ` Bjorn Helgaas
2013-09-06 16:35 ` Lan Tianyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).