Patchwork [v4,28/28] PCI, x86, ACPI: get ioapic address from acpi device

login
register
mail settings
Submitter Lan Tianyu
Date Aug. 26, 2013, 3:46 p.m.
Message ID <CAOLK0pxxAFBiDTac_5yPYO7X96Y4Tnd4kjwZ-Wvk1mZ3bD-w2A@mail.gmail.com>
Download mbox | patch
Permalink /patch/269917/
State Not Applicable
Headers show

Comments

Lan Tianyu - Aug. 26, 2013, 3:46 p.m.
2013/8/24 Yinghai Lu <yinghai@kernel.org>:
> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>
>>>>
>>>> Hi Rui&Yinghai:
>>>>            How about using the following code to translate struct
>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>
>>>>          if (acpi_dev_resource_address_space(...)
>>>>                 || acpi_dev_resource_memory(..))
>>>>               return AE_OK;
>>>
>>> Yest, that could be better, will update that.
>>>
>>> Also can you submit patch that will use that in res_to_addr of
>>> arch/x86/pci/acpi.c?
>>
>> looks acpi_dev_resource_address_space... does not handle
>> PREFTCH and translation offset.
>>
>> So now i have to use res_to_addr alike one.
>
> Raphael,
>
> Maybe we should move resource_to_addr to acpi generic.
>
> Please check if you are ok with attached.
>
> Thanks
>
> Yinghai

Hi Rafael & Yinghai:
            I wrote 4 proposal patches to try to make acpi resource
functions to replace
 resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
resource_to_addr()
does most work which acpi resource functions have done. So please have
a look. Thanks.
The following patches just passed through compiling test.

Patch 1
-------------------
commit 1800bc9dda7318314607fbae7afda8be38e056dc
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 14:40:39 2013 +0800

    ACPI/Resource: Add setting PREFETCH flag support
Yinghai Lu - Aug. 27, 2013, 1:03 a.m.
On Mon, Aug 26, 2013 at 8:46 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
> 2013/8/24 Yinghai Lu <yinghai@kernel.org>:
>> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>>
>>>>>
>>>>> Hi Rui&Yinghai:
>>>>>            How about using the following code to translate struct
>>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>>
>>>>>          if (acpi_dev_resource_address_space(...)
>>>>>                 || acpi_dev_resource_memory(..))
>>>>>               return AE_OK;
>>>>
>>>> Yest, that could be better, will update that.
>>>>
>>>> Also can you submit patch that will use that in res_to_addr of
>>>> arch/x86/pci/acpi.c?
>>>
>>> looks acpi_dev_resource_address_space... does not handle
>>> PREFTCH and translation offset.
>>>
>>> So now i have to use res_to_addr alike one.
>>
>> Raphael,
>>
>> Maybe we should move resource_to_addr to acpi generic.
>>
>> Please check if you are ok with attached.
>>
>> Thanks
>>
>> Yinghai
>
> Hi Rafael & Yinghai:
>             I wrote 4 proposal patches to try to make acpi resource
> functions to replace
>  resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
> resource_to_addr()
> does most work which acpi resource functions have done. So please have
> a look. Thanks.
> The following patches just passed through compiling test.

Thanks for doing that.

Overall it's Good to me.

some comments inline.

