This patchset is to add memory prefecth flag setting and address
translation support in the ACPI resource function. Convert PCI
root bridge's ACPI resource to generic resource via ACPI resource
functions.
PATCH 3 is to add new function acpi_dev_resource_address_space_with_addr()
to meet requirement that ACPI address space info is still needed after
being converted to generic resource.
[RFC PATCH 1/4] ACPI/Resource: Add memory prefetch check support
[RFC PATCH 2/4] ACPI/Resource: Add address translation support
[RFC PATCH 3/4] ACPI: Add new acpi_dev_resource_address_space_with_addr()
[RFC PATCH 4/4] X86/PCI/ACPI: Rework setup_resource() via functions ACPI
This patch is to check mem address space's acpi resource caching ability
and set prefetch flag of struct resource if it's prefetchable.
Signed-off-by: Lan Tianyu <[email protected]>
---
drivers/acpi/resource.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index b7201fc..929f416 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -202,6 +202,9 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
res->flags = acpi_dev_memresource_flags(len,
addr.info.mem.write_protect,
window);
+
+ if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
+ res->flags |= IORESOURCE_PREFETCH;
break;
case ACPI_IO_RANGE:
io_decode = addr.granularity == 0xfff ?
--
1.8.2.1
According ACPI 5.0 spec Section 19.1.8
"For bridges, translate addresses across the bridge, this is the
offset that must be added to the address on the secondary side
to obtain the address on the primary side. Non-bridge devices
must list 0."
This patch is to add address translation offset to the start/end
of struct resource in the acpi_dev_resource_address_space().
Further more, non-bridge device's translation_offset should 0.
So this change will affect other devices.
Signed-off-by: Lan Tianyu <[email protected]>
---
drivers/acpi/resource.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 929f416..84bc3db 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -192,8 +192,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
if (ACPI_FAILURE(status))
return true;
- res->start = addr.minimum;
- res->end = addr.maximum;
+ res->start = addr.minimum + addr.translation_offset;
+ res->end = addr.maximum + addr.translation_offset;
window = addr.producer_consumer == ACPI_PRODUCER;
switch(addr.resource_type) {
--
1.8.2.1
Make acpi_dev_resource_address_space() to accept struct
acpi_resource_address64 as param and rename it to *_with_addr.
This is for some cases that acpi address info is also needed
after convert from acpi resouce to generic resource.
Add acpi_devi_resource_addres_space() again as a wrapper of new
function for original users.
Signed-off-by: Lan Tianyu <[email protected]>
---
drivers/acpi/resource.c | 53 ++++++++++++++++++++++++++++++++++---------------
include/linux/acpi.h | 3 +++
2 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 84bc3db..76da28b 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -162,19 +162,21 @@ bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res)
EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
/**
- * acpi_dev_resource_address_space - Extract ACPI address space information.
+ * acpi_dev_resource_address_space_with_addr - Extract ACPI address space information.
* @ares: Input ACPI resource object.
+ * @addr: Output ACPI resource address64 space object.
* @res: Output generic resource object.
*
* Check if the given ACPI resource object represents an address space resource
- * and if that's the case, use the information in it to populate the generic
- * resource object pointed to by @res.
+ * and if that's the case, convert it to ACPI resource address64 space object
+ * pointed to by @addr and use the information to populate the generic resource
+ * object pointed to by @re.
*/
-bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
struct resource *res)
{
acpi_status status;
- struct acpi_resource_address64 addr;
bool window;
u64 len;
u8 io_decode;
@@ -188,29 +190,29 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
return false;
}
- status = acpi_resource_to_address64(ares, &addr);
+ status = acpi_resource_to_address64(ares, addr);
if (ACPI_FAILURE(status))
return true;
- res->start = addr.minimum + addr.translation_offset;
- res->end = addr.maximum + addr.translation_offset;
- window = addr.producer_consumer == ACPI_PRODUCER;
+ res->start = addr->minimum + addr->translation_offset;
+ res->end = addr->maximum + addr->translation_offset;
+ window = addr->producer_consumer == ACPI_PRODUCER;
- switch(addr.resource_type) {
+ switch (addr->resource_type) {
case ACPI_MEMORY_RANGE:
- len = addr.maximum - addr.minimum + 1;
+ len = addr->maximum - addr->minimum + 1;
res->flags = acpi_dev_memresource_flags(len,
- addr.info.mem.write_protect,
+ addr->info.mem.write_protect,
window);
- if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
+ if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
res->flags |= IORESOURCE_PREFETCH;
break;
case ACPI_IO_RANGE:
- io_decode = addr.granularity == 0xfff ?
+ io_decode = addr->granularity == 0xfff ?
ACPI_DECODE_10 : ACPI_DECODE_16;
- res->flags = acpi_dev_ioresource_flags(addr.minimum,
- addr.maximum,
+ res->flags = acpi_dev_ioresource_flags(addr->minimum,
+ addr->maximum,
io_decode, window);
break;
case ACPI_BUS_NUMBER_RANGE:
@@ -222,6 +224,25 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
return true;
}
+EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
+
+/**
+ * acpi_dev_resource_address_space - Extract ACPI address space information.
+ * @ares: Input ACPI resource object.
+ * @res: Output generic resource object.
+ *
+ * Check if the given ACPI resource object represents an address space resource
+ * and if that's the case, use the information in it to populate the generic
+ * resource object pointed to by @res.
+ */
+bool acpi_dev_resource_address_space(struct acpi_resource *ares,
+ struct resource *res)
+{
+ struct acpi_resource_address64 addr;
+
+ return acpi_dev_resource_address_space_with_addr(ares, &addr,
+ res);
+}
EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
/**
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index a5db4ae..9f5c0d5 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -260,6 +260,9 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res);
bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
bool acpi_dev_resource_address_space(struct acpi_resource *ares,
struct resource *res);
+bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res);
bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
struct resource *res);
unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
--
1.8.2.1
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 <[email protected]>
---
arch/x86/pci/acpi.c | 81 ++++++++---------------------------------------------
1 file changed, 12 insertions(+), 69 deletions(-)
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
[+cc Tony, linux-ia64]
On Fri, Sep 6, 2013 at 8:24 AM, Lan Tianyu <[email protected]> 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 <[email protected]>
> ---
> 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
>
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 <[email protected]> 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 <[email protected]>
>> ---
>> 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
>>
On Fri, Sep 6, 2013 at 10:01 AM, Lan Tianyu <[email protected]> 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
On 09/06/2013 12:10 PM, Bjorn Helgaas wrote:
> On Fri, Sep 6, 2013 at 10:01 AM, Lan Tianyu <[email protected]> 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
>
On Friday, September 06, 2013 10:24:43 AM Lan Tianyu wrote:
> This patch is to check mem address space's acpi resource caching ability
> and set prefetch flag of struct resource if it's prefetchable.
>
> Signed-off-by: Lan Tianyu <[email protected]>
Looks good to me.
Thanks,
Rafael
> ---
> drivers/acpi/resource.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index b7201fc..929f416 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -202,6 +202,9 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> res->flags = acpi_dev_memresource_flags(len,
> addr.info.mem.write_protect,
> window);
> +
> + if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> + res->flags |= IORESOURCE_PREFETCH;
> break;
> case ACPI_IO_RANGE:
> io_decode = addr.granularity == 0xfff ?
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Friday, September 06, 2013 10:24:44 AM Lan Tianyu wrote:
> According ACPI 5.0 spec Section 19.1.8
> "For bridges, translate addresses across the bridge, this is the
> offset that must be added to the address on the secondary side
> to obtain the address on the primary side. Non-bridge devices
> must list 0."
Can you please have a look into the previous versions of the spec and double
check that this change won't confuse systems that implement them?
Otherwise it looks OK to me.
Thanks,
Rafael
> This patch is to add address translation offset to the start/end
> of struct resource in the acpi_dev_resource_address_space().
> Further more, non-bridge device's translation_offset should 0.
> So this change will affect other devices.
>
>
> Signed-off-by: Lan Tianyu <[email protected]>
> ---
> drivers/acpi/resource.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 929f416..84bc3db 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -192,8 +192,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> if (ACPI_FAILURE(status))
> return true;
>
> - res->start = addr.minimum;
> - res->end = addr.maximum;
> + res->start = addr.minimum + addr.translation_offset;
> + res->end = addr.maximum + addr.translation_offset;
> window = addr.producer_consumer == ACPI_PRODUCER;
>
> switch(addr.resource_type) {
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Friday, September 06, 2013 10:24:45 AM Lan Tianyu wrote:
> Make acpi_dev_resource_address_space() to accept struct
> acpi_resource_address64 as param and rename it to *_with_addr.
I'd prefer acpi_dev_resource_address_space_full() or something like this.
Apart from this it is fine by me.
Thanks,
Rafael
> This is for some cases that acpi address info is also needed
> after convert from acpi resouce to generic resource.
>
> Add acpi_devi_resource_addres_space() again as a wrapper of new
> function for original users.
>
> Signed-off-by: Lan Tianyu <[email protected]>
> ---
> drivers/acpi/resource.c | 53 ++++++++++++++++++++++++++++++++++---------------
> include/linux/acpi.h | 3 +++
> 2 files changed, 40 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 84bc3db..76da28b 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -162,19 +162,21 @@ bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res)
> EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
>
> /**
> - * acpi_dev_resource_address_space - Extract ACPI address space information.
> + * acpi_dev_resource_address_space_with_addr - Extract ACPI address space information.
> * @ares: Input ACPI resource object.
> + * @addr: Output ACPI resource address64 space object.
> * @res: Output generic resource object.
> *
> * Check if the given ACPI resource object represents an address space resource
> - * and if that's the case, use the information in it to populate the generic
> - * resource object pointed to by @res.
> + * and if that's the case, convert it to ACPI resource address64 space object
> + * pointed to by @addr and use the information to populate the generic resource
> + * object pointed to by @re.
> */
> -bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
> + struct acpi_resource_address64 *addr,
> struct resource *res)
> {
> acpi_status status;
> - struct acpi_resource_address64 addr;
> bool window;
> u64 len;
> u8 io_decode;
> @@ -188,29 +190,29 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> return false;
> }
>
> - status = acpi_resource_to_address64(ares, &addr);
> + status = acpi_resource_to_address64(ares, addr);
> if (ACPI_FAILURE(status))
> return true;
>
> - res->start = addr.minimum + addr.translation_offset;
> - res->end = addr.maximum + addr.translation_offset;
> - window = addr.producer_consumer == ACPI_PRODUCER;
> + res->start = addr->minimum + addr->translation_offset;
> + res->end = addr->maximum + addr->translation_offset;
> + window = addr->producer_consumer == ACPI_PRODUCER;
>
> - switch(addr.resource_type) {
> + switch (addr->resource_type) {
> case ACPI_MEMORY_RANGE:
> - len = addr.maximum - addr.minimum + 1;
> + len = addr->maximum - addr->minimum + 1;
> res->flags = acpi_dev_memresource_flags(len,
> - addr.info.mem.write_protect,
> + addr->info.mem.write_protect,
> window);
>
> - if (addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> + if (addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY)
> res->flags |= IORESOURCE_PREFETCH;
> break;
> case ACPI_IO_RANGE:
> - io_decode = addr.granularity == 0xfff ?
> + io_decode = addr->granularity == 0xfff ?
> ACPI_DECODE_10 : ACPI_DECODE_16;
> - res->flags = acpi_dev_ioresource_flags(addr.minimum,
> - addr.maximum,
> + res->flags = acpi_dev_ioresource_flags(addr->minimum,
> + addr->maximum,
> io_decode, window);
> break;
> case ACPI_BUS_NUMBER_RANGE:
> @@ -222,6 +224,25 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>
> return true;
> }
> +EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
> +
> +/**
> + * acpi_dev_resource_address_space - Extract ACPI address space information.
> + * @ares: Input ACPI resource object.
> + * @res: Output generic resource object.
> + *
> + * Check if the given ACPI resource object represents an address space resource
> + * and if that's the case, use the information in it to populate the generic
> + * resource object pointed to by @res.
> + */
> +bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> + struct resource *res)
> +{
> + struct acpi_resource_address64 addr;
> +
> + return acpi_dev_resource_address_space_with_addr(ares, &addr,
> + res);
> +}
> EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space);
>
> /**
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index a5db4ae..9f5c0d5 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -260,6 +260,9 @@ bool acpi_dev_resource_memory(struct acpi_resource *ares, struct resource *res);
> bool acpi_dev_resource_io(struct acpi_resource *ares, struct resource *res);
> bool acpi_dev_resource_address_space(struct acpi_resource *ares,
> struct resource *res);
> +bool acpi_dev_resource_address_space_with_addr(struct acpi_resource *ares,
> + struct acpi_resource_address64 *addr,
> + struct resource *res);
> bool acpi_dev_resource_ext_address_space(struct acpi_resource *ares,
> struct resource *res);
> unsigned long acpi_dev_irq_flags(u8 triggering, u8 polarity, u8 shareable);
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On Friday, September 06, 2013 10:24:46 AM Lan Tianyu wrote:
> Using ACPI resource functions to convert ACPI resource to generic resource
> instead of resource_to_addr(). Remove resource_to_addr().
Apart from the Bjorn's comment that this should be done for ia64 too,
it looks OK to me.
Thanks,
Rafael
> Signed-off-by: Lan Tianyu <[email protected]>
> ---
> arch/x86/pci/acpi.c | 81 ++++++++---------------------------------------------
> 1 file changed, 12 insertions(+), 69 deletions(-)
>
> 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++;
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
On 2013年09月07日 08:15, Rafael J. Wysocki wrote:
> On Friday, September 06, 2013 10:24:44 AM Lan Tianyu wrote:
>> According ACPI 5.0 spec Section 19.1.8
>> "For bridges, translate addresses across the bridge, this is the
>> offset that must be added to the address on the secondary side
>> to obtain the address on the primary side. Non-bridge devices
>> must list 0."
>
> Can you please have a look into the previous versions of the spec and double
> check that this change won't confuse systems that implement them?
>
Hi Rafael:
I check all versions. This part has existed and not been
changed since ACPI 1.0.
> Otherwise it looks OK to me.
>
> Thanks,
> Rafael
>
>
>> This patch is to add address translation offset to the start/end
>> of struct resource in the acpi_dev_resource_address_space().
>> Further more, non-bridge device's translation_offset should 0.
>> So this change will affect other devices.
>>
>>
>> Signed-off-by: Lan Tianyu <[email protected]>
>> ---
>> drivers/acpi/resource.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 929f416..84bc3db 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -192,8 +192,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>> if (ACPI_FAILURE(status))
>> return true;
>>
>> - res->start = addr.minimum;
>> - res->end = addr.maximum;
>> + res->start = addr.minimum + addr.translation_offset;
>> + res->end = addr.maximum + addr.translation_offset;
>> window = addr.producer_consumer == ACPI_PRODUCER;
>>
>> switch(addr.resource_type) {
>>
--
Best regards
Tianyu Lan
On 09/08/2013 10:18 PM, Lan Tianyu wrote:
> On 2013年09月07日 08:15, Rafael J. Wysocki wrote:
>> On Friday, September 06, 2013 10:24:44 AM Lan Tianyu wrote:
>>> According ACPI 5.0 spec Section 19.1.8
>>> "For bridges, translate addresses across the bridge, this is the
>>> offset that must be added to the address on the secondary side
>>> to obtain the address on the primary side. Non-bridge devices
>>> must list 0."
>>
>> Can you please have a look into the previous versions of the spec and double
>> check that this change won't confuse systems that implement them?
>>
>
> Hi Rafael:
> I check all versions. This part has existed and not been
> changed since ACPI 1.0.
>
Hi Rafael:
Do you have other comments on this?
>> Otherwise it looks OK to me.
>>
>> Thanks,
>> Rafael
>>
>>
>>> This patch is to add address translation offset to the start/end
>>> of struct resource in the acpi_dev_resource_address_space().
>>> Further more, non-bridge device's translation_offset should 0.
>>> So this change will affect other devices.
>>>
>>>
>>> Signed-off-by: Lan Tianyu <[email protected]>
>>> ---
>>> drivers/acpi/resource.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>>> index 929f416..84bc3db 100644
>>> --- a/drivers/acpi/resource.c
>>> +++ b/drivers/acpi/resource.c
>>> @@ -192,8 +192,8 @@ bool acpi_dev_resource_address_space(struct acpi_resource *ares,
>>> if (ACPI_FAILURE(status))
>>> return true;
>>>
>>> - res->start = addr.minimum;
>>> - res->end = addr.maximum;
>>> + res->start = addr.minimum + addr.translation_offset;
>>> + res->end = addr.maximum + addr.translation_offset;
>>> window = addr.producer_consumer == ACPI_PRODUCER;
>>>
>>> switch(addr.resource_type) {
>>>
>
>
On Wednesday, September 11, 2013 10:26:37 AM Lan Tianyu wrote:
> On 09/08/2013 10:18 PM, Lan Tianyu wrote:
> > On 2013年09月07日 08:15, Rafael J. Wysocki wrote:
> >> On Friday, September 06, 2013 10:24:44 AM Lan Tianyu wrote:
> >>> According ACPI 5.0 spec Section 19.1.8
> >>> "For bridges, translate addresses across the bridge, this is the
> >>> offset that must be added to the address on the secondary side
> >>> to obtain the address on the primary side. Non-bridge devices
> >>> must list 0."
> >>
> >> Can you please have a look into the previous versions of the spec and double
> >> check that this change won't confuse systems that implement them?
> >>
> >
> > Hi Rafael:
> > I check all versions. This part has existed and not been
> > changed since ACPI 1.0.
> >
>
> Hi Rafael:
> Do you have other comments on this?
Well, why would I? You're saying that you've verified it and that's OK.
Thanks,
Rafael