Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Thu, Oct 31, 2013 at 10:26:03AM +0800, Lan Tianyu wrote:
> On 2013年10月31日 00:23, Bjorn Helgaas wrote:
> > On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu wrote:
> >> On 2013年10月29日 01:32, Bjorn Helgaas wrote:
> >>>
> >>> On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu wrote:
>
> On 10/24/2013 06:39 AM, Bjorn Helgaas wrote:
> >
> > On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote:
> >>
> >>
> >> On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
> >>>
> >>>
> >>> I wonder if it would make sense to make
> >>> acpi_dev_resource_address_space() ignore addr.translation_offset for
> >>> IO resources. Or maybe ignore it if the _TTP (type translation) bit
> >>> is set?
> >>
> >>
> >>
> >> I wonder why current code doesn't check _TTP? The code in the
> >> add_io_space() seems to think _TTP is always set, right?
> >
> >
> > I think it's an oversight, and you should fix it. I suggest that you
> > ignore the _TRA value when _TTP is set. Obviously this only applies
> > to I/O port resources, since _TTP is only defined in the I/O Resource
> > Flag (Table 6-185 in ACPI 5.0 spec).
>
>
> _TTP is also defined in the Memory Resource flag, Please have a look at
> Table 6-184 in the ACPI 5.0 Spec.
> >>>
> >>>
> >>> Yes, you're right. That would be for a host bridge that converts I/O
> >>> on the primary (upstream) side of the bridge to memory on the PCI
> >>> side. I've never seen such a bridge, and I can't really imagine why
> >>> anybody would do that. But I guess you should be able to safely
> >>> ignore _TRA when _TTP is set in either a MEM or IO descriptor, because
> >>> the same reasoning should apply to both.
> >>>
> I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting
> mean the resource is IO on the primary side and also IO on the secondary
> side.
> >>>
> >>>
> >>> If _TTP is not set, I guess you would apply _TRA. That's what you
> >>> already do for MEM descriptors, and think you should just do the same
> >>> for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0
> >>> is rare for IO descriptors, but I suppose it could happen.
> >>
> >>
> >> Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The
> >> only reason for this case I think of is that the IO resource offsets on
> >> the prime bus and second bus are different. In this case, we still need
> >> to pass _TRA to new_space() and the finial resource->start still should be
> >> acpi_resource->min + offset returned by add_io_space(), right?
> >
> > No, I don't think so. If the "phys_base" argument to new_space() is
> > non-zero, it is the base of an MMIO region that needs to be
> > ioremapped. This is handling the _TTP=1 case, where the MMIO region
> > is translated by the bridge into an IO region on PCI.
> >
> > If _TTP=0, the region is IO on both the upstream and downstream sides
> > of the host bridge, and we don't want to ioremap a new MMIO region for
> > it. It might be part of the "legacy I/O port space," but that's
> > already covered elsewhere.
> >
> > I don't think we need to add special handling for the _TTP=0 and _TRA
> > != 0 case because I don't think it exists in the field. If and when
> > it *does* exist, we'll know what to do. In the meantime, it should
> > look just like the MEM path.
>
>
> OK. I get it. acpi_dev_resource_address_space() will only apply _TRA to
> resource ->start and ->end for both mem and io resource when _TTP=0. In
> the add_window(), the offset returned by add_io_space() will be added
> directly to ->start and ->end.
>
> add_window() {
> ...
> if (resource->flags & IORESOURCE_MEM) {
> root = _resource;
> offset = addr.translation_offset;
I can wait for your patch to see the whole thing, but I would expect
"offset = 0" here. For MEM resources, the arch code should not need to
look inside "addr" at all.
> } else if (resource->flags & IORESOURCE_IO) {
> root = _resource;
>
> offset = add_io_space(info, );
> if (offset == ~0)
> return AE_OK;
>
> resource->start += offset;
> resource->end += offset;
> } else
> return AE_OK;
>
> ...
> }
>
> >
> >> If yes, I think _TRA can't be applied to IO resource in the
> >> acpi_dev_resource_address_space() regardless of the value of _TTP.
> >>
> >> BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table
> >> 6-185). The add_io_space() doesn't check _TTP when set sparse. So this
> >> should be corrected?
> >
> > Sure, I'm OK with this. It's possible we could trip over a BIOS bug
> > where _TRS=1 but _TTP=0, but I think the risk is low because only
> > large ia64 boxes would use this, and there aren't very many of those.
> >
>
> Ok. I will add a check for _TTP before setting sparse. Something likes this.
>
>
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Thu, Oct 31, 2013 at 10:26:03AM +0800, Lan Tianyu wrote:
On 2013年10月31日 00:23, Bjorn Helgaas wrote:
On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu tianyu@intel.com wrote:
On 2013年10月29日 01:32, Bjorn Helgaas wrote:
On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu tianyu@intel.com wrote:
On 10/24/2013 06:39 AM, Bjorn Helgaas wrote:
On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote:
On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
I wonder if it would make sense to make
acpi_dev_resource_address_space() ignore addr.translation_offset for
IO resources. Or maybe ignore it if the _TTP (type translation) bit
is set?
I wonder why current code doesn't check _TTP? The code in the
add_io_space() seems to think _TTP is always set, right?
I think it's an oversight, and you should fix it. I suggest that you
ignore the _TRA value when _TTP is set. Obviously this only applies
to I/O port resources, since _TTP is only defined in the I/O Resource
Flag (Table 6-185 in ACPI 5.0 spec).
_TTP is also defined in the Memory Resource flag, Please have a look at
Table 6-184 in the ACPI 5.0 Spec.
Yes, you're right. That would be for a host bridge that converts I/O
on the primary (upstream) side of the bridge to memory on the PCI
side. I've never seen such a bridge, and I can't really imagine why
anybody would do that. But I guess you should be able to safely
ignore _TRA when _TTP is set in either a MEM or IO descriptor, because
the same reasoning should apply to both.
I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting
mean the resource is IO on the primary side and also IO on the secondary
side.
If _TTP is not set, I guess you would apply _TRA. That's what you
already do for MEM descriptors, and think you should just do the same
for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0
is rare for IO descriptors, but I suppose it could happen.
Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The
only reason for this case I think of is that the IO resource offsets on
the prime bus and second bus are different. In this case, we still need
to pass _TRA to new_space() and the finial resource-start still should be
acpi_resource-min + offset returned by add_io_space(), right?
No, I don't think so. If the phys_base argument to new_space() is
non-zero, it is the base of an MMIO region that needs to be
ioremapped. This is handling the _TTP=1 case, where the MMIO region
is translated by the bridge into an IO region on PCI.
If _TTP=0, the region is IO on both the upstream and downstream sides
of the host bridge, and we don't want to ioremap a new MMIO region for
it. It might be part of the legacy I/O port space, but that's
already covered elsewhere.
I don't think we need to add special handling for the _TTP=0 and _TRA
!= 0 case because I don't think it exists in the field. If and when
it *does* exist, we'll know what to do. In the meantime, it should
look just like the MEM path.
OK. I get it. acpi_dev_resource_address_space() will only apply _TRA to
resource -start and -end for both mem and io resource when _TTP=0. In
the add_window(), the offset returned by add_io_space() will be added
directly to -start and -end.
add_window() {
...
if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
I can wait for your patch to see the whole thing, but I would expect
offset = 0 here. For MEM resources, the arch code should not need to
look inside addr at all.
} else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
resource-start += offset;
resource-end += offset;
} else
return AE_OK;
...
}
If yes, I think _TRA can't be applied to IO resource in the
acpi_dev_resource_address_space() regardless of the value of _TTP.
BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table
6-185). The add_io_space() doesn't check _TTP when set sparse. So this
should be corrected?
Sure, I'm OK with this. It's possible we could trip over a BIOS bug
where _TRS=1 but _TTP=0, but I think the risk is low because only
large ia64 boxes would use this, and there aren't very many of those.
Ok. I will add a check for _TTP before setting sparse. Something likes this.
add_io_space()
{
...
if (addr-info.io.translation == ACPI_TYPE_TRANSLATION
addr-info.io.translation_type == ACPI_SPARSE_TRANSLATION)
sparse = 1;
...
}
Bjorn
--
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月31日 00:23, Bjorn Helgaas wrote:
> On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu wrote:
>> On 2013年10月29日 01:32, Bjorn Helgaas wrote:
>>>
>>> On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu wrote:
On 10/24/2013 06:39 AM, Bjorn Helgaas wrote:
>
> On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote:
>>
>>
>> On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
>>>
>>>
>>> I wonder if it would make sense to make
>>> acpi_dev_resource_address_space() ignore addr.translation_offset for
>>> IO resources. Or maybe ignore it if the _TTP (type translation) bit
>>> is set?
>>
>>
>>
>> I wonder why current code doesn't check _TTP? The code in the
>> add_io_space() seems to think _TTP is always set, right?
>
>
> I think it's an oversight, and you should fix it. I suggest that you
> ignore the _TRA value when _TTP is set. Obviously this only applies
> to I/O port resources, since _TTP is only defined in the I/O Resource
> Flag (Table 6-185 in ACPI 5.0 spec).
_TTP is also defined in the Memory Resource flag, Please have a look at
Table 6-184 in the ACPI 5.0 Spec.
>>>
>>>
>>> Yes, you're right. That would be for a host bridge that converts I/O
>>> on the primary (upstream) side of the bridge to memory on the PCI
>>> side. I've never seen such a bridge, and I can't really imagine why
>>> anybody would do that. But I guess you should be able to safely
>>> ignore _TRA when _TTP is set in either a MEM or IO descriptor, because
>>> the same reasoning should apply to both.
>>>
I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting
mean the resource is IO on the primary side and also IO on the secondary
side.
>>>
>>>
>>> If _TTP is not set, I guess you would apply _TRA. That's what you
>>> already do for MEM descriptors, and think you should just do the same
>>> for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0
>>> is rare for IO descriptors, but I suppose it could happen.
>>
>>
>> Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The
>> only reason for this case I think of is that the IO resource offsets on
>> the prime bus and second bus are different. In this case, we still need
>> to pass _TRA to new_space() and the finial resource->start still should be
>> acpi_resource->min + offset returned by add_io_space(), right?
>
> No, I don't think so. If the "phys_base" argument to new_space() is
> non-zero, it is the base of an MMIO region that needs to be
> ioremapped. This is handling the _TTP=1 case, where the MMIO region
> is translated by the bridge into an IO region on PCI.
>
> If _TTP=0, the region is IO on both the upstream and downstream sides
> of the host bridge, and we don't want to ioremap a new MMIO region for
> it. It might be part of the "legacy I/O port space," but that's
> already covered elsewhere.
>
> I don't think we need to add special handling for the _TTP=0 and _TRA
> != 0 case because I don't think it exists in the field. If and when
> it *does* exist, we'll know what to do. In the meantime, it should
> look just like the MEM path.
OK. I get it. acpi_dev_resource_address_space() will only apply _TRA to
resource ->start and ->end for both mem and io resource when _TTP=0. In
the add_window(), the offset returned by add_io_space() will be added
directly to ->start and ->end.
add_window() {
...
if (resource->flags & IORESOURCE_MEM) {
root = _resource;
offset = addr.translation_offset;
} else if (resource->flags & IORESOURCE_IO) {
root = _resource;
offset = add_io_space(info, );
if (offset == ~0)
return AE_OK;
resource->start += offset;
resource->end += offset;
} else
return AE_OK;
...
}
>
>> If yes, I think _TRA can't be applied to IO resource in the
>> acpi_dev_resource_address_space() regardless of the value of _TTP.
>>
>> BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table
>> 6-185). The add_io_space() doesn't check _TTP when set sparse. So this
>> should be corrected?
>
> Sure, I'm OK with this. It's possible we could trip over a BIOS bug
> where _TRS=1 but _TTP=0, but I think the risk is low because only
> large ia64 boxes would use this, and there aren't very many of those.
>
Ok. I will add a check for _TTP before setting sparse. Something likes this.
add_io_space()
{
...
if (addr->info.io.translation == ACPI_TYPE_TRANSLATION &&
addr->info.io.translation_type == ACPI_SPARSE_TRANSLATION)
sparse = 1;
...
}
> Bjorn
>
--
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu wrote: > On 2013年10月29日 01:32, Bjorn Helgaas wrote: >> >> On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu wrote: >>> >>> On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: > > > On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: >> >> >> I wonder if it would make sense to make >> acpi_dev_resource_address_space() ignore addr.translation_offset for >> IO resources. Or maybe ignore it if the _TTP (type translation) bit >> is set? > > > > I wonder why current code doesn't check _TTP? The code in the > add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). >>> >>> >>> _TTP is also defined in the Memory Resource flag, Please have a look at >>> Table 6-184 in the ACPI 5.0 Spec. >> >> >> Yes, you're right. That would be for a host bridge that converts I/O >> on the primary (upstream) side of the bridge to memory on the PCI >> side. I've never seen such a bridge, and I can't really imagine why >> anybody would do that. But I guess you should be able to safely >> ignore _TRA when _TTP is set in either a MEM or IO descriptor, because >> the same reasoning should apply to both. >> >>> I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting >>> mean the resource is IO on the primary side and also IO on the secondary >>> side. >> >> >> If _TTP is not set, I guess you would apply _TRA. That's what you >> already do for MEM descriptors, and think you should just do the same >> for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 >> is rare for IO descriptors, but I suppose it could happen. > > > Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The > only reason for this case I think of is that the IO resource offsets on > the prime bus and second bus are different. In this case, we still need > to pass _TRA to new_space() and the finial resource->start still should be > acpi_resource->min + offset returned by add_io_space(), right? No, I don't think so. If the "phys_base" argument to new_space() is non-zero, it is the base of an MMIO region that needs to be ioremapped. This is handling the _TTP=1 case, where the MMIO region is translated by the bridge into an IO region on PCI. If _TTP=0, the region is IO on both the upstream and downstream sides of the host bridge, and we don't want to ioremap a new MMIO region for it. It might be part of the "legacy I/O port space," but that's already covered elsewhere. I don't think we need to add special handling for the _TTP=0 and _TRA != 0 case because I don't think it exists in the field. If and when it *does* exist, we'll know what to do. In the meantime, it should look just like the MEM path. > If yes, I think _TRA can't be applied to IO resource in the > acpi_dev_resource_address_space() regardless of the value of _TTP. > > BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table > 6-185). The add_io_space() doesn't check _TTP when set sparse. So this > should be corrected? Sure, I'm OK with this. It's possible we could trip over a BIOS bug where _TRS=1 but _TTP=0, but I think the risk is low because only large ia64 boxes would use this, and there aren't very many of those. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月29日 01:32, Bjorn Helgaas wrote: On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu wrote: On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. Yes, you're right. That would be for a host bridge that converts I/O on the primary (upstream) side of the bridge to memory on the PCI side. I've never seen such a bridge, and I can't really imagine why anybody would do that. But I guess you should be able to safely ignore _TRA when _TTP is set in either a MEM or IO descriptor, because the same reasoning should apply to both. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. If _TTP is not set, I guess you would apply _TRA. That's what you already do for MEM descriptors, and think you should just do the same for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 is rare for IO descriptors, but I suppose it could happen. Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The only reason for this case I think of is that the IO resource offsets on the prime bus and second bus are different. In this case, we still need to pass _TRA to new_space() and the finial resource->start still should be acpi_resource->min + offset returned by add_io_space(), right? If yes, I think _TRA can't be applied to IO resource in the acpi_dev_resource_address_space() regardless of the value of _TTP. BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table 6-185). The add_io_space() doesn't check _TTP when set sparse. So this should be corrected? Bjorn -- Best regards Tianyu Lan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月29日 01:32, Bjorn Helgaas wrote: On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu tianyu@intel.com wrote: On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. Yes, you're right. That would be for a host bridge that converts I/O on the primary (upstream) side of the bridge to memory on the PCI side. I've never seen such a bridge, and I can't really imagine why anybody would do that. But I guess you should be able to safely ignore _TRA when _TTP is set in either a MEM or IO descriptor, because the same reasoning should apply to both. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. If _TTP is not set, I guess you would apply _TRA. That's what you already do for MEM descriptors, and think you should just do the same for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 is rare for IO descriptors, but I suppose it could happen. Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The only reason for this case I think of is that the IO resource offsets on the prime bus and second bus are different. In this case, we still need to pass _TRA to new_space() and the finial resource-start still should be acpi_resource-min + offset returned by add_io_space(), right? If yes, I think _TRA can't be applied to IO resource in the acpi_dev_resource_address_space() regardless of the value of _TTP. BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table 6-185). The add_io_space() doesn't check _TTP when set sparse. So this should be corrected? Bjorn -- Best regards Tianyu Lan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu tianyu@intel.com wrote: On 2013年10月29日 01:32, Bjorn Helgaas wrote: On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu tianyu@intel.com wrote: On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. Yes, you're right. That would be for a host bridge that converts I/O on the primary (upstream) side of the bridge to memory on the PCI side. I've never seen such a bridge, and I can't really imagine why anybody would do that. But I guess you should be able to safely ignore _TRA when _TTP is set in either a MEM or IO descriptor, because the same reasoning should apply to both. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. If _TTP is not set, I guess you would apply _TRA. That's what you already do for MEM descriptors, and think you should just do the same for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 is rare for IO descriptors, but I suppose it could happen. Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The only reason for this case I think of is that the IO resource offsets on the prime bus and second bus are different. In this case, we still need to pass _TRA to new_space() and the finial resource-start still should be acpi_resource-min + offset returned by add_io_space(), right? No, I don't think so. If the phys_base argument to new_space() is non-zero, it is the base of an MMIO region that needs to be ioremapped. This is handling the _TTP=1 case, where the MMIO region is translated by the bridge into an IO region on PCI. If _TTP=0, the region is IO on both the upstream and downstream sides of the host bridge, and we don't want to ioremap a new MMIO region for it. It might be part of the legacy I/O port space, but that's already covered elsewhere. I don't think we need to add special handling for the _TTP=0 and _TRA != 0 case because I don't think it exists in the field. If and when it *does* exist, we'll know what to do. In the meantime, it should look just like the MEM path. If yes, I think _TRA can't be applied to IO resource in the acpi_dev_resource_address_space() regardless of the value of _TTP. BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table 6-185). The add_io_space() doesn't check _TTP when set sparse. So this should be corrected? Sure, I'm OK with this. It's possible we could trip over a BIOS bug where _TRS=1 but _TTP=0, but I think the risk is low because only large ia64 boxes would use this, and there aren't very many of those. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月31日 00:23, Bjorn Helgaas wrote:
On Wed, Oct 30, 2013 at 2:34 AM, Lan Tianyu tianyu@intel.com wrote:
On 2013年10月29日 01:32, Bjorn Helgaas wrote:
On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu tianyu@intel.com wrote:
On 10/24/2013 06:39 AM, Bjorn Helgaas wrote:
On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote:
On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
I wonder if it would make sense to make
acpi_dev_resource_address_space() ignore addr.translation_offset for
IO resources. Or maybe ignore it if the _TTP (type translation) bit
is set?
I wonder why current code doesn't check _TTP? The code in the
add_io_space() seems to think _TTP is always set, right?
I think it's an oversight, and you should fix it. I suggest that you
ignore the _TRA value when _TTP is set. Obviously this only applies
to I/O port resources, since _TTP is only defined in the I/O Resource
Flag (Table 6-185 in ACPI 5.0 spec).
_TTP is also defined in the Memory Resource flag, Please have a look at
Table 6-184 in the ACPI 5.0 Spec.
Yes, you're right. That would be for a host bridge that converts I/O
on the primary (upstream) side of the bridge to memory on the PCI
side. I've never seen such a bridge, and I can't really imagine why
anybody would do that. But I guess you should be able to safely
ignore _TRA when _TTP is set in either a MEM or IO descriptor, because
the same reasoning should apply to both.
I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting
mean the resource is IO on the primary side and also IO on the secondary
side.
If _TTP is not set, I guess you would apply _TRA. That's what you
already do for MEM descriptors, and think you should just do the same
for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0
is rare for IO descriptors, but I suppose it could happen.
Yes, my concern is for the IO resource case of _TTP=0 and _TRA !=0. The
only reason for this case I think of is that the IO resource offsets on
the prime bus and second bus are different. In this case, we still need
to pass _TRA to new_space() and the finial resource-start still should be
acpi_resource-min + offset returned by add_io_space(), right?
No, I don't think so. If the phys_base argument to new_space() is
non-zero, it is the base of an MMIO region that needs to be
ioremapped. This is handling the _TTP=1 case, where the MMIO region
is translated by the bridge into an IO region on PCI.
If _TTP=0, the region is IO on both the upstream and downstream sides
of the host bridge, and we don't want to ioremap a new MMIO region for
it. It might be part of the legacy I/O port space, but that's
already covered elsewhere.
I don't think we need to add special handling for the _TTP=0 and _TRA
!= 0 case because I don't think it exists in the field. If and when
it *does* exist, we'll know what to do. In the meantime, it should
look just like the MEM path.
OK. I get it. acpi_dev_resource_address_space() will only apply _TRA to
resource -start and -end for both mem and io resource when _TTP=0. In
the add_window(), the offset returned by add_io_space() will be added
directly to -start and -end.
add_window() {
...
if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
} else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
resource-start += offset;
resource-end += offset;
} else
return AE_OK;
...
}
If yes, I think _TRA can't be applied to IO resource in the
acpi_dev_resource_address_space() regardless of the value of _TTP.
BTW, Translation Sparse(_TRS) is only meaningful if _TTP is set.(Table
6-185). The add_io_space() doesn't check _TTP when set sparse. So this
should be corrected?
Sure, I'm OK with this. It's possible we could trip over a BIOS bug
where _TRS=1 but _TTP=0, but I think the risk is low because only
large ia64 boxes would use this, and there aren't very many of those.
Ok. I will add a check for _TTP before setting sparse. Something likes this.
add_io_space()
{
...
if (addr-info.io.translation == ACPI_TYPE_TRANSLATION
addr-info.io.translation_type == ACPI_SPARSE_TRANSLATION)
sparse = 1;
...
}
Bjorn
--
Best regards
Tianyu Lan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu wrote: > On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: >> On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: >>> >>> On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? >>> >>> >>> I wonder why current code doesn't check _TTP? The code in the >>> add_io_space() seems to think _TTP is always set, right? >> >> I think it's an oversight, and you should fix it. I suggest that you >> ignore the _TRA value when _TTP is set. Obviously this only applies >> to I/O port resources, since _TTP is only defined in the I/O Resource >> Flag (Table 6-185 in ACPI 5.0 spec). > > _TTP is also defined in the Memory Resource flag, Please have a look at > Table 6-184 in the ACPI 5.0 Spec. Yes, you're right. That would be for a host bridge that converts I/O on the primary (upstream) side of the bridge to memory on the PCI side. I've never seen such a bridge, and I can't really imagine why anybody would do that. But I guess you should be able to safely ignore _TRA when _TTP is set in either a MEM or IO descriptor, because the same reasoning should apply to both. > I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting > mean the resource is IO on the primary side and also IO on the secondary > side. If _TTP is not set, I guess you would apply _TRA. That's what you already do for MEM descriptors, and think you should just do the same for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 is rare for IO descriptors, but I suppose it could happen. Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Sat, Oct 26, 2013 at 10:53 AM, Lan Tianyu tianyu@intel.com wrote: On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. Yes, you're right. That would be for a host bridge that converts I/O on the primary (upstream) side of the bridge to memory on the PCI side. I've never seen such a bridge, and I can't really imagine why anybody would do that. But I guess you should be able to safely ignore _TRA when _TTP is set in either a MEM or IO descriptor, because the same reasoning should apply to both. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. If _TTP is not set, I guess you would apply _TRA. That's what you already do for MEM descriptors, and think you should just do the same for IO descriptors. I would guess that having _TTP = 0 and _TRA != 0 is rare for IO descriptors, but I suppose it could happen. Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: [+cc Greg] On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu wrote: On 2013年10月17日 07:55, Bjorn Helgaas wrote: On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. Mike, is there any chance you could collect an acpidump from an rx7620 or similar ia64 system? In particular, I want to see a multi-node system where we have several PCI domains, and whether it sets the _TTP bits. Greg collected an acpidump from an HP system that uses these I/O port ranges. Unfortunately the system wasn't running Linux, so it's an EFI dump, not the usual one we get from the "pmtools" package. But I think it has the information we want. It's huge, and I put some of the relevant parts of it here: https://bugzilla.kernel.org/show_bug.cgi?id=63581 Here's a sample that shows the _TTP bit is set for the I/O aperture: Device E000 (\_SB_.N000.E000) Name _SEG (\_SB_.N000.E000._SEG) 0x01 Name _CRS (\_SB_.N000.E001._CRS) Buffer 0x0092 ByteList <0x88 0x0d 0x00 0x02 0x0e 0x00 0x00 0x00 0x00 0x00 0xff 0x00 0x00 0x00 0x00 0x01 0x8a 0x2b 0x00 0x01 0x0c 0x33 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xd0 0x00 0x04 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 Byte 0: 0x8a (QWORD address space descriptor) Byte 3: Resource Type = 0x01 (I/O range) Byte 5: Type Specific Flags = 0x33 (_TRS, _TTP, _RNG = 3) QWORD Address Space Descriptor: Type: I/O Flags: Sparse, Translate, ISA I/O addresses, Non-ISA I/O addresses GRA: 0x MIN: 0x MAX: 0x TRA: 0x0400d000 LEN: 0x0001 MAX address fixed MIN address fixed Address positively decoded Device produces and consumes this resource Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 10/24/2013 06:39 AM, Bjorn Helgaas wrote: [+cc Greg] On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu tianyu@intel.com wrote: On 2013年10月17日 07:55, Bjorn Helgaas wrote: On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). _TTP is also defined in the Memory Resource flag, Please have a look at Table 6-184 in the ACPI 5.0 Spec. I am not sure how to deal with _TTP unsetting io resource? _TTP unsetting mean the resource is IO on the primary side and also IO on the secondary side. Mike, is there any chance you could collect an acpidump from an rx7620 or similar ia64 system? In particular, I want to see a multi-node system where we have several PCI domains, and whether it sets the _TTP bits. Greg collected an acpidump from an HP system that uses these I/O port ranges. Unfortunately the system wasn't running Linux, so it's an EFI dump, not the usual one we get from the pmtools package. But I think it has the information we want. It's huge, and I put some of the relevant parts of it here: https://bugzilla.kernel.org/show_bug.cgi?id=63581 Here's a sample that shows the _TTP bit is set for the I/O aperture: Device E000 (\_SB_.N000.E000) Name _SEG (\_SB_.N000.E000._SEG) 0x01 Name _CRS (\_SB_.N000.E001._CRS) Buffer 0x0092 ByteList 0x88 0x0d 0x00 0x02 0x0e 0x00 0x00 0x00 0x00 0x00 0xff 0x00 0x00 0x00 0x00 0x01 0x8a 0x2b 0x00 0x01 0x0c 0x33 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xd0 0x00 0x04 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 Byte 0: 0x8a (QWORD address space descriptor) Byte 3: Resource Type = 0x01 (I/O range) Byte 5: Type Specific Flags = 0x33 (_TRS, _TTP, _RNG = 3) QWORD Address Space Descriptor: Type: I/O Flags: Sparse, Translate, ISA I/O addresses, Non-ISA I/O addresses GRA: 0x MIN: 0x MAX: 0x TRA: 0x0400d000 LEN: 0x0001 MAX address fixed MIN address fixed Address positively decoded Device produces and consumes this resource Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
[+cc Greg] On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: > On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: >> On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu wrote: >>>On 2013年10月17日 07:55, Bjorn Helgaas wrote: On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote: >>I wonder if it would make sense to make >>acpi_dev_resource_address_space() ignore addr.translation_offset for >>IO resources. Or maybe ignore it if the _TTP (type translation) bit >>is set? > > I wonder why current code doesn't check _TTP? The code in the > add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). >>Mike, is there any chance you could collect an acpidump from an >>rx7620 or similar ia64 system? In particular, I want to see a >>multi-node system where we have several PCI domains, and whether it >>sets the _TTP bits. Greg collected an acpidump from an HP system that uses these I/O port ranges. Unfortunately the system wasn't running Linux, so it's an EFI dump, not the usual one we get from the "pmtools" package. But I think it has the information we want. It's huge, and I put some of the relevant parts of it here: https://bugzilla.kernel.org/show_bug.cgi?id=63581 Here's a sample that shows the _TTP bit is set for the I/O aperture: Device E000 (\_SB_.N000.E000) Name _SEG (\_SB_.N000.E000._SEG) 0x01 Name _CRS (\_SB_.N000.E001._CRS) Buffer 0x0092 ByteList <0x88 0x0d 0x00 0x02 0x0e 0x00 0x00 0x00 0x00 0x00 0xff 0x00 0x00 0x00 0x00 0x01 0x8a 0x2b 0x00 0x01 0x0c 0x33 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xd0 0x00 0x04 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 Byte 0: 0x8a (QWORD address space descriptor) Byte 3: Resource Type = 0x01 (I/O range) Byte 5: Type Specific Flags = 0x33 (_TRS, _TTP, _RNG = 3) QWORD Address Space Descriptor: Type: I/O Flags: Sparse, Translate, ISA I/O addresses, Non-ISA I/O addresses GRA: 0x MIN: 0x MAX: 0x TRA: 0x0400d000 LEN: 0x0001 MAX address fixed MIN address fixed Address positively decoded Device produces and consumes this resource Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
[+cc Greg] On Fri, Oct 18, 2013 at 08:44:12PM +0800, Lan Tianyu wrote: On 10/18/2013 04:33 AM, Bjorn Helgaas wrote: On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu tianyu@intel.com wrote: On 2013年10月17日 07:55, Bjorn Helgaas wrote: On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote: I wonder if it would make sense to make acpi_dev_resource_address_space() ignore addr.translation_offset for IO resources. Or maybe ignore it if the _TTP (type translation) bit is set? I wonder why current code doesn't check _TTP? The code in the add_io_space() seems to think _TTP is always set, right? I think it's an oversight, and you should fix it. I suggest that you ignore the _TRA value when _TTP is set. Obviously this only applies to I/O port resources, since _TTP is only defined in the I/O Resource Flag (Table 6-185 in ACPI 5.0 spec). Mike, is there any chance you could collect an acpidump from an rx7620 or similar ia64 system? In particular, I want to see a multi-node system where we have several PCI domains, and whether it sets the _TTP bits. Greg collected an acpidump from an HP system that uses these I/O port ranges. Unfortunately the system wasn't running Linux, so it's an EFI dump, not the usual one we get from the pmtools package. But I think it has the information we want. It's huge, and I put some of the relevant parts of it here: https://bugzilla.kernel.org/show_bug.cgi?id=63581 Here's a sample that shows the _TTP bit is set for the I/O aperture: Device E000 (\_SB_.N000.E000) Name _SEG (\_SB_.N000.E000._SEG) 0x01 Name _CRS (\_SB_.N000.E001._CRS) Buffer 0x0092 ByteList 0x88 0x0d 0x00 0x02 0x0e 0x00 0x00 0x00 0x00 0x00 0xff 0x00 0x00 0x00 0x00 0x01 0x8a 0x2b 0x00 0x01 0x0c 0x33 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xff 0xff 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0xd0 0x00 0x04 0x00 0x00 0x00 0x00 0x01 0x00 0x00 0x00 0x00 0x00 Byte 0: 0x8a (QWORD address space descriptor) Byte 3: Resource Type = 0x01 (I/O range) Byte 5: Type Specific Flags = 0x33 (_TRS, _TTP, _RNG = 3) QWORD Address Space Descriptor: Type: I/O Flags: Sparse, Translate, ISA I/O addresses, Non-ISA I/O addresses GRA: 0x MIN: 0x MAX: 0x TRA: 0x0400d000 LEN: 0x0001 MAX address fixed MIN address fixed Address positively decoded Device produces and consumes this resource Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
[+cc Mike]
On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu
wrote:
On 2013年10月17日 07:55, Bjorn Helgaas wrote:
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com
wrote:
From: Lan Tianyu
Using ACPI resource functions to convert ACPI resource to
generic resource
Signed-off-by: Lan Tianyu --- This patch
just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38
+- 1 file changed, 21
insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c index
2326790..14fa175 100644 --- a/arch/ia64/pci/pci.c +++
b/arch/ia64/pci/pci.c @@ -232,8 +232,9 @@ out: return ~0; }
-static acpi_status resource_to_window(struct acpi_resource
*resource, - struct
acpi_resource_address64 *addr) +static acpi_status
resource_to_window(struct acpi_resource *ares, +
struct acpi_resource_address64 *addr, +
struct resource *res) { acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status
resource_to_window(struct acpi_resource *resource, * -
producers, i.e., the address space is routed downstream, *
not consumed by the bridge itself */ -status =
acpi_resource_to_address64(resource, addr); +status =
acpi_dev_resource_address_space_full(ares, addr, res); if
(ACPI_SUCCESS(status) && (addr->resource_type ==
ACPI_MEMORY_RANGE || addr->resource_type == ACPI_IO_RANGE) &&
@@ -255,51 +256,54 @@ static acpi_status
resource_to_window(struct acpi_resource *resource, return
AE_ERROR; }
-static acpi_status count_window(struct acpi_resource
*resource, void *data) +static acpi_status count_window(struct
acpi_resource *ares, void *data) { unsigned int *windows =
(unsigned int *) data; struct acpi_resource_address64 addr; +
struct resource res; acpi_status status;
-status = resource_to_window(resource, ); +status
= resource_to_window(ares, , ); if
(ACPI_SUCCESS(status)) (*windows)++;
return AE_OK; }
-static acpi_status add_window(struct acpi_resource *res, void
*data) +static acpi_status add_window(struct acpi_resource
*ares, void *data) { struct pci_root_info *info = data; -
struct resource *resource; +struct resource *resource =
>res[info->res_num]; struct acpi_resource_address64
addr; acpi_status status; -unsigned long flags, offset =
0; +unsigned long offset = 0; struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for
more */ -status = resource_to_window(res, ); +
status = resource_to_window(ares, , resource); if
(!ACPI_SUCCESS(status)) return AE_OK;
-if (addr.resource_type == ACPI_MEMORY_RANGE) { -
flags = IORESOURCE_MEM; +if (resource->flags &
IORESOURCE_MEM) { root = _resource; offset =
addr.translation_offset; -} else if (addr.resource_type ==
ACPI_IO_RANGE) { -flags = IORESOURCE_IO; +}
else if (resource->flags & IORESOURCE_IO) { root =
_resource; offset = add_io_space(info, ); if
(offset == ~0) return AE_OK; + +/* + *
io space address translation offset depends + * on
the return value of add_io_space(). So + *
Repopulate resource->start and end here.
"Repopulate" makes it sound like "resource->start" got clobbered
somewhere. But it didn't. I think it's just that each bridge
can support its own I/O port range, and the PCI port numbers
reported in the acpi_resource may not be distinct, and
add_io_space() adds an offset so all the I/O port ranges fit into
one global I/O port space.
For example, I think these two bridges have the same port
numbers (0x0-0xfff) in their acpi_resource, but the second has an
offset of 0x100 in the system I/O port space, and I think
this offset is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI
[0x0-0xfff]) HWP0002:09: host bridge window [io
0x100-0x1000fff] (PCI [0x0-0xfff])
+ */ +resource->start = addr.minimum +
offset; +resource->end = resource->start +
addr.address_length - 1;
Can't we use this:
resource->start += offset; resource->end += offset;
to avoid breaking the encapsulation of struct
acpi_resource_address64?
resource->start has been populated with "addr.minimum +
addr.translation_offset" in the acpi_dev_resource_address_space().
That's true, but this is a change from previous behavior.
Previously, x86 applied addr.translation_offset to both MEM and IO
resources (in setup_resource()), but ia64 applied it only to MEM
resources (in add_window()). With your patch, we apply it to both
types in acpi_dev_resource_address_space(), which is a change for
ia64.
Yes, this is why I repopulate resource->start and ->end after
add_io_space().
I know translation_offset is used on some HP ia64 boxes, but I'm not
aware of it being used for IO resources on any x86 boxes. On those
ia64 boxes, the bridge also does type translation (the resource is
MEM on the primary side but IO on the
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 10/18/2013 04:33 AM, Bjorn Helgaas wrote:
[+cc Mike]
On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu tianyu@intel.com
wrote:
On 2013年10月17日 07:55, Bjorn Helgaas wrote:
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com
wrote:
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to
generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com --- This patch
just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38
+- 1 file changed, 21
insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c index
2326790..14fa175 100644 --- a/arch/ia64/pci/pci.c +++
b/arch/ia64/pci/pci.c @@ -232,8 +232,9 @@ out: return ~0; }
-static acpi_status resource_to_window(struct acpi_resource
*resource, - struct
acpi_resource_address64 *addr) +static acpi_status
resource_to_window(struct acpi_resource *ares, +
struct acpi_resource_address64 *addr, +
struct resource *res) { acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status
resource_to_window(struct acpi_resource *resource, * -
producers, i.e., the address space is routed downstream, *
not consumed by the bridge itself */ -status =
acpi_resource_to_address64(resource, addr); +status =
acpi_dev_resource_address_space_full(ares, addr, res); if
(ACPI_SUCCESS(status) (addr-resource_type ==
ACPI_MEMORY_RANGE || addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status
resource_to_window(struct acpi_resource *resource, return
AE_ERROR; }
-static acpi_status count_window(struct acpi_resource
*resource, void *data) +static acpi_status count_window(struct
acpi_resource *ares, void *data) { unsigned int *windows =
(unsigned int *) data; struct acpi_resource_address64 addr; +
struct resource res; acpi_status status;
-status = resource_to_window(resource, addr); +status
= resource_to_window(ares, addr, res); if
(ACPI_SUCCESS(status)) (*windows)++;
return AE_OK; }
-static acpi_status add_window(struct acpi_resource *res, void
*data) +static acpi_status add_window(struct acpi_resource
*ares, void *data) { struct pci_root_info *info = data; -
struct resource *resource; +struct resource *resource =
info-res[info-res_num]; struct acpi_resource_address64
addr; acpi_status status; -unsigned long flags, offset =
0; +unsigned long offset = 0; struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for
more */ -status = resource_to_window(res, addr); +
status = resource_to_window(ares, addr, resource); if
(!ACPI_SUCCESS(status)) return AE_OK;
-if (addr.resource_type == ACPI_MEMORY_RANGE) { -
flags = IORESOURCE_MEM; +if (resource-flags
IORESOURCE_MEM) { root = iomem_resource; offset =
addr.translation_offset; -} else if (addr.resource_type ==
ACPI_IO_RANGE) { -flags = IORESOURCE_IO; +}
else if (resource-flags IORESOURCE_IO) { root =
ioport_resource; offset = add_io_space(info, addr); if
(offset == ~0) return AE_OK; + +/* + *
io space address translation offset depends + * on
the return value of add_io_space(). So + *
Repopulate resource-start and end here.
Repopulate makes it sound like resource-start got clobbered
somewhere. But it didn't. I think it's just that each bridge
can support its own I/O port range, and the PCI port numbers
reported in the acpi_resource may not be distinct, and
add_io_space() adds an offset so all the I/O port ranges fit into
one global I/O port space.
For example, I think these two bridges have the same port
numbers (0x0-0xfff) in their acpi_resource, but the second has an
offset of 0x100 in the system I/O port space, and I think
this offset is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI
[0x0-0xfff]) HWP0002:09: host bridge window [io
0x100-0x1000fff] (PCI [0x0-0xfff])
+ */ +resource-start = addr.minimum +
offset; +resource-end = resource-start +
addr.address_length - 1;
Can't we use this:
resource-start += offset; resource-end += offset;
to avoid breaking the encapsulation of struct
acpi_resource_address64?
resource-start has been populated with addr.minimum +
addr.translation_offset in the acpi_dev_resource_address_space().
That's true, but this is a change from previous behavior.
Previously, x86 applied addr.translation_offset to both MEM and IO
resources (in setup_resource()), but ia64 applied it only to MEM
resources (in add_window()). With your patch, we apply it to both
types in acpi_dev_resource_address_space(), which is a change for
ia64.
Yes, this is why I repopulate resource-start and -end after
add_io_space().
I know translation_offset is used on some HP ia64 boxes, but I'm not
aware of it being used for IO resources on any x86 boxes. On those
ia64 boxes, the bridge also does type
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
[+cc Mike]
On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu wrote:
> On 2013年10月17日 07:55, Bjorn Helgaas wrote:
>> On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
>>> From: Lan Tianyu
>>>
>>> Using ACPI resource functions to convert ACPI resource to generic resource
>>>
>>> Signed-off-by: Lan Tianyu
>>> ---
>>> This patch just passes through compilation test due to no ia64 machine on
>>> hand.
>>>
>>> arch/ia64/pci/pci.c | 38 +-
>>> 1 file changed, 21 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
>>> index 2326790..14fa175 100644
>>> --- a/arch/ia64/pci/pci.c
>>> +++ b/arch/ia64/pci/pci.c
>>> @@ -232,8 +232,9 @@ out:
>>> return ~0;
>>> }
>>>
>>> -static acpi_status resource_to_window(struct acpi_resource *resource,
>>> - struct acpi_resource_address64 *addr)
>>> +static acpi_status resource_to_window(struct acpi_resource *ares,
>>> + struct acpi_resource_address64 *addr,
>>> + struct resource *res)
>>> {
>>> acpi_status status;
>>>
>>> @@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
>>> acpi_resource *resource,
>>> * - producers, i.e., the address space is routed downstream,
>>> *not consumed by the bridge itself
>>> */
>>> -status = acpi_resource_to_address64(resource, addr);
>>> +status = acpi_dev_resource_address_space_full(ares, addr, res);
>>> if (ACPI_SUCCESS(status) &&
>>> (addr->resource_type == ACPI_MEMORY_RANGE ||
>>> addr->resource_type == ACPI_IO_RANGE) &&
>>> @@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
>>> acpi_resource *resource,
>>> return AE_ERROR;
>>> }
>>>
>>> -static acpi_status count_window(struct acpi_resource *resource, void *data)
>>> +static acpi_status count_window(struct acpi_resource *ares, void *data)
>>> {
>>> unsigned int *windows = (unsigned int *) data;
>>> struct acpi_resource_address64 addr;
>>> +struct resource res;
>>> acpi_status status;
>>>
>>> -status = resource_to_window(resource, );
>>> +status = resource_to_window(ares, , );
>>> if (ACPI_SUCCESS(status))
>>> (*windows)++;
>>>
>>> return AE_OK;
>>> }
>>>
>>> -static acpi_status add_window(struct acpi_resource *res, void *data)
>>> +static acpi_status add_window(struct acpi_resource *ares, void *data)
>>> {
>>> struct pci_root_info *info = data;
>>> -struct resource *resource;
>>> +struct resource *resource = >res[info->res_num];
>>> struct acpi_resource_address64 addr;
>>> acpi_status status;
>>> -unsigned long flags, offset = 0;
>>> +unsigned long offset = 0;
>>> struct resource *root;
>>>
>>> /* Return AE_OK for non-window resources to keep scanning for more */
>>> -status = resource_to_window(res, );
>>> +status = resource_to_window(ares, , resource);
>>> if (!ACPI_SUCCESS(status))
>>> return AE_OK;
>>>
>>> -if (addr.resource_type == ACPI_MEMORY_RANGE) {
>>> -flags = IORESOURCE_MEM;
>>> +if (resource->flags & IORESOURCE_MEM) {
>>> root = _resource;
>>> offset = addr.translation_offset;
>>> -} else if (addr.resource_type == ACPI_IO_RANGE) {
>>> -flags = IORESOURCE_IO;
>>> +} else if (resource->flags & IORESOURCE_IO) {
>>> root = _resource;
>>> offset = add_io_space(info, );
>>> if (offset == ~0)
>>> return AE_OK;
>>> +
>>> +/*
>>> + * io space address translation offset depends
>>> + * on the return value of add_io_space(). So
>>> + * Repopulate resource->start and end here.
>>
>> "Repopulate" makes it sound like "resource->start" got clobbered
>> somewhere. But it didn't. I think it's just that each bridge can
>> support its own I/O port range, and the PCI port numbers reported
>> in the acpi_resource may not be distinct, and add_io_space() adds
>> an offset so all the I/O port ranges fit into one global I/O port
>> space.
>>
>> For example, I think these two bridges have the same port numbers
>> (0x0-0xfff) in their acpi_resource, but the second has an offset
>> of 0x100 in the system I/O port space, and I think this offset
>> is what add_io_space() returns:
>>
>> HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
>> HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
>>
>>> + */
>>> +resource->start = addr.minimum + offset;
>>> +resource->end = resource->start + addr.address_length - 1;
>>
>> Can't we use this:
>>
>> resource->start += offset;
>> resource->end += offset;
>>
>> to avoid breaking the encapsulation of struct acpi_resource_address64?
>
> resource->start has been
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月17日 07:55, Bjorn Helgaas wrote:
> On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
>> From: Lan Tianyu
>>
>> Using ACPI resource functions to convert ACPI resource to generic resource
>>
>> Signed-off-by: Lan Tianyu
>> ---
>> This patch just passes through compilation test due to no ia64 machine on
>> hand.
>>
>> arch/ia64/pci/pci.c | 38 +-
>> 1 file changed, 21 insertions(+), 17 deletions(-)
>>
>> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
>> index 2326790..14fa175 100644
>> --- a/arch/ia64/pci/pci.c
>> +++ b/arch/ia64/pci/pci.c
>> @@ -232,8 +232,9 @@ out:
>> return ~0;
>> }
>>
>> -static acpi_status resource_to_window(struct acpi_resource *resource,
>> - struct acpi_resource_address64 *addr)
>> +static acpi_status resource_to_window(struct acpi_resource *ares,
>> + struct acpi_resource_address64 *addr,
>> + struct resource *res)
>> {
>> acpi_status status;
>>
>> @@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
>> acpi_resource *resource,
>> * - producers, i.e., the address space is routed downstream,
>> *not consumed by the bridge itself
>> */
>> -status = acpi_resource_to_address64(resource, addr);
>> +status = acpi_dev_resource_address_space_full(ares, addr, res);
>> if (ACPI_SUCCESS(status) &&
>> (addr->resource_type == ACPI_MEMORY_RANGE ||
>> addr->resource_type == ACPI_IO_RANGE) &&
>> @@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
>> acpi_resource *resource,
>> return AE_ERROR;
>> }
>>
>> -static acpi_status count_window(struct acpi_resource *resource, void *data)
>> +static acpi_status count_window(struct acpi_resource *ares, void *data)
>> {
>> unsigned int *windows = (unsigned int *) data;
>> struct acpi_resource_address64 addr;
>> +struct resource res;
>> acpi_status status;
>>
>> -status = resource_to_window(resource, );
>> +status = resource_to_window(ares, , );
>> if (ACPI_SUCCESS(status))
>> (*windows)++;
>>
>> return AE_OK;
>> }
>>
>> -static acpi_status add_window(struct acpi_resource *res, void *data)
>> +static acpi_status add_window(struct acpi_resource *ares, void *data)
>> {
>> struct pci_root_info *info = data;
>> -struct resource *resource;
>> +struct resource *resource = >res[info->res_num];
>> struct acpi_resource_address64 addr;
>> acpi_status status;
>> -unsigned long flags, offset = 0;
>> +unsigned long offset = 0;
>> struct resource *root;
>>
>> /* Return AE_OK for non-window resources to keep scanning for more */
>> -status = resource_to_window(res, );
>> +status = resource_to_window(ares, , resource);
>> if (!ACPI_SUCCESS(status))
>> return AE_OK;
>>
>> -if (addr.resource_type == ACPI_MEMORY_RANGE) {
>> -flags = IORESOURCE_MEM;
>> +if (resource->flags & IORESOURCE_MEM) {
>> root = _resource;
>> offset = addr.translation_offset;
>> -} else if (addr.resource_type == ACPI_IO_RANGE) {
>> -flags = IORESOURCE_IO;
>> +} else if (resource->flags & IORESOURCE_IO) {
>> root = _resource;
>> offset = add_io_space(info, );
>> if (offset == ~0)
>> return AE_OK;
>> +
>> +/*
>> + * io space address translation offset depends
>> + * on the return value of add_io_space(). So
>> + * Repopulate resource->start and end here.
>
> "Repopulate" makes it sound like "resource->start" got clobbered
> somewhere. But it didn't. I think it's just that each bridge can
> support its own I/O port range, and the PCI port numbers reported
> in the acpi_resource may not be distinct, and add_io_space() adds
> an offset so all the I/O port ranges fit into one global I/O port
> space.
>
> For example, I think these two bridges have the same port numbers
> (0x0-0xfff) in their acpi_resource, but the second has an offset
> of 0x100 in the system I/O port space, and I think this offset
> is what add_io_space() returns:
>
> HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
> HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
>
>> + */
>> +resource->start = addr.minimum + offset;
>> +resource->end = resource->start + addr.address_length - 1;
>
> Can't we use this:
>
> resource->start += offset;
> resource->end += offset;
>
> to avoid breaking the encapsulation of struct acpi_resource_address64?
resource->start has been populated with "addr.minimum +
addr.translation_offset" in the acpi_dev_resource_address_space().
continuing to add the offset to resource->start seems not right.
The
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On 2013年10月17日 07:55, Bjorn Helgaas wrote:
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com
---
This patch just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
-status = acpi_resource_to_address64(resource, addr);
+status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status)
(addr-resource_type == ACPI_MEMORY_RANGE ||
addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+struct resource res;
acpi_status status;
-status = resource_to_window(resource, addr);
+status = resource_to_window(ares, addr, res);
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
-struct resource *resource;
+struct resource *resource = info-res[info-res_num];
struct acpi_resource_address64 addr;
acpi_status status;
-unsigned long flags, offset = 0;
+unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
-status = resource_to_window(res, addr);
+status = resource_to_window(ares, addr, resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
-if (addr.resource_type == ACPI_MEMORY_RANGE) {
-flags = IORESOURCE_MEM;
+if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
-} else if (addr.resource_type == ACPI_IO_RANGE) {
-flags = IORESOURCE_IO;
+} else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
+
+/*
+ * io space address translation offset depends
+ * on the return value of add_io_space(). So
+ * Repopulate resource-start and end here.
Repopulate makes it sound like resource-start got clobbered
somewhere. But it didn't. I think it's just that each bridge can
support its own I/O port range, and the PCI port numbers reported
in the acpi_resource may not be distinct, and add_io_space() adds
an offset so all the I/O port ranges fit into one global I/O port
space.
For example, I think these two bridges have the same port numbers
(0x0-0xfff) in their acpi_resource, but the second has an offset
of 0x100 in the system I/O port space, and I think this offset
is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
+ */
+resource-start = addr.minimum + offset;
+resource-end = resource-start + addr.address_length - 1;
Can't we use this:
resource-start += offset;
resource-end += offset;
to avoid breaking the encapsulation of struct acpi_resource_address64?
resource-start has been populated with addr.minimum +
addr.translation_offset in the acpi_dev_resource_address_space().
continuing to add the offset to resource-start seems not right.
The add_io_space() accepts translation_offset and then ioremap it to
mmio address. Add the result to io_space array and assign a space
number. Left shift the space number 24
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
[+cc Mike]
On Thu, Oct 17, 2013 at 12:09 AM, Lan Tianyu tianyu@intel.com wrote:
On 2013年10月17日 07:55, Bjorn Helgaas wrote:
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com
---
This patch just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
-status = acpi_resource_to_address64(resource, addr);
+status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status)
(addr-resource_type == ACPI_MEMORY_RANGE ||
addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+struct resource res;
acpi_status status;
-status = resource_to_window(resource, addr);
+status = resource_to_window(ares, addr, res);
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
-struct resource *resource;
+struct resource *resource = info-res[info-res_num];
struct acpi_resource_address64 addr;
acpi_status status;
-unsigned long flags, offset = 0;
+unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
-status = resource_to_window(res, addr);
+status = resource_to_window(ares, addr, resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
-if (addr.resource_type == ACPI_MEMORY_RANGE) {
-flags = IORESOURCE_MEM;
+if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
-} else if (addr.resource_type == ACPI_IO_RANGE) {
-flags = IORESOURCE_IO;
+} else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
+
+/*
+ * io space address translation offset depends
+ * on the return value of add_io_space(). So
+ * Repopulate resource-start and end here.
Repopulate makes it sound like resource-start got clobbered
somewhere. But it didn't. I think it's just that each bridge can
support its own I/O port range, and the PCI port numbers reported
in the acpi_resource may not be distinct, and add_io_space() adds
an offset so all the I/O port ranges fit into one global I/O port
space.
For example, I think these two bridges have the same port numbers
(0x0-0xfff) in their acpi_resource, but the second has an offset
of 0x100 in the system I/O port space, and I think this offset
is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
+ */
+resource-start = addr.minimum + offset;
+resource-end = resource-start + addr.address_length - 1;
Can't we use this:
resource-start += offset;
resource-end += offset;
to avoid breaking the encapsulation of struct acpi_resource_address64?
resource-start has been populated with addr.minimum +
addr.translation_offset in the acpi_dev_resource_address_space().
That's true, but this is a change from previous behavior. Previously,
x86 applied addr.translation_offset to both MEM and IO resources (in
setup_resource()), but ia64
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
> From: Lan Tianyu
>
> Using ACPI resource functions to convert ACPI resource to generic resource
>
> Signed-off-by: Lan Tianyu
> ---
> This patch just passes through compilation test due to no ia64 machine on
> hand.
>
> arch/ia64/pci/pci.c | 38 +-
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 2326790..14fa175 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -232,8 +232,9 @@ out:
> return ~0;
> }
>
> -static acpi_status resource_to_window(struct acpi_resource *resource,
> - struct acpi_resource_address64 *addr)
> +static acpi_status resource_to_window(struct acpi_resource *ares,
> + struct acpi_resource_address64 *addr,
> + struct resource *res)
> {
> acpi_status status;
>
> @@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
> acpi_resource *resource,
>* - producers, i.e., the address space is routed downstream,
>*not consumed by the bridge itself
>*/
> - status = acpi_resource_to_address64(resource, addr);
> + status = acpi_dev_resource_address_space_full(ares, addr, res);
> if (ACPI_SUCCESS(status) &&
> (addr->resource_type == ACPI_MEMORY_RANGE ||
>addr->resource_type == ACPI_IO_RANGE) &&
> @@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
> acpi_resource *resource,
> return AE_ERROR;
> }
>
> -static acpi_status count_window(struct acpi_resource *resource, void *data)
> +static acpi_status count_window(struct acpi_resource *ares, void *data)
> {
> unsigned int *windows = (unsigned int *) data;
> struct acpi_resource_address64 addr;
> + struct resource res;
> acpi_status status;
>
> - status = resource_to_window(resource, );
> + status = resource_to_window(ares, , );
> if (ACPI_SUCCESS(status))
> (*windows)++;
>
> return AE_OK;
> }
>
> -static acpi_status add_window(struct acpi_resource *res, void *data)
> +static acpi_status add_window(struct acpi_resource *ares, void *data)
> {
> struct pci_root_info *info = data;
> - struct resource *resource;
> + struct resource *resource = >res[info->res_num];
> struct acpi_resource_address64 addr;
> acpi_status status;
> - unsigned long flags, offset = 0;
> + unsigned long offset = 0;
> struct resource *root;
>
> /* Return AE_OK for non-window resources to keep scanning for more */
> - status = resource_to_window(res, );
> + status = resource_to_window(ares, , resource);
> if (!ACPI_SUCCESS(status))
> return AE_OK;
>
> - if (addr.resource_type == ACPI_MEMORY_RANGE) {
> - flags = IORESOURCE_MEM;
> + if (resource->flags & IORESOURCE_MEM) {
> root = _resource;
> offset = addr.translation_offset;
> - } else if (addr.resource_type == ACPI_IO_RANGE) {
> - flags = IORESOURCE_IO;
> + } else if (resource->flags & IORESOURCE_IO) {
> root = _resource;
> offset = add_io_space(info, );
> if (offset == ~0)
> return AE_OK;
> +
> + /*
> + * io space address translation offset depends
> + * on the return value of add_io_space(). So
> + * Repopulate resource->start and end here.
"Repopulate" makes it sound like "resource->start" got clobbered
somewhere. But it didn't. I think it's just that each bridge can
support its own I/O port range, and the PCI port numbers reported
in the acpi_resource may not be distinct, and add_io_space() adds
an offset so all the I/O port ranges fit into one global I/O port
space.
For example, I think these two bridges have the same port numbers
(0x0-0xfff) in their acpi_resource, but the second has an offset
of 0x100 in the system I/O port space, and I think this offset
is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
> + */
> + resource->start = addr.minimum + offset;
> + resource->end = resource->start + addr.address_length - 1;
Can't we use this:
resource->start += offset;
resource->end += offset;
to avoid breaking the encapsulation of struct acpi_resource_address64?
> } else
> return AE_OK;
>
> - resource = >res[info->res_num];
> resource->name = info->name;
> - resource->flags = flags;
> - resource->start = addr.minimum + offset;
> - resource->end = resource->start + addr.address_length - 1;
> info->res_offset[info->res_num] =
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Fri, Oct 11, 2013 at 08:19:01PM +0800, tianyu@intel.com wrote:
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com
---
This patch just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
- status = acpi_resource_to_address64(resource, addr);
+ status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status)
(addr-resource_type == ACPI_MEMORY_RANGE ||
addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+ struct resource res;
acpi_status status;
- status = resource_to_window(resource, addr);
+ status = resource_to_window(ares, addr, res);
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
- struct resource *resource;
+ struct resource *resource = info-res[info-res_num];
struct acpi_resource_address64 addr;
acpi_status status;
- unsigned long flags, offset = 0;
+ unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
- status = resource_to_window(res, addr);
+ status = resource_to_window(ares, addr, resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
- if (addr.resource_type == ACPI_MEMORY_RANGE) {
- flags = IORESOURCE_MEM;
+ if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
- } else if (addr.resource_type == ACPI_IO_RANGE) {
- flags = IORESOURCE_IO;
+ } else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
+
+ /*
+ * io space address translation offset depends
+ * on the return value of add_io_space(). So
+ * Repopulate resource-start and end here.
Repopulate makes it sound like resource-start got clobbered
somewhere. But it didn't. I think it's just that each bridge can
support its own I/O port range, and the PCI port numbers reported
in the acpi_resource may not be distinct, and add_io_space() adds
an offset so all the I/O port ranges fit into one global I/O port
space.
For example, I think these two bridges have the same port numbers
(0x0-0xfff) in their acpi_resource, but the second has an offset
of 0x100 in the system I/O port space, and I think this offset
is what add_io_space() returns:
HWP0002:00: host bridge window [io 0x-0x0fff] (PCI [0x0-0xfff])
HWP0002:09: host bridge window [io 0x100-0x1000fff] (PCI [0x0-0xfff])
+ */
+ resource-start = addr.minimum + offset;
+ resource-end = resource-start + addr.address_length - 1;
Can't we use this:
resource-start += offset;
resource-end += offset;
to avoid breaking the encapsulation of struct acpi_resource_address64?
} else
return AE_OK;
- resource = info-res[info-res_num];
resource-name = info-name;
- resource-flags = flags;
- resource-start = addr.minimum + offset;
- resource-end = resource-start + addr.address_length - 1;
info-res_offset[info-res_num] = offset;
if (insert_resource(root, resource)) {
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Friday, October 11, 2013 08:19:01 PM tianyu@intel.com wrote:
> From: Lan Tianyu
>
> Using ACPI resource functions to convert ACPI resource to generic resource
>
> Signed-off-by: Lan Tianyu
Tony, any objections against this?
> ---
> This patch just passes through compilation test due to no ia64 machine on
> hand.
>
> arch/ia64/pci/pci.c | 38 +-
> 1 file changed, 21 insertions(+), 17 deletions(-)
>
> diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
> index 2326790..14fa175 100644
> --- a/arch/ia64/pci/pci.c
> +++ b/arch/ia64/pci/pci.c
> @@ -232,8 +232,9 @@ out:
> return ~0;
> }
>
> -static acpi_status resource_to_window(struct acpi_resource *resource,
> - struct acpi_resource_address64 *addr)
> +static acpi_status resource_to_window(struct acpi_resource *ares,
> + struct acpi_resource_address64 *addr,
> + struct resource *res)
> {
> acpi_status status;
>
> @@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
> acpi_resource *resource,
>* - producers, i.e., the address space is routed downstream,
>*not consumed by the bridge itself
>*/
> - status = acpi_resource_to_address64(resource, addr);
> + status = acpi_dev_resource_address_space_full(ares, addr, res);
> if (ACPI_SUCCESS(status) &&
> (addr->resource_type == ACPI_MEMORY_RANGE ||
>addr->resource_type == ACPI_IO_RANGE) &&
> @@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
> acpi_resource *resource,
> return AE_ERROR;
> }
>
> -static acpi_status count_window(struct acpi_resource *resource, void *data)
> +static acpi_status count_window(struct acpi_resource *ares, void *data)
> {
> unsigned int *windows = (unsigned int *) data;
> struct acpi_resource_address64 addr;
> + struct resource res;
> acpi_status status;
>
> - status = resource_to_window(resource, );
> + status = resource_to_window(ares, , );
> if (ACPI_SUCCESS(status))
> (*windows)++;
>
> return AE_OK;
> }
>
> -static acpi_status add_window(struct acpi_resource *res, void *data)
> +static acpi_status add_window(struct acpi_resource *ares, void *data)
> {
> struct pci_root_info *info = data;
> - struct resource *resource;
> + struct resource *resource = >res[info->res_num];
> struct acpi_resource_address64 addr;
> acpi_status status;
> - unsigned long flags, offset = 0;
> + unsigned long offset = 0;
> struct resource *root;
>
> /* Return AE_OK for non-window resources to keep scanning for more */
> - status = resource_to_window(res, );
> + status = resource_to_window(ares, , resource);
> if (!ACPI_SUCCESS(status))
> return AE_OK;
>
> - if (addr.resource_type == ACPI_MEMORY_RANGE) {
> - flags = IORESOURCE_MEM;
> + if (resource->flags & IORESOURCE_MEM) {
> root = _resource;
> offset = addr.translation_offset;
> - } else if (addr.resource_type == ACPI_IO_RANGE) {
> - flags = IORESOURCE_IO;
> + } else if (resource->flags & IORESOURCE_IO) {
> root = _resource;
> offset = add_io_space(info, );
> if (offset == ~0)
> return AE_OK;
> +
> + /*
> + * io space address translation offset depends
> + * on the return value of add_io_space(). So
> + * Repopulate resource->start and end here.
> + */
> + resource->start = addr.minimum + offset;
> + resource->end = resource->start + addr.address_length - 1;
> } else
> return AE_OK;
>
> - resource = >res[info->res_num];
> resource->name = info->name;
> - resource->flags = flags;
> - resource->start = addr.minimum + offset;
> - resource->end = resource->start + addr.address_length - 1;
> info->res_offset[info->res_num] = offset;
>
> if (insert_resource(root, resource)) {
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
On Friday, October 11, 2013 08:19:01 PM tianyu@intel.com wrote:
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com
Tony, any objections against this?
---
This patch just passes through compilation test due to no ia64 machine on
hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
- status = acpi_resource_to_address64(resource, addr);
+ status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status)
(addr-resource_type == ACPI_MEMORY_RANGE ||
addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+ struct resource res;
acpi_status status;
- status = resource_to_window(resource, addr);
+ status = resource_to_window(ares, addr, res);
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
- struct resource *resource;
+ struct resource *resource = info-res[info-res_num];
struct acpi_resource_address64 addr;
acpi_status status;
- unsigned long flags, offset = 0;
+ unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
- status = resource_to_window(res, addr);
+ status = resource_to_window(ares, addr, resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
- if (addr.resource_type == ACPI_MEMORY_RANGE) {
- flags = IORESOURCE_MEM;
+ if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
- } else if (addr.resource_type == ACPI_IO_RANGE) {
- flags = IORESOURCE_IO;
+ } else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
+
+ /*
+ * io space address translation offset depends
+ * on the return value of add_io_space(). So
+ * Repopulate resource-start and end here.
+ */
+ resource-start = addr.minimum + offset;
+ resource-end = resource-start + addr.address_length - 1;
} else
return AE_OK;
- resource = info-res[info-res_num];
resource-name = info-name;
- resource-flags = flags;
- resource-start = addr.minimum + offset;
- resource-end = resource-start + addr.address_length - 1;
info-res_offset[info-res_num] = offset;
if (insert_resource(root, resource)) {
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
From: Lan Tianyu
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu
---
This patch just passes through compilation test due to no ia64 machine on hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct acpi_resource
*resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
- status = acpi_resource_to_address64(resource, addr);
+ status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status) &&
(addr->resource_type == ACPI_MEMORY_RANGE ||
addr->resource_type == ACPI_IO_RANGE) &&
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+ struct resource res;
acpi_status status;
- status = resource_to_window(resource, );
+ status = resource_to_window(ares, , );
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
- struct resource *resource;
+ struct resource *resource = >res[info->res_num];
struct acpi_resource_address64 addr;
acpi_status status;
- unsigned long flags, offset = 0;
+ unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
- status = resource_to_window(res, );
+ status = resource_to_window(ares, , resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
- if (addr.resource_type == ACPI_MEMORY_RANGE) {
- flags = IORESOURCE_MEM;
+ if (resource->flags & IORESOURCE_MEM) {
root = _resource;
offset = addr.translation_offset;
- } else if (addr.resource_type == ACPI_IO_RANGE) {
- flags = IORESOURCE_IO;
+ } else if (resource->flags & IORESOURCE_IO) {
root = _resource;
offset = add_io_space(info, );
if (offset == ~0)
return AE_OK;
+
+ /*
+* io space address translation offset depends
+* on the return value of add_io_space(). So
+* Repopulate resource->start and end here.
+*/
+ resource->start = addr.minimum + offset;
+ resource->end = resource->start + addr.address_length - 1;
} else
return AE_OK;
- resource = >res[info->res_num];
resource->name = info->name;
- resource->flags = flags;
- resource->start = addr.minimum + offset;
- resource->end = resource->start + addr.address_length - 1;
info->res_offset[info->res_num] = offset;
if (insert_resource(root, resource)) {
--
1.8.2.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Resend PATCH 5/5] IA64/PCI/ACPI: Rework PCI root bridge ACPI resource conversion
From: Lan Tianyu tianyu@intel.com
Using ACPI resource functions to convert ACPI resource to generic resource
Signed-off-by: Lan Tianyu tianyu@intel.com
---
This patch just passes through compilation test due to no ia64 machine on hand.
arch/ia64/pci/pci.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index 2326790..14fa175 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -232,8 +232,9 @@ out:
return ~0;
}
-static acpi_status resource_to_window(struct acpi_resource *resource,
- struct acpi_resource_address64 *addr)
+static acpi_status resource_to_window(struct acpi_resource *ares,
+ struct acpi_resource_address64 *addr,
+ struct resource *res)
{
acpi_status status;
@@ -244,7 +245,7 @@ static acpi_status resource_to_window(struct acpi_resource
*resource,
* - producers, i.e., the address space is routed downstream,
*not consumed by the bridge itself
*/
- status = acpi_resource_to_address64(resource, addr);
+ status = acpi_dev_resource_address_space_full(ares, addr, res);
if (ACPI_SUCCESS(status)
(addr-resource_type == ACPI_MEMORY_RANGE ||
addr-resource_type == ACPI_IO_RANGE)
@@ -255,51 +256,54 @@ static acpi_status resource_to_window(struct
acpi_resource *resource,
return AE_ERROR;
}
-static acpi_status count_window(struct acpi_resource *resource, void *data)
+static acpi_status count_window(struct acpi_resource *ares, void *data)
{
unsigned int *windows = (unsigned int *) data;
struct acpi_resource_address64 addr;
+ struct resource res;
acpi_status status;
- status = resource_to_window(resource, addr);
+ status = resource_to_window(ares, addr, res);
if (ACPI_SUCCESS(status))
(*windows)++;
return AE_OK;
}
-static acpi_status add_window(struct acpi_resource *res, void *data)
+static acpi_status add_window(struct acpi_resource *ares, void *data)
{
struct pci_root_info *info = data;
- struct resource *resource;
+ struct resource *resource = info-res[info-res_num];
struct acpi_resource_address64 addr;
acpi_status status;
- unsigned long flags, offset = 0;
+ unsigned long offset = 0;
struct resource *root;
/* Return AE_OK for non-window resources to keep scanning for more */
- status = resource_to_window(res, addr);
+ status = resource_to_window(ares, addr, resource);
if (!ACPI_SUCCESS(status))
return AE_OK;
- if (addr.resource_type == ACPI_MEMORY_RANGE) {
- flags = IORESOURCE_MEM;
+ if (resource-flags IORESOURCE_MEM) {
root = iomem_resource;
offset = addr.translation_offset;
- } else if (addr.resource_type == ACPI_IO_RANGE) {
- flags = IORESOURCE_IO;
+ } else if (resource-flags IORESOURCE_IO) {
root = ioport_resource;
offset = add_io_space(info, addr);
if (offset == ~0)
return AE_OK;
+
+ /*
+* io space address translation offset depends
+* on the return value of add_io_space(). So
+* Repopulate resource-start and end here.
+*/
+ resource-start = addr.minimum + offset;
+ resource-end = resource-start + addr.address_length - 1;
} else
return AE_OK;
- resource = info-res[info-res_num];
resource-name = info-name;
- resource-flags = flags;
- resource-start = addr.minimum + offset;
- resource-end = resource-start + addr.address_length - 1;
info-res_offset[info-res_num] = offset;
if (insert_resource(root, resource)) {
--
1.8.2.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