>
> Patch 1
> -------------------
> commit 1800bc9dda7318314607fbae7afda8be38e056dc
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 14:40:39 2013 +0800
>
>     ACPI/Resource: Add setting PREFETCH flag support
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index b7201fc..23a560b 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -35,7 +35,7 @@
>  #endif
>
>  static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
> -                        bool window)
> +                        bool window, bool prefetchable)
>  {
>      unsigned long flags = IORESOURCE_MEM;
>
> @@ -48,6 +48,9 @@ static unsigned long acpi_dev_memresource_flags(u64
> len, u8 write_protect,
>      if (window)
>          flags |= IORESOURCE_WINDOW;
>
> +    if (prefetchable)
> +        flags |= IORESOURCE_PREFETCH;
> +
>      return flags;
>  }
>
> @@ -56,7 +59,8 @@ static void acpi_dev_get_memresource(struct resource
> *res, u64 start, u64 len,
>  {
>      res->start = start;
>      res->end = start + len - 1;
> -    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
> +    res->flags = acpi_dev_memresource_flags(len, write_protect,
> +                        false, false);
>  }

passing write_protect, window then pref looks silly.

may use | IO_REOURCE_... etc outside of acpi_dev_memresource_flags directly.

>
>  /**
> @@ -175,7 +179,7 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>  {
>      acpi_status status;
>      struct acpi_resource_address64 addr;
> -    bool window;
> +    bool window, prefetchable;
>      u64 len;
>      u8 io_decode;
>
> @@ -199,9 +203,11 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>      switch(addr.resource_type) {
>      case ACPI_MEMORY_RANGE:
>          len = addr.maximum - addr.minimum + 1;
> +        prefetchable =
> +            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>          res->flags = acpi_dev_memresource_flags(len,
>                          addr.info.mem.write_protect,
> -                        window);
> +                        window, prefetchable);
>          break;
>      case ACPI_IO_RANGE:
>          io_decode = addr.granularity == 0xfff ?
> @@ -252,7 +258,7 @@ bool acpi_dev_resource_ext_address_space(struct
> acpi_resource *ares,
>          len = ext_addr->maximum - ext_addr->minimum + 1;
>          res->flags = acpi_dev_memresource_flags(len,
>                      ext_addr->info.mem.write_protect,
> -                    window);
> +                    window, false);
>          break;
>      case ACPI_IO_RANGE:
>          io_decode = ext_addr->granularity == 0xfff ?
>
> Patch 2
> -----------
> commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 15:57:14 2013 +0800
>
>     ACPI/Resource: Add address translation support
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 23a560b..439ee44 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -196,8 +196,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) {
>
>
> Patch 3
> ---------------
> commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 21:45:32 2013 +0800
>
>     ACPI: Add new acpi_dev_resource_address_space_with_addr() function
>
>     Add new function which can return the converted struct
> acpi_resource_address64.
> This will be used to get transaction offset in the setup_resource().
>
> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
> index 439ee44..98a6c35 100644
> --- a/drivers/acpi/resource.c
> +++ b/drivers/acpi/resource.c
> @@ -174,11 +174,11 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
>   * 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,
> +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, prefetchable;
>      u64 len;
>      u8 io_decode;
> @@ -192,28 +192,28 @@ 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;
>          prefetchable =
> -            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
> +            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>          res->flags = acpi_dev_memresource_flags(len,
> -                        addr.info.mem.write_protect,
> +                        addr->info.mem.write_protect,
>                          window, prefetchable);
>          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:
> @@ -225,6 +225,16 @@ bool acpi_dev_resource_address_space(struct
> acpi_resource *ares,
>
>      return true;
>  }
> +EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
> +
> +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);
>
> Patch 4
> --------------
> commit a8733a1da756a257d55e68994268174b84b33670
> Author: Lan Tianyu <tianyu.lan@intel.com>
> Date:   Mon Aug 26 22:53:38 2013 +0800
>
>     X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions
>
> 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)))

acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)

passing extra addr, just for translation_offset?

maybe pass res_offset directly?




>          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;

Yinghai
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lan Tianyu - Aug. 27, 2013, 2:56 p.m.
2013/8/27 Yinghai Lu <yinghai@kernel.org>:
> On Mon, Aug 26, 2013 at 8:46 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>> 2013/8/24 Yinghai Lu <yinghai@kernel.org>:
>>> On Fri, Aug 23, 2013 at 12:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Fri, Aug 23, 2013 at 11:38 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>>> On Fri, Aug 23, 2013 at 8:34 AM, Lan Tianyu <lantianyu1986@gmail.com> wrote:
>>>>>>> I worked around the problem by replacing acpi_resource_to_address64()
>>>>>>> with resource_to_addr().  But resource_to_addr() is a static function
>>>>>>> in arch/x86/pci/acpi.c, not very convenient to use. Here's what I did:
>>>>>>>
>>>>>>
>>>>>> Hi Rui&Yinghai:
>>>>>>            How about using the following code to translate struct
>>>>>>  acpi_resource to struct resouce in this setup_res()?
>>>>>>
>>>>>>          if (acpi_dev_resource_address_space(...)
>>>>>>                 || acpi_dev_resource_memory(..))
>>>>>>               return AE_OK;
>>>>>
>>>>> Yest, that could be better, will update that.
>>>>>
>>>>> Also can you submit patch that will use that in res_to_addr of
>>>>> arch/x86/pci/acpi.c?
>>>>
>>>> looks acpi_dev_resource_address_space... does not handle
>>>> PREFTCH and translation offset.
>>>>
>>>> So now i have to use res_to_addr alike one.
>>>
>>> Raphael,
>>>
>>> Maybe we should move resource_to_addr to acpi generic.
>>>
>>> Please check if you are ok with attached.
>>>
>>> Thanks
>>>
>>> Yinghai
>>
>> Hi Rafael & Yinghai:
>>             I wrote 4 proposal patches to try to make acpi resource
>> functions to replace
>>  resource_to_addr() in the arch/x86/pci/acpi.c. From my opinion,
>> resource_to_addr()
>> does most work which acpi resource functions have done. So please have
>> a look. Thanks.
>> The following patches just passed through compiling test.
>
> Thanks for doing that.
>
> Overall it's Good to me.
>
> some comments inline.
>
>>
>> Patch 1
>> -------------------
>> commit 1800bc9dda7318314607fbae7afda8be38e056dc
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 14:40:39 2013 +0800
>>
>>     ACPI/Resource: Add setting PREFETCH flag support
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index b7201fc..23a560b 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -35,7 +35,7 @@
>>  #endif
>>
>>  static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
>> -                        bool window)
>> +                        bool window, bool prefetchable)
>>  {
>>      unsigned long flags = IORESOURCE_MEM;
>>
>> @@ -48,6 +48,9 @@ static unsigned long acpi_dev_memresource_flags(u64
>> len, u8 write_protect,
>>      if (window)
>>          flags |= IORESOURCE_WINDOW;
>>
>> +    if (prefetchable)
>> +        flags |= IORESOURCE_PREFETCH;
>> +
>>      return flags;
>>  }
>>
>> @@ -56,7 +59,8 @@ static void acpi_dev_get_memresource(struct resource
>> *res, u64 start, u64 len,
>>  {
>>      res->start = start;
>>      res->end = start + len - 1;
>> -    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
>> +    res->flags = acpi_dev_memresource_flags(len, write_protect,
>> +                        false, false);
>>  }
>
> passing write_protect, window then pref looks silly.
>
> may use | IO_REOURCE_... etc outside of acpi_dev_memresource_flags directly.
>

Yes, this looks better.

>>
>>  /**
>> @@ -175,7 +179,7 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>  {
>>      acpi_status status;
>>      struct acpi_resource_address64 addr;
>> -    bool window;
>> +    bool window, prefetchable;
>>      u64 len;
>>      u8 io_decode;
>>
>> @@ -199,9 +203,11 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>      switch(addr.resource_type) {
>>      case ACPI_MEMORY_RANGE:
>>          len = addr.maximum - addr.minimum + 1;
>> +        prefetchable =
>> +            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>>          res->flags = acpi_dev_memresource_flags(len,
>>                          addr.info.mem.write_protect,
>> -                        window);
>> +                        window, prefetchable);
>>          break;
>>      case ACPI_IO_RANGE:
>>          io_decode = addr.granularity == 0xfff ?
>> @@ -252,7 +258,7 @@ bool acpi_dev_resource_ext_address_space(struct
>> acpi_resource *ares,
>>          len = ext_addr->maximum - ext_addr->minimum + 1;
>>          res->flags = acpi_dev_memresource_flags(len,
>>                      ext_addr->info.mem.write_protect,
>> -                    window);
>> +                    window, false);
>>          break;
>>      case ACPI_IO_RANGE:
>>          io_decode = ext_addr->granularity == 0xfff ?
>>
>> Patch 2
>> -----------
>> commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 15:57:14 2013 +0800
>>
>>     ACPI/Resource: Add address translation support
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 23a560b..439ee44 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -196,8 +196,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) {
>>
>>
>> Patch 3
>> ---------------
>> commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 21:45:32 2013 +0800
>>
>>     ACPI: Add new acpi_dev_resource_address_space_with_addr() function
>>
>>     Add new function which can return the converted struct
>> acpi_resource_address64.
>> This will be used to get transaction offset in the setup_resource().
>>
>> diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
>> index 439ee44..98a6c35 100644
>> --- a/drivers/acpi/resource.c
>> +++ b/drivers/acpi/resource.c
>> @@ -174,11 +174,11 @@ EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
>>   * 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,
>> +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, prefetchable;
>>      u64 len;
>>      u8 io_decode;
>> @@ -192,28 +192,28 @@ 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;
>>          prefetchable =
>> -            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>> +            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
>>          res->flags = acpi_dev_memresource_flags(len,
>> -                        addr.info.mem.write_protect,
>> +                        addr->info.mem.write_protect,
>>                          window, prefetchable);
>>          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:
>> @@ -225,6 +225,16 @@ bool acpi_dev_resource_address_space(struct
>> acpi_resource *ares,
>>
>>      return true;
>>  }
>> +EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
>> +
>> +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);
>>
>> Patch 4
>> --------------
>> commit a8733a1da756a257d55e68994268174b84b33670
>> Author: Lan Tianyu <tianyu.lan@intel.com>
>> Date:   Mon Aug 26 22:53:38 2013 +0800
>>
>>     X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions
>>
>> 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)))
>
> acpi_dev_resource_address_space_with_addr(acpi_res, &addr, res)
>
> passing extra addr, just for translation_offset?
>

For this case,  it's "Yes", . For some other cases(e.g
arch/ia64/pci/pci.c add_window()),
whole addr maybe necessary. To be more general, I hope it can return more infos

> maybe pass res_offset directly?
>
>
>
>
>>          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;
>
> Yinghai

Patch

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index b7201fc..23a560b 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -35,7 +35,7 @@ 
 #endif

 static unsigned long acpi_dev_memresource_flags(u64 len, u8 write_protect,
-                        bool window)
+                        bool window, bool prefetchable)
 {
     unsigned long flags = IORESOURCE_MEM;

@@ -48,6 +48,9 @@  static unsigned long acpi_dev_memresource_flags(u64
len, u8 write_protect,
     if (window)
         flags |= IORESOURCE_WINDOW;

+    if (prefetchable)
+        flags |= IORESOURCE_PREFETCH;
+
     return flags;
 }

@@ -56,7 +59,8 @@  static void acpi_dev_get_memresource(struct resource
*res, u64 start, u64 len,
 {
     res->start = start;
     res->end = start + len - 1;
-    res->flags = acpi_dev_memresource_flags(len, write_protect, false);
+    res->flags = acpi_dev_memresource_flags(len, write_protect,
+                        false, false);
 }

 /**
@@ -175,7 +179,7 @@  bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
 {
     acpi_status status;
     struct acpi_resource_address64 addr;
-    bool window;
+    bool window, prefetchable;
     u64 len;
     u8 io_decode;

@@ -199,9 +203,11 @@  bool acpi_dev_resource_address_space(struct
acpi_resource *ares,
     switch(addr.resource_type) {
     case ACPI_MEMORY_RANGE:
         len = addr.maximum - addr.minimum + 1;
+        prefetchable =
+            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
         res->flags = acpi_dev_memresource_flags(len,
                         addr.info.mem.write_protect,
-                        window);
+                        window, prefetchable);
         break;
     case ACPI_IO_RANGE:
         io_decode = addr.granularity == 0xfff ?
@@ -252,7 +258,7 @@  bool acpi_dev_resource_ext_address_space(struct
acpi_resource *ares,
         len = ext_addr->maximum - ext_addr->minimum + 1;
         res->flags = acpi_dev_memresource_flags(len,
                     ext_addr->info.mem.write_protect,
-                    window);
+                    window, false);
         break;
     case ACPI_IO_RANGE:
         io_decode = ext_addr->granularity == 0xfff ?

Patch 2
-----------
commit a3ff8345ffb51885b216ba0ae231252cd88d4e76
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 15:57:14 2013 +0800

    ACPI/Resource: Add address translation support

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 23a560b..439ee44 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -196,8 +196,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) {


Patch 3
---------------
commit 68bfefe619b49baa5d372e0664a6cb6d5138fad1
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 21:45:32 2013 +0800

    ACPI: Add new acpi_dev_resource_address_space_with_addr() function

    Add new function which can return the converted struct
acpi_resource_address64.
This will be used to get transaction offset in the setup_resource().

diff --git a/drivers/acpi/resource.c b/drivers/acpi/resource.c
index 439ee44..98a6c35 100644
--- a/drivers/acpi/resource.c
+++ b/drivers/acpi/resource.c
@@ -174,11 +174,11 @@  EXPORT_SYMBOL_GPL(acpi_dev_resource_io);
  * 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,
+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, prefetchable;
     u64 len;
     u8 io_decode;
@@ -192,28 +192,28 @@  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;
         prefetchable =
-            addr.info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
+            addr->info.mem.caching == ACPI_PREFETCHABLE_MEMORY;
         res->flags = acpi_dev_memresource_flags(len,
-                        addr.info.mem.write_protect,
+                        addr->info.mem.write_protect,
                         window, prefetchable);
         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:
@@ -225,6 +225,16 @@  bool acpi_dev_resource_address_space(struct
acpi_resource *ares,

     return true;
 }
+EXPORT_SYMBOL_GPL(acpi_dev_resource_address_space_with_addr);
+
+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);

Patch 4
--------------
commit a8733a1da756a257d55e68994268174b84b33670
Author: Lan Tianyu <tianyu.lan@intel.com>
Date:   Mon Aug 26 22:53:38 2013 +0800

    X86/PCI/ACPI: Rework setup_resource() via functions acpi resource functions

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++;