Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Marek Szyprowski]

Hi Lorenzo,

On 2016-04-19 13:30, Lorenzo Pieralisi wrote:

Hi Marek,

On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:

Hello,

On 2016-04-14 19:25, Lorenzo Pieralisi wrote:

On systems booting with ACPI, the IOMMU drivers require the same
kind of id mapping carried out with a DT tree through the of_xlate()
API in order to map devices identifiers to IOMMU ones.

On ACPI systems, since DT nodes are not present (ie struct
device.of_node == NULL), to identify the device requiring the translation
the struct device_node (and the structure used to pass translation
information - struct of_phandle_args - that contains a struct device_node)
cannot be used, so a generic translation structure to be used for IOMMU
mapping should be defined, based on the firmware agnostic fwnode_handle
type.

This patch mechanically refactors/renames the of_xlate API to make
it DT agnostic, by declaring a new type (struct iommu_fwspec), that
allows the kernel to pass a device identifier (fwnode - which can
represent either a DT node or an IOMMU FW node) and by changing the
of_xlate signature so that it does not take anymore the DT specific
of_phandle_args argument and replaces it with the DT agnostic
iommu_fwspec one.

Signed-off-by: Lorenzo Pieralisi 
Cc: Matthias Brugger 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Rob Herring 
Cc: Krzysztof Kozlowski 
Cc: Robin Murphy 
Cc: Tomasz Nowicki 
Cc: Joerg Roedel 
Cc: Marek Szyprowski 

I'm not sure if this is the right approach, although I have not enough
knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
new "fwspec" based interface? Right now of_xlate is rather common
interface used by various subsystems. Maybe it will be much easier to

Yes, that's a valid concern, when you say "it is rather common" though,
it seems to me that the of_xlate footprint is still subsystem specific,
so this patch should be self-contained anyway (granted, doing this
conversion for a specific subsystem is questionable, I guess that what
you are asking is, if you do it for IOMMU, why would not you do it for
other subsystems ?).


I was curious if you want to replace of_xlate() interface in other 
subsystems

like clocks, power domains, regulators, etc. Each of_xlate interface is
specific to particular subsystem, but they all more or less follows the same
style, what makes it easier to understand the code.


It is an RFC for this specific reason.


just add acpi_xlate callback and plumb it to the generic code? Maybe later
when similar code will be in other subsystems and drivers, it can be
unified, having much more real use cases?

Yes, that's a possibility, it means adding yet another hook into
the IOMMU drivers, probably a simpler change than this one, I
posted this code as and RFC to see which direction we want to take
so further feedback is welcome we can then choose the best approach.

Thanks,
Lorenzo


---
  drivers/iommu/arm-smmu.c | 12 +++-
  drivers/iommu/exynos-iommu.c | 11 +++
  drivers/iommu/mtk_iommu.c| 13 -
  drivers/iommu/of_iommu.c | 20 ++--
  include/linux/iommu.h| 24 
  5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6c42770..84bcff7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,18 +1440,20 @@ out_unlock:
return ret;
  }
-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
  {
struct arm_smmu_device *smmu;
-   struct platform_device *smmu_pdev;
+   struct platform_device *smmu_pdev = NULL;
+
+   if (is_of_node(args->fwnode))
+   smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
-   smmu_pdev = of_find_device_by_node(args->np);
if (!smmu_pdev)
return -ENODEV;
smmu = platform_get_drvdata(smmu_pdev);
-   return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
+   return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
  }
  static struct iommu_ops arm_smmu_ops = {
@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group   = arm_smmu_device_group,
.domain_get_attr= arm_smmu_domain_get_attr,
.domain_set_attr= arm_smmu_domain_set_attr,
-   .of_xlate   = arm_smmu_of_xlate,
+   .fw_xlate   = arm_smmu_fw_xlate,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
  };
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 5ecc86c..84ff5bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ 

Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Marek Szyprowski]

Hi Lorenzo,

On 2016-04-19 13:30, Lorenzo Pieralisi wrote:

Hi Marek,

On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:

Hello,

On 2016-04-14 19:25, Lorenzo Pieralisi wrote:

On systems booting with ACPI, the IOMMU drivers require the same
kind of id mapping carried out with a DT tree through the of_xlate()
API in order to map devices identifiers to IOMMU ones.

On ACPI systems, since DT nodes are not present (ie struct
device.of_node == NULL), to identify the device requiring the translation
the struct device_node (and the structure used to pass translation
information - struct of_phandle_args - that contains a struct device_node)
cannot be used, so a generic translation structure to be used for IOMMU
mapping should be defined, based on the firmware agnostic fwnode_handle
type.

This patch mechanically refactors/renames the of_xlate API to make
it DT agnostic, by declaring a new type (struct iommu_fwspec), that
allows the kernel to pass a device identifier (fwnode - which can
represent either a DT node or an IOMMU FW node) and by changing the
of_xlate signature so that it does not take anymore the DT specific
of_phandle_args argument and replaces it with the DT agnostic
iommu_fwspec one.

Signed-off-by: Lorenzo Pieralisi 
Cc: Matthias Brugger 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Rob Herring 
Cc: Krzysztof Kozlowski 
Cc: Robin Murphy 
Cc: Tomasz Nowicki 
Cc: Joerg Roedel 
Cc: Marek Szyprowski 

I'm not sure if this is the right approach, although I have not enough
knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
new "fwspec" based interface? Right now of_xlate is rather common
interface used by various subsystems. Maybe it will be much easier to

Yes, that's a valid concern, when you say "it is rather common" though,
it seems to me that the of_xlate footprint is still subsystem specific,
so this patch should be self-contained anyway (granted, doing this
conversion for a specific subsystem is questionable, I guess that what
you are asking is, if you do it for IOMMU, why would not you do it for
other subsystems ?).


I was curious if you want to replace of_xlate() interface in other 
subsystems

like clocks, power domains, regulators, etc. Each of_xlate interface is
specific to particular subsystem, but they all more or less follows the same
style, what makes it easier to understand the code.


It is an RFC for this specific reason.


just add acpi_xlate callback and plumb it to the generic code? Maybe later
when similar code will be in other subsystems and drivers, it can be
unified, having much more real use cases?

Yes, that's a possibility, it means adding yet another hook into
the IOMMU drivers, probably a simpler change than this one, I
posted this code as and RFC to see which direction we want to take
so further feedback is welcome we can then choose the best approach.

Thanks,
Lorenzo


---
  drivers/iommu/arm-smmu.c | 12 +++-
  drivers/iommu/exynos-iommu.c | 11 +++
  drivers/iommu/mtk_iommu.c| 13 -
  drivers/iommu/of_iommu.c | 20 ++--
  include/linux/iommu.h| 24 
  5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6c42770..84bcff7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,18 +1440,20 @@ out_unlock:
return ret;
  }
-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
  {
struct arm_smmu_device *smmu;
-   struct platform_device *smmu_pdev;
+   struct platform_device *smmu_pdev = NULL;
+
+   if (is_of_node(args->fwnode))
+   smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
-   smmu_pdev = of_find_device_by_node(args->np);
if (!smmu_pdev)
return -ENODEV;
smmu = platform_get_drvdata(smmu_pdev);
-   return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
+   return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
  }
  static struct iommu_ops arm_smmu_ops = {
@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group   = arm_smmu_device_group,
.domain_get_attr= arm_smmu_domain_get_attr,
.domain_set_attr= arm_smmu_domain_set_attr,
-   .of_xlate   = arm_smmu_of_xlate,
+   .fw_xlate   = arm_smmu_fw_xlate,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
  };
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 5ecc86c..84ff5bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device 
*dev)
iommu_group_remove_device(dev);
  }
-static int exynos_iommu_of_xlate(struct device *dev,
-

Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Lorenzo Pieralisi]

Hi Marek,

On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:
> Hello,
> 
> On 2016-04-14 19:25, Lorenzo Pieralisi wrote:
> >On systems booting with ACPI, the IOMMU drivers require the same
> >kind of id mapping carried out with a DT tree through the of_xlate()
> >API in order to map devices identifiers to IOMMU ones.
> >
> >On ACPI systems, since DT nodes are not present (ie struct
> >device.of_node == NULL), to identify the device requiring the translation
> >the struct device_node (and the structure used to pass translation
> >information - struct of_phandle_args - that contains a struct device_node)
> >cannot be used, so a generic translation structure to be used for IOMMU
> >mapping should be defined, based on the firmware agnostic fwnode_handle
> >type.
> >
> >This patch mechanically refactors/renames the of_xlate API to make
> >it DT agnostic, by declaring a new type (struct iommu_fwspec), that
> >allows the kernel to pass a device identifier (fwnode - which can
> >represent either a DT node or an IOMMU FW node) and by changing the
> >of_xlate signature so that it does not take anymore the DT specific
> >of_phandle_args argument and replaces it with the DT agnostic
> >iommu_fwspec one.
> >
> >Signed-off-by: Lorenzo Pieralisi 
> >Cc: Matthias Brugger 
> >Cc: Will Deacon 
> >Cc: Hanjun Guo 
> >Cc: Rob Herring 
> >Cc: Krzysztof Kozlowski 
> >Cc: Robin Murphy 
> >Cc: Tomasz Nowicki 
> >Cc: Joerg Roedel 
> >Cc: Marek Szyprowski 
> 
> I'm not sure if this is the right approach, although I have not enough
> knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
> new "fwspec" based interface? Right now of_xlate is rather common
> interface used by various subsystems. Maybe it will be much easier to

Yes, that's a valid concern, when you say "it is rather common" though,
it seems to me that the of_xlate footprint is still subsystem specific,
so this patch should be self-contained anyway (granted, doing this
conversion for a specific subsystem is questionable, I guess that what
you are asking is, if you do it for IOMMU, why would not you do it for
other subsystems ?).

It is an RFC for this specific reason.

> just add acpi_xlate callback and plumb it to the generic code? Maybe later
> when similar code will be in other subsystems and drivers, it can be
> unified, having much more real use cases?

Yes, that's a possibility, it means adding yet another hook into
the IOMMU drivers, probably a simpler change than this one, I
posted this code as and RFC to see which direction we want to take
so further feedback is welcome we can then choose the best approach.

Thanks,
Lorenzo

> 
> >---
> >  drivers/iommu/arm-smmu.c | 12 +++-
> >  drivers/iommu/exynos-iommu.c | 11 +++
> >  drivers/iommu/mtk_iommu.c| 13 -
> >  drivers/iommu/of_iommu.c | 20 ++--
> >  include/linux/iommu.h| 24 
> >  5 files changed, 60 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >index 6c42770..84bcff7 100644
> >--- a/drivers/iommu/arm-smmu.c
> >+++ b/drivers/iommu/arm-smmu.c
> >@@ -1440,18 +1440,20 @@ out_unlock:
> > return ret;
> >  }
> >-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args 
> >*args)
> >+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> >  {
> > struct arm_smmu_device *smmu;
> >-struct platform_device *smmu_pdev;
> >+struct platform_device *smmu_pdev = NULL;
> >+
> >+if (is_of_node(args->fwnode))
> >+smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
> >-smmu_pdev = of_find_device_by_node(args->np);
> > if (!smmu_pdev)
> > return -ENODEV;
> > smmu = platform_get_drvdata(smmu_pdev);
> >-return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
> >+return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
> >  }
> >  static struct iommu_ops arm_smmu_ops = {
> >@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
> > .device_group   = arm_smmu_device_group,
> > .domain_get_attr= arm_smmu_domain_get_attr,
> > .domain_set_attr= arm_smmu_domain_set_attr,
> >-.of_xlate   = arm_smmu_of_xlate,
> >+.fw_xlate   = arm_smmu_fw_xlate,
> > .pgsize_bitmap  = -1UL, /* Restricted during device attach */
> >  };
> >diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> >index 5ecc86c..84ff5bb 100644
> >--- a/drivers/iommu/exynos-iommu.c
> >+++ b/drivers/iommu/exynos-iommu.c
> >@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device 
> >*dev)
> > iommu_group_remove_device(dev);
> >  }
> 

Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Lorenzo Pieralisi]

Hi Marek,

On Tue, Apr 19, 2016 at 10:28:02AM +0200, Marek Szyprowski wrote:
> Hello,
> 
> On 2016-04-14 19:25, Lorenzo Pieralisi wrote:
> >On systems booting with ACPI, the IOMMU drivers require the same
> >kind of id mapping carried out with a DT tree through the of_xlate()
> >API in order to map devices identifiers to IOMMU ones.
> >
> >On ACPI systems, since DT nodes are not present (ie struct
> >device.of_node == NULL), to identify the device requiring the translation
> >the struct device_node (and the structure used to pass translation
> >information - struct of_phandle_args - that contains a struct device_node)
> >cannot be used, so a generic translation structure to be used for IOMMU
> >mapping should be defined, based on the firmware agnostic fwnode_handle
> >type.
> >
> >This patch mechanically refactors/renames the of_xlate API to make
> >it DT agnostic, by declaring a new type (struct iommu_fwspec), that
> >allows the kernel to pass a device identifier (fwnode - which can
> >represent either a DT node or an IOMMU FW node) and by changing the
> >of_xlate signature so that it does not take anymore the DT specific
> >of_phandle_args argument and replaces it with the DT agnostic
> >iommu_fwspec one.
> >
> >Signed-off-by: Lorenzo Pieralisi 
> >Cc: Matthias Brugger 
> >Cc: Will Deacon 
> >Cc: Hanjun Guo 
> >Cc: Rob Herring 
> >Cc: Krzysztof Kozlowski 
> >Cc: Robin Murphy 
> >Cc: Tomasz Nowicki 
> >Cc: Joerg Roedel 
> >Cc: Marek Szyprowski 
> 
> I'm not sure if this is the right approach, although I have not enough
> knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
> new "fwspec" based interface? Right now of_xlate is rather common
> interface used by various subsystems. Maybe it will be much easier to

Yes, that's a valid concern, when you say "it is rather common" though,
it seems to me that the of_xlate footprint is still subsystem specific,
so this patch should be self-contained anyway (granted, doing this
conversion for a specific subsystem is questionable, I guess that what
you are asking is, if you do it for IOMMU, why would not you do it for
other subsystems ?).

It is an RFC for this specific reason.

> just add acpi_xlate callback and plumb it to the generic code? Maybe later
> when similar code will be in other subsystems and drivers, it can be
> unified, having much more real use cases?

Yes, that's a possibility, it means adding yet another hook into
the IOMMU drivers, probably a simpler change than this one, I
posted this code as and RFC to see which direction we want to take
so further feedback is welcome we can then choose the best approach.

Thanks,
Lorenzo

> 
> >---
> >  drivers/iommu/arm-smmu.c | 12 +++-
> >  drivers/iommu/exynos-iommu.c | 11 +++
> >  drivers/iommu/mtk_iommu.c| 13 -
> >  drivers/iommu/of_iommu.c | 20 ++--
> >  include/linux/iommu.h| 24 
> >  5 files changed, 60 insertions(+), 20 deletions(-)
> >
> >diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> >index 6c42770..84bcff7 100644
> >--- a/drivers/iommu/arm-smmu.c
> >+++ b/drivers/iommu/arm-smmu.c
> >@@ -1440,18 +1440,20 @@ out_unlock:
> > return ret;
> >  }
> >-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args 
> >*args)
> >+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
> >  {
> > struct arm_smmu_device *smmu;
> >-struct platform_device *smmu_pdev;
> >+struct platform_device *smmu_pdev = NULL;
> >+
> >+if (is_of_node(args->fwnode))
> >+smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
> >-smmu_pdev = of_find_device_by_node(args->np);
> > if (!smmu_pdev)
> > return -ENODEV;
> > smmu = platform_get_drvdata(smmu_pdev);
> >-return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);
> >+return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
> >  }
> >  static struct iommu_ops arm_smmu_ops = {
> >@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
> > .device_group   = arm_smmu_device_group,
> > .domain_get_attr= arm_smmu_domain_get_attr,
> > .domain_set_attr= arm_smmu_domain_set_attr,
> >-.of_xlate   = arm_smmu_of_xlate,
> >+.fw_xlate   = arm_smmu_fw_xlate,
> > .pgsize_bitmap  = -1UL, /* Restricted during device attach */
> >  };
> >diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
> >index 5ecc86c..84ff5bb 100644
> >--- a/drivers/iommu/exynos-iommu.c
> >+++ b/drivers/iommu/exynos-iommu.c
> >@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device 
> >*dev)
> > iommu_group_remove_device(dev);
> >  }
> >-static int exynos_iommu_of_xlate(struct device *dev,
> >- struct of_phandle_args *spec)
> >+static int exynos_iommu_fw_xlate(struct device *dev,
> >+ struct iommu_fwspec 

Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Marek Szyprowski]

Hello,

On 2016-04-14 19:25, Lorenzo Pieralisi wrote:

On systems booting with ACPI, the IOMMU drivers require the same
kind of id mapping carried out with a DT tree through the of_xlate()
API in order to map devices identifiers to IOMMU ones.

On ACPI systems, since DT nodes are not present (ie struct
device.of_node == NULL), to identify the device requiring the translation
the struct device_node (and the structure used to pass translation
information - struct of_phandle_args - that contains a struct device_node)
cannot be used, so a generic translation structure to be used for IOMMU
mapping should be defined, based on the firmware agnostic fwnode_handle
type.

This patch mechanically refactors/renames the of_xlate API to make
it DT agnostic, by declaring a new type (struct iommu_fwspec), that
allows the kernel to pass a device identifier (fwnode - which can
represent either a DT node or an IOMMU FW node) and by changing the
of_xlate signature so that it does not take anymore the DT specific
of_phandle_args argument and replaces it with the DT agnostic
iommu_fwspec one.

Signed-off-by: Lorenzo Pieralisi 
Cc: Matthias Brugger 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Rob Herring 
Cc: Krzysztof Kozlowski 
Cc: Robin Murphy 
Cc: Tomasz Nowicki 
Cc: Joerg Roedel 
Cc: Marek Szyprowski 


I'm not sure if this is the right approach, although I have not enough
knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
new "fwspec" based interface? Right now of_xlate is rather common
interface used by various subsystems. Maybe it will be much easier to
just add acpi_xlate callback and plumb it to the generic code? Maybe later
when similar code will be in other subsystems and drivers, it can be
unified, having much more real use cases?


---
  drivers/iommu/arm-smmu.c | 12 +++-
  drivers/iommu/exynos-iommu.c | 11 +++
  drivers/iommu/mtk_iommu.c| 13 -
  drivers/iommu/of_iommu.c | 20 ++--
  include/linux/iommu.h| 24 
  5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6c42770..84bcff7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,18 +1440,20 @@ out_unlock:
return ret;
  }
  
-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)

+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
  {
struct arm_smmu_device *smmu;
-   struct platform_device *smmu_pdev;
+   struct platform_device *smmu_pdev = NULL;
+
+   if (is_of_node(args->fwnode))
+   smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
  
-	smmu_pdev = of_find_device_by_node(args->np);

if (!smmu_pdev)
return -ENODEV;
  
  	smmu = platform_get_drvdata(smmu_pdev);
  
-	return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);

+   return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
  }
  
  static struct iommu_ops arm_smmu_ops = {

@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group   = arm_smmu_device_group,
.domain_get_attr= arm_smmu_domain_get_attr,
.domain_set_attr= arm_smmu_domain_set_attr,
-   .of_xlate   = arm_smmu_of_xlate,
+   .fw_xlate   = arm_smmu_fw_xlate,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
  };
  
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c

index 5ecc86c..84ff5bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device 
*dev)
iommu_group_remove_device(dev);
  }
  
-static int exynos_iommu_of_xlate(struct device *dev,

-struct of_phandle_args *spec)
+static int exynos_iommu_fw_xlate(struct device *dev,
+struct iommu_fwspec *args)
  {
struct exynos_iommu_owner *owner = dev->archdata.iommu;
-   struct platform_device *sysmmu = of_find_device_by_node(spec->np);
+   struct platform_device *sysmmu = NULL;
struct sysmmu_drvdata *data;
  
+	if (is_of_node(args->fwnode))

+   sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
+
if (!sysmmu)
return -ENODEV;
  
@@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {

.add_device = exynos_iommu_add_device,
.remove_device = exynos_iommu_remove_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
-   .of_xlate = exynos_iommu_of_xlate,
+   .fw_xlate = exynos_iommu_fw_xlate,
  };
  
  static bool 

Re: [RFC PATCH 06/11] drivers: iommu: make of_xlate() interface DT agnostic

[Marek Szyprowski]

Hello,

On 2016-04-14 19:25, Lorenzo Pieralisi wrote:

On systems booting with ACPI, the IOMMU drivers require the same
kind of id mapping carried out with a DT tree through the of_xlate()
API in order to map devices identifiers to IOMMU ones.

On ACPI systems, since DT nodes are not present (ie struct
device.of_node == NULL), to identify the device requiring the translation
the struct device_node (and the structure used to pass translation
information - struct of_phandle_args - that contains a struct device_node)
cannot be used, so a generic translation structure to be used for IOMMU
mapping should be defined, based on the firmware agnostic fwnode_handle
type.

This patch mechanically refactors/renames the of_xlate API to make
it DT agnostic, by declaring a new type (struct iommu_fwspec), that
allows the kernel to pass a device identifier (fwnode - which can
represent either a DT node or an IOMMU FW node) and by changing the
of_xlate signature so that it does not take anymore the DT specific
of_phandle_args argument and replaces it with the DT agnostic
iommu_fwspec one.

Signed-off-by: Lorenzo Pieralisi 
Cc: Matthias Brugger 
Cc: Will Deacon 
Cc: Hanjun Guo 
Cc: Rob Herring 
Cc: Krzysztof Kozlowski 
Cc: Robin Murphy 
Cc: Tomasz Nowicki 
Cc: Joerg Roedel 
Cc: Marek Szyprowski 


I'm not sure if this is the right approach, although I have not enough
knowledge on ACPI firmware. Do you plan to rewrite all subsystems to the
new "fwspec" based interface? Right now of_xlate is rather common
interface used by various subsystems. Maybe it will be much easier to
just add acpi_xlate callback and plumb it to the generic code? Maybe later
when similar code will be in other subsystems and drivers, it can be
unified, having much more real use cases?


---
  drivers/iommu/arm-smmu.c | 12 +++-
  drivers/iommu/exynos-iommu.c | 11 +++
  drivers/iommu/mtk_iommu.c| 13 -
  drivers/iommu/of_iommu.c | 20 ++--
  include/linux/iommu.h| 24 
  5 files changed, 60 insertions(+), 20 deletions(-)

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 6c42770..84bcff7 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1440,18 +1440,20 @@ out_unlock:
return ret;
  }
  
-static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)

+static int arm_smmu_fw_xlate(struct device *dev, struct iommu_fwspec *args)
  {
struct arm_smmu_device *smmu;
-   struct platform_device *smmu_pdev;
+   struct platform_device *smmu_pdev = NULL;
+
+   if (is_of_node(args->fwnode))
+   smmu_pdev = of_find_device_by_node(to_of_node(args->fwnode));
  
-	smmu_pdev = of_find_device_by_node(args->np);

if (!smmu_pdev)
return -ENODEV;
  
  	smmu = platform_get_drvdata(smmu_pdev);
  
-	return arm_smmu_add_dev_streamid(smmu, dev, args->args[0]);

+   return arm_smmu_add_dev_streamid(smmu, dev, args->param[0]);
  }
  
  static struct iommu_ops arm_smmu_ops = {

@@ -1468,7 +1470,7 @@ static struct iommu_ops arm_smmu_ops = {
.device_group   = arm_smmu_device_group,
.domain_get_attr= arm_smmu_domain_get_attr,
.domain_set_attr= arm_smmu_domain_set_attr,
-   .of_xlate   = arm_smmu_of_xlate,
+   .fw_xlate   = arm_smmu_fw_xlate,
.pgsize_bitmap  = -1UL, /* Restricted during device attach */
  };
  
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c

index 5ecc86c..84ff5bb 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -1250,13 +1250,16 @@ static void exynos_iommu_remove_device(struct device 
*dev)
iommu_group_remove_device(dev);
  }
  
-static int exynos_iommu_of_xlate(struct device *dev,

-struct of_phandle_args *spec)
+static int exynos_iommu_fw_xlate(struct device *dev,
+struct iommu_fwspec *args)
  {
struct exynos_iommu_owner *owner = dev->archdata.iommu;
-   struct platform_device *sysmmu = of_find_device_by_node(spec->np);
+   struct platform_device *sysmmu = NULL;
struct sysmmu_drvdata *data;
  
+	if (is_of_node(args->fwnode))

+   sysmmu = of_find_device_by_node(to_of_node(args->fwnode));
+
if (!sysmmu)
return -ENODEV;
  
@@ -1290,7 +1293,7 @@ static struct iommu_ops exynos_iommu_ops = {

.add_device = exynos_iommu_add_device,
.remove_device = exynos_iommu_remove_device,
.pgsize_bitmap = SECT_SIZE | LPAGE_SIZE | SPAGE_SIZE,
-   .of_xlate = exynos_iommu_of_xlate,
+   .fw_xlate = exynos_iommu_fw_xlate,
  };
  
  static bool init_done;

diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 929a66a..e08dc0a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -436,20 +436,23 @@ static struct iommu_group