Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-08-17 Thread Hanjun Guo

On 2020/7/2 16:22, Hanjun Guo wrote:


As I said in previous email, I'm not against this patch, and seems
have no regressions for platforms that using named component node
such as D05/D06 (I will test it shortly to make sure), but it's better
to update the wording of the spec (even after this patch set is merged).


I can see that IORT revision E was updated to add IMP_DEF input
IDs in ID mappings for Named Component nodes, thanks for that.

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-07-09 Thread Hanjun Guo

On 2020/7/9 17:21, Lorenzo Pieralisi wrote:

On Thu, Jul 02, 2020 at 04:22:00PM +0800, Hanjun Guo wrote:

Hi Robin,

On 2020/7/2 0:12, Robin Murphy wrote:

On 2020-06-30 14:04, Hanjun Guo wrote:

On 2020/6/30 18:24, Lorenzo Pieralisi wrote:

On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:

[...]


For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?


The spec says "Describes the IO relationships between devices
represented in the ACPI namespace.", and in section 3.1.1.3 Named
component node, it says:


PCI devices aren't necessarily described in the ACPI namespace and we
still use IORT to describe them - through the RC node.


"Named component nodes are used to describe devices that are also
included in the Differentiated System Description Table (DSDT). See
[ACPI]."

So from my understanding, the IORT spec for now, can only do ID
translations for devices in the DSDT.


I think you can read this multiple ways but this patch does not
change this concept. What changes, is applying parent's node IORT
mapping to child nodes with no associated DSDT nodes, it is the
same thing we do with PCI and the _DMA method - we could update
the wording in the specs if that clarifies but I don't think this
deliberately disregards the specifications.


I agree, but it's better to update the wording of the spec.




For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.


Can you elaborate on this please I don't get the point you
are making.


For example, device A is not described in DSDT so can't represent
as a NC node in IORT. Device B can be described in DSDT and it
is the parent of device A, so device B can be represented in IORT
with memory access properties and node flags with Substream width
and Stall supported info.

When we trying to translate device A's ID, we reuse all the memory
access properties and node flags from its parent (device B), but
will it the same?


I assume so why wouldn't it be ? Why would be describe them in
a parent-child relationship if that's not how the system looks like
in HW ?


The point I'm making is that I'm not sure all the memory access and
stall properties are the same for the parent and the device itself.


Is that even a valid case though? The principal thing we want to
accommodate here is when device B *is* the one accessing memory, either
because it is a bridge with device A sat behind it, or because device A
is actually just some logical function or subset of physical device B.


Thanks for the clarify, for CCA attributes, child device should be the
same as its parent and that was written in the ACPI spec, so it's better
to make it clear for other properties in the spec as well.



If the topology is such that device A is a completely independent device
with its own path to memory such that it could have different
properties, I would expect that it *should* be described in DSDT, and I
can't easily think of a good reason why it wouldn't be. I'm also
struggling to imagine how it might even have an ID that had to be
interpreted in the context of device B if it wasn't one of the cases
above :/

I don't doubt that people could - or maybe even have - come up with crap
DSDT bindings that don't represent the hardware sufficiently accurately,
but I'm not sure that should be IORT's problem...


As I said in previous email, I'm not against this patch, and seems
have no regressions for platforms that using named component node
such as D05/D06 (I will test it shortly to make sure), but it's better
to update the wording of the spec (even after this patch set is merged).


Have you managed to test this series ?


Sorry, I forgot to reply the email after the testing, adding this patch
set on top of 5.8-rc3 and test it on D06, it worked well, and I printed
every match device's name to confirm that there are no regressions.

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-07-09 Thread Lorenzo Pieralisi
On Thu, Jul 02, 2020 at 04:22:00PM +0800, Hanjun Guo wrote:
> Hi Robin,
> 
> On 2020/7/2 0:12, Robin Murphy wrote:
> > On 2020-06-30 14:04, Hanjun Guo wrote:
> > > On 2020/6/30 18:24, Lorenzo Pieralisi wrote:
> > > > On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > > For devices that aren't described in the DSDT - IORT translations
> > > > > > are determined by their ACPI parent device. Do you see/Have you
> > > > > > found any issue with this approach ?
> > > > > 
> > > > > The spec says "Describes the IO relationships between devices
> > > > > represented in the ACPI namespace.", and in section 3.1.1.3 Named
> > > > > component node, it says:
> > > > 
> > > > PCI devices aren't necessarily described in the ACPI namespace and we
> > > > still use IORT to describe them - through the RC node.
> > > > 
> > > > > "Named component nodes are used to describe devices that are also
> > > > > included in the Differentiated System Description Table (DSDT). See
> > > > > [ACPI]."
> > > > > 
> > > > > So from my understanding, the IORT spec for now, can only do ID
> > > > > translations for devices in the DSDT.
> > > > 
> > > > I think you can read this multiple ways but this patch does not
> > > > change this concept. What changes, is applying parent's node IORT
> > > > mapping to child nodes with no associated DSDT nodes, it is the
> > > > same thing we do with PCI and the _DMA method - we could update
> > > > the wording in the specs if that clarifies but I don't think this
> > > > deliberately disregards the specifications.
> > > 
> > > I agree, but it's better to update the wording of the spec.
> > > 
> > > > 
> > > > > > > For a platform device, if I use its parent's full path name for
> > > > > > > its named component entry, then it will match, but this will 
> > > > > > > violate
> > > > > > > the IORT spec.
> > > > > > 
> > > > > > Can you elaborate on this please I don't get the point you
> > > > > > are making.
> > > > > 
> > > > > For example, device A is not described in DSDT so can't represent
> > > > > as a NC node in IORT. Device B can be described in DSDT and it
> > > > > is the parent of device A, so device B can be represented in IORT
> > > > > with memory access properties and node flags with Substream width
> > > > > and Stall supported info.
> > > > > 
> > > > > When we trying to translate device A's ID, we reuse all the memory
> > > > > access properties and node flags from its parent (device B), but
> > > > > will it the same?
> > > > 
> > > > I assume so why wouldn't it be ? Why would be describe them in
> > > > a parent-child relationship if that's not how the system looks like
> > > > in HW ?
> > > 
> > > The point I'm making is that I'm not sure all the memory access and
> > > stall properties are the same for the parent and the device itself.
> > 
> > Is that even a valid case though? The principal thing we want to
> > accommodate here is when device B *is* the one accessing memory, either
> > because it is a bridge with device A sat behind it, or because device A
> > is actually just some logical function or subset of physical device B.
> 
> Thanks for the clarify, for CCA attributes, child device should be the
> same as its parent and that was written in the ACPI spec, so it's better
> to make it clear for other properties in the spec as well.
> 
> > 
> > If the topology is such that device A is a completely independent device
> > with its own path to memory such that it could have different
> > properties, I would expect that it *should* be described in DSDT, and I
> > can't easily think of a good reason why it wouldn't be. I'm also
> > struggling to imagine how it might even have an ID that had to be
> > interpreted in the context of device B if it wasn't one of the cases
> > above :/
> > 
> > I don't doubt that people could - or maybe even have - come up with crap
> > DSDT bindings that don't represent the hardware sufficiently accurately,
> > but I'm not sure that should be IORT's problem...
> 
> As I said in previous email, I'm not against this patch, and seems
> have no regressions for platforms that using named component node
> such as D05/D06 (I will test it shortly to make sure), but it's better
> to update the wording of the spec (even after this patch set is merged).

Have you managed to test this series ?

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-07-02 Thread Hanjun Guo

Hi Robin,

On 2020/7/2 0:12, Robin Murphy wrote:

On 2020-06-30 14:04, Hanjun Guo wrote:

On 2020/6/30 18:24, Lorenzo Pieralisi wrote:

On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:

[...]


For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?


The spec says "Describes the IO relationships between devices
represented in the ACPI namespace.", and in section 3.1.1.3 Named
component node, it says:


PCI devices aren't necessarily described in the ACPI namespace and we
still use IORT to describe them - through the RC node.


"Named component nodes are used to describe devices that are also
included in the Differentiated System Description Table (DSDT). See
[ACPI]."

So from my understanding, the IORT spec for now, can only do ID
translations for devices in the DSDT.


I think you can read this multiple ways but this patch does not
change this concept. What changes, is applying parent's node IORT
mapping to child nodes with no associated DSDT nodes, it is the
same thing we do with PCI and the _DMA method - we could update
the wording in the specs if that clarifies but I don't think this
deliberately disregards the specifications.


I agree, but it's better to update the wording of the spec.




For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.


Can you elaborate on this please I don't get the point you
are making.


For example, device A is not described in DSDT so can't represent
as a NC node in IORT. Device B can be described in DSDT and it
is the parent of device A, so device B can be represented in IORT
with memory access properties and node flags with Substream width
and Stall supported info.

When we trying to translate device A's ID, we reuse all the memory
access properties and node flags from its parent (device B), but
will it the same?


I assume so why wouldn't it be ? Why would be describe them in
a parent-child relationship if that's not how the system looks like
in HW ?


The point I'm making is that I'm not sure all the memory access and
stall properties are the same for the parent and the device itself.


Is that even a valid case though? The principal thing we want to 
accommodate here is when device B *is* the one accessing memory, either 
because it is a bridge with device A sat behind it, or because device A 
is actually just some logical function or subset of physical device B.


Thanks for the clarify, for CCA attributes, child device should be the
same as its parent and that was written in the ACPI spec, so it's better
to make it clear for other properties in the spec as well.



If the topology is such that device A is a completely independent device 
with its own path to memory such that it could have different 
properties, I would expect that it *should* be described in DSDT, and I 
can't easily think of a good reason why it wouldn't be. I'm also 
struggling to imagine how it might even have an ID that had to be 
interpreted in the context of device B if it wasn't one of the cases 
above :/


I don't doubt that people could - or maybe even have - come up with crap 
DSDT bindings that don't represent the hardware sufficiently accurately, 
but I'm not sure that should be IORT's problem...


As I said in previous email, I'm not against this patch, and seems
have no regressions for platforms that using named component node
such as D05/D06 (I will test it shortly to make sure), but it's better
to update the wording of the spec (even after this patch set is merged).

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-07-01 Thread Robin Murphy

On 2020-06-30 14:04, Hanjun Guo wrote:

On 2020/6/30 18:24, Lorenzo Pieralisi wrote:

On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:

[...]


For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?


The spec says "Describes the IO relationships between devices
represented in the ACPI namespace.", and in section 3.1.1.3 Named
component node, it says:


PCI devices aren't necessarily described in the ACPI namespace and we
still use IORT to describe them - through the RC node.


"Named component nodes are used to describe devices that are also
included in the Differentiated System Description Table (DSDT). See
[ACPI]."

So from my understanding, the IORT spec for now, can only do ID
translations for devices in the DSDT.


I think you can read this multiple ways but this patch does not
change this concept. What changes, is applying parent's node IORT
mapping to child nodes with no associated DSDT nodes, it is the
same thing we do with PCI and the _DMA method - we could update
the wording in the specs if that clarifies but I don't think this
deliberately disregards the specifications.


I agree, but it's better to update the wording of the spec.




For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.


Can you elaborate on this please I don't get the point you
are making.


For example, device A is not described in DSDT so can't represent
as a NC node in IORT. Device B can be described in DSDT and it
is the parent of device A, so device B can be represented in IORT
with memory access properties and node flags with Substream width
and Stall supported info.

When we trying to translate device A's ID, we reuse all the memory
access properties and node flags from its parent (device B), but
will it the same?


I assume so why wouldn't it be ? Why would be describe them in
a parent-child relationship if that's not how the system looks like
in HW ?


The point I'm making is that I'm not sure all the memory access and
stall properties are the same for the parent and the device itself.


Is that even a valid case though? The principal thing we want to 
accommodate here is when device B *is* the one accessing memory, either 
because it is a bridge with device A sat behind it, or because device A 
is actually just some logical function or subset of physical device B.


If the topology is such that device A is a completely independent device 
with its own path to memory such that it could have different 
properties, I would expect that it *should* be described in DSDT, and I 
can't easily think of a good reason why it wouldn't be. I'm also 
struggling to imagine how it might even have an ID that had to be 
interpreted in the context of device B if it wasn't one of the cases 
above :/


I don't doubt that people could - or maybe even have - come up with crap 
DSDT bindings that don't represent the hardware sufficiently accurately, 
but I'm not sure that should be IORT's problem...


Robin.


Do you have a specific example in mind that we should be aware of ?


So the IORT spec don't support this, at least it's pretty vague
I think.


I think that's a matter of wording, it can be updated if it needs be,
reach out if you see any issue with the current approach please.


If the all the properties for parent and device itself are the same,
I have no strong opinion for this patch, but it's better to update
the wording of the spec as well.

Thanks
Hanjun


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-30 Thread Hanjun Guo

On 2020/6/30 18:24, Lorenzo Pieralisi wrote:

On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:

[...]


For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?


The spec says "Describes the IO relationships between devices
represented in the ACPI namespace.", and in section 3.1.1.3 Named
component node, it says:


PCI devices aren't necessarily described in the ACPI namespace and we
still use IORT to describe them - through the RC node.


"Named component nodes are used to describe devices that are also
included in the Differentiated System Description Table (DSDT). See
[ACPI]."

So from my understanding, the IORT spec for now, can only do ID
translations for devices in the DSDT.


I think you can read this multiple ways but this patch does not
change this concept. What changes, is applying parent's node IORT
mapping to child nodes with no associated DSDT nodes, it is the
same thing we do with PCI and the _DMA method - we could update
the wording in the specs if that clarifies but I don't think this
deliberately disregards the specifications.


I agree, but it's better to update the wording of the spec.




For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.


Can you elaborate on this please I don't get the point you
are making.


For example, device A is not described in DSDT so can't represent
as a NC node in IORT. Device B can be described in DSDT and it
is the parent of device A, so device B can be represented in IORT
with memory access properties and node flags with Substream width
and Stall supported info.

When we trying to translate device A's ID, we reuse all the memory
access properties and node flags from its parent (device B), but
will it the same?


I assume so why wouldn't it be ? Why would be describe them in
a parent-child relationship if that's not how the system looks like
in HW ?


The point I'm making is that I'm not sure all the memory access and
stall properties are the same for the parent and the device itself.



Do you have a specific example in mind that we should be aware of ?


So the IORT spec don't support this, at least it's pretty vague
I think.


I think that's a matter of wording, it can be updated if it needs be,
reach out if you see any issue with the current approach please.


If the all the properties for parent and device itself are the same,
I have no strong opinion for this patch, but it's better to update
the wording of the spec as well.

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-30 Thread Lorenzo Pieralisi
On Tue, Jun 30, 2020 at 11:06:41AM +0800, Hanjun Guo wrote:

[...]

> > For devices that aren't described in the DSDT - IORT translations
> > are determined by their ACPI parent device. Do you see/Have you
> > found any issue with this approach ?
> 
> The spec says "Describes the IO relationships between devices
> represented in the ACPI namespace.", and in section 3.1.1.3 Named
> component node, it says:

PCI devices aren't necessarily described in the ACPI namespace and we
still use IORT to describe them - through the RC node.

> "Named component nodes are used to describe devices that are also
> included in the Differentiated System Description Table (DSDT). See
> [ACPI]."
> 
> So from my understanding, the IORT spec for now, can only do ID
> translations for devices in the DSDT.

I think you can read this multiple ways but this patch does not
change this concept. What changes, is applying parent's node IORT
mapping to child nodes with no associated DSDT nodes, it is the
same thing we do with PCI and the _DMA method - we could update
the wording in the specs if that clarifies but I don't think this
deliberately disregards the specifications.

> > > For a platform device, if I use its parent's full path name for
> > > its named component entry, then it will match, but this will violate
> > > the IORT spec.
> > 
> > Can you elaborate on this please I don't get the point you
> > are making.
> 
> For example, device A is not described in DSDT so can't represent
> as a NC node in IORT. Device B can be described in DSDT and it
> is the parent of device A, so device B can be represented in IORT
> with memory access properties and node flags with Substream width
> and Stall supported info.
> 
> When we trying to translate device A's ID, we reuse all the memory
> access properties and node flags from its parent (device B), but
> will it the same?

I assume so why wouldn't it be ? Why would be describe them in
a parent-child relationship if that's not how the system looks like
in HW ?

Do you have a specific example in mind that we should be aware of ?

> So the IORT spec don't support this, at least it's pretty vague
> I think.

I think that's a matter of wording, it can be updated if it needs be,
reach out if you see any issue with the current approach please.

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-29 Thread Hanjun Guo

On 2020/6/29 17:05, Lorenzo Pieralisi wrote:

On Mon, Jun 29, 2020 at 12:24:43PM +0800, Hanjun Guo wrote:

Hi Lorenzo,

On 2020/6/19 16:20, Lorenzo Pieralisi wrote:

When the iort_match_node_callback is invoked for a named component
the match should be executed upon a device with an ACPI companion.

For devices with no ACPI companion set-up the ACPI device tree must be
walked in order to find the first parent node with a companion set and
check the parent node against the named component entry to check whether
there is a match and therefore an IORT node describing the in/out ID
translation for the device has been found.

Signed-off-by: Lorenzo Pieralisi 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Sudeep Holla 
Cc: Catalin Marinas 
Cc: Robin Murphy 
Cc: "Rafael J. Wysocki" 
---
   drivers/acpi/arm64/iort.c | 20 ++--
   1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 28a6b387e80e..5eee81758184 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -264,15 +264,31 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
-   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+   struct acpi_device *adev;
struct acpi_iort_named_component *ncomp;
+   struct device *nc_dev = dev;
+
+   /*
+* Walk the device tree to find a device with an
+* ACPI companion; there is no point in scanning
+* IORT for a device matching a named component if
+* the device does not have an ACPI companion to
+* start with.
+*/
+   do {
+   adev = ACPI_COMPANION(nc_dev);
+   if (adev)
+   break;
+
+   nc_dev = nc_dev->parent;
+   } while (nc_dev);


I'm lost here, we need the ACPI_COMPANION (the same as
to_acpi_device_node()) of the device, but why do we need to go
up to find the parent node?


For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?


The spec says "Describes the IO relationships between devices
represented in the ACPI namespace.", and in section 3.1.1.3 Named
component node, it says:

"Named component nodes are used to describe devices that are also
included in the Differentiated System Description Table (DSDT). See
[ACPI]."

So from my understanding, the IORT spec for now, can only do ID
translations for devices in the DSDT.




For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.


Can you elaborate on this please I don't get the point you
are making.


For example, device A is not described in DSDT so can't represent
as a NC node in IORT. Device B can be described in DSDT and it
is the parent of device A, so device B can be represented in IORT
with memory access properties and node flags with Substream width
and Stall supported info.

When we trying to translate device A's ID, we reuse all the memory
access properties and node flags from its parent (device B), but
will it the same?

So the IORT spec don't support this, at least it's pretty vague
I think.

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-29 Thread Lorenzo Pieralisi
On Mon, Jun 29, 2020 at 12:24:43PM +0800, Hanjun Guo wrote:
> Hi Lorenzo,
> 
> On 2020/6/19 16:20, Lorenzo Pieralisi wrote:
> > When the iort_match_node_callback is invoked for a named component
> > the match should be executed upon a device with an ACPI companion.
> > 
> > For devices with no ACPI companion set-up the ACPI device tree must be
> > walked in order to find the first parent node with a companion set and
> > check the parent node against the named component entry to check whether
> > there is a match and therefore an IORT node describing the in/out ID
> > translation for the device has been found.
> > 
> > Signed-off-by: Lorenzo Pieralisi 
> > Cc: Will Deacon 
> > Cc: Hanjun Guo 
> > Cc: Sudeep Holla 
> > Cc: Catalin Marinas 
> > Cc: Robin Murphy 
> > Cc: "Rafael J. Wysocki" 
> > ---
> >   drivers/acpi/arm64/iort.c | 20 ++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 28a6b387e80e..5eee81758184 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -264,15 +264,31 @@ static acpi_status iort_match_node_callback(struct 
> > acpi_iort_node *node,
> > if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
> > struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
> > -   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
> > +   struct acpi_device *adev;
> > struct acpi_iort_named_component *ncomp;
> > +   struct device *nc_dev = dev;
> > +
> > +   /*
> > +* Walk the device tree to find a device with an
> > +* ACPI companion; there is no point in scanning
> > +* IORT for a device matching a named component if
> > +* the device does not have an ACPI companion to
> > +* start with.
> > +*/
> > +   do {
> > +   adev = ACPI_COMPANION(nc_dev);
> > +   if (adev)
> > +   break;
> > +
> > +   nc_dev = nc_dev->parent;
> > +   } while (nc_dev);
> 
> I'm lost here, we need the ACPI_COMPANION (the same as
> to_acpi_device_node()) of the device, but why do we need to go
> up to find the parent node?

For devices that aren't described in the DSDT - IORT translations
are determined by their ACPI parent device. Do you see/Have you
found any issue with this approach ?

> For a platform device, if I use its parent's full path name for
> its named component entry, then it will match, but this will violate
> the IORT spec.

Can you elaborate on this please I don't get the point you
are making.

Thanks,
Lorenzo
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-28 Thread Hanjun Guo

Hi Lorenzo,

On 2020/6/19 16:20, Lorenzo Pieralisi wrote:

When the iort_match_node_callback is invoked for a named component
the match should be executed upon a device with an ACPI companion.

For devices with no ACPI companion set-up the ACPI device tree must be
walked in order to find the first parent node with a companion set and
check the parent node against the named component entry to check whether
there is a match and therefore an IORT node describing the in/out ID
translation for the device has been found.

Signed-off-by: Lorenzo Pieralisi 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Sudeep Holla 
Cc: Catalin Marinas 
Cc: Robin Murphy 
Cc: "Rafael J. Wysocki" 
---
  drivers/acpi/arm64/iort.c | 20 ++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 28a6b387e80e..5eee81758184 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -264,15 +264,31 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
  
  	if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {

struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
-   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+   struct acpi_device *adev;
struct acpi_iort_named_component *ncomp;
+   struct device *nc_dev = dev;
+
+   /*
+* Walk the device tree to find a device with an
+* ACPI companion; there is no point in scanning
+* IORT for a device matching a named component if
+* the device does not have an ACPI companion to
+* start with.
+*/
+   do {
+   adev = ACPI_COMPANION(nc_dev);
+   if (adev)
+   break;
+
+   nc_dev = nc_dev->parent;
+   } while (nc_dev);


I'm lost here, we need the ACPI_COMPANION (the same as
to_acpi_device_node()) of the device, but why do we need to go
up to find the parent node?

For a platform device, if I use its parent's full path name for
its named component entry, then it will match, but this will violate
the IORT spec.

Thanks
Hanjun

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


[PATCH v2 01/12] ACPI/IORT: Make iort_match_node_callback walk the ACPI namespace for NC

2020-06-19 Thread Lorenzo Pieralisi
When the iort_match_node_callback is invoked for a named component
the match should be executed upon a device with an ACPI companion.

For devices with no ACPI companion set-up the ACPI device tree must be
walked in order to find the first parent node with a companion set and
check the parent node against the named component entry to check whether
there is a match and therefore an IORT node describing the in/out ID
translation for the device has been found.

Signed-off-by: Lorenzo Pieralisi 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Sudeep Holla 
Cc: Catalin Marinas 
Cc: Robin Murphy 
Cc: "Rafael J. Wysocki" 
---
 drivers/acpi/arm64/iort.c | 20 ++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 28a6b387e80e..5eee81758184 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -264,15 +264,31 @@ static acpi_status iort_match_node_callback(struct 
acpi_iort_node *node,
 
if (node->type == ACPI_IORT_NODE_NAMED_COMPONENT) {
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
-   struct acpi_device *adev = to_acpi_device_node(dev->fwnode);
+   struct acpi_device *adev;
struct acpi_iort_named_component *ncomp;
+   struct device *nc_dev = dev;
+
+   /*
+* Walk the device tree to find a device with an
+* ACPI companion; there is no point in scanning
+* IORT for a device matching a named component if
+* the device does not have an ACPI companion to
+* start with.
+*/
+   do {
+   adev = ACPI_COMPANION(nc_dev);
+   if (adev)
+   break;
+
+   nc_dev = nc_dev->parent;
+   } while (nc_dev);
 
if (!adev)
goto out;
 
status = acpi_get_name(adev->handle, ACPI_FULL_PATHNAME, &buf);
if (ACPI_FAILURE(status)) {
-   dev_warn(dev, "Can't get device full path name\n");
+   dev_warn(nc_dev, "Can't get device full path name\n");
goto out;
}
 
-- 
2.26.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu