[PATCH 0/3] ACPI/IORT: Clean-up/rework for v4.15

[PATCH 0/3] ACPI/IORT: Clean-up/rework for v4.15

[Lorenzo Pieralisi]

A three-patch series collection of backlog clean-ups/rework in
preparation for future code restructuring and features support.

Patch series depends on:

https://marc.info/?l=linux-arm-kernel&m=150696522211494&w=2

Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>

Lorenzo Pieralisi (3):
  ACPI/IORT: Remove leftover ACPI_IORT_SMMU_V3_PXM_VALID guard
  ACPI/IORT: Improve functions return type/storage class specifier
    indentation
  ACPI/IORT: Make platform devices initialization code SMMU agnostic

 drivers/acpi/arm64/iort.c | 83 ++++++++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 44 deletions(-)

-- 
2.4.12

[PATCH 1/3] ACPI/IORT: Remove leftover ACPI_IORT_SMMU_V3_PXM_VALID guard

[Lorenzo Pieralisi]

The conditional ACPI_IORT_SMMU_V3_PXM_VALID guard around
arm_smmu_v3_set_proximity() was added to manage a cross tree
ACPICA merge dependency; with ACPICA changes merged in:

commit c944230064eb ("ACPICA: iasl: Update to IORT SMMUv3
disassembling")

the guard has become useless. Remove it.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
---
 drivers/acpi/arm64/iort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index de56394..69f21f3 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -968,7 +968,7 @@ static bool __init arm_smmu_v3_is_coherent(struct acpi_iort_node *node)
 	return smmu->flags & ACPI_IORT_SMMU_V3_COHACC_OVERRIDE;
 }
 
-#if defined(CONFIG_ACPI_NUMA) && defined(ACPI_IORT_SMMU_V3_PXM_VALID)
+#if defined(CONFIG_ACPI_NUMA)
 /*
  * set numa proximity domain for smmuv3 device
  */
-- 
2.4.12

[PATCH 1/3] ACPI/IORT: Remove leftover ACPI_IORT_SMMU_V3_PXM_VALID guard

[Hanjun Guo]

On 2017/10/3 21:53, Lorenzo Pieralisi wrote:
> The conditional ACPI_IORT_SMMU_V3_PXM_VALID guard around
> arm_smmu_v3_set_proximity() was added to manage a cross tree
> ACPICA merge dependency; with ACPICA changes merged in:
>
> commit c944230064eb ("ACPICA: iasl: Update to IORT SMMUv3
> disassembling")
>
> the guard has become useless. Remove it.
>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> Cc: Ganapatrao Kulkarni <ganapatrao.kulkarni@cavium.com>
>

Acked-by: Hanjun Guo <hanjun.guo@linaro.org>

[PATCH 2/3] ACPI/IORT: Improve functions return type/storage class specifier indentation

[Lorenzo Pieralisi]

Some functions definition indentations are using a style that is frowned
upon with return value type/storage class specifier in a separate line.

Reindent the function definitions to fix them.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/arm64/iort.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index 69f21f3..f4775e7 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -88,8 +88,8 @@ static inline int iort_set_fwnode(struct acpi_iort_node *iort_node,
  *
  * Returns: fwnode_handle pointer on success, NULL on failure
  */
-static inline
-struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
+static inline struct fwnode_handle *iort_get_fwnode(
+			struct acpi_iort_node *node)
 {
 	struct iort_fwnode *curr;
 	struct fwnode_handle *fwnode = NULL;
@@ -306,9 +306,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
 	return 0;
 }
 
-static
-struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
-					u32 *id_out, int index)
+static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
+					       u32 *id_out, int index)
 {
 	struct acpi_iort_node *parent;
 	struct acpi_iort_id_mapping *map;
@@ -392,10 +391,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
 	return NULL;
 }
 
-static
-struct acpi_iort_node *iort_node_map_platform_id(struct acpi_iort_node *node,
-						 u32 *id_out, u8 type_mask,
-						 int index)
+static struct acpi_iort_node *iort_node_map_platform_id(
+		struct acpi_iort_node *node, u32 *id_out, u8 type_mask,
+		int index)
 {
 	struct acpi_iort_node *parent;
 	u32 id;
@@ -1077,8 +1075,8 @@ static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
 	.iommu_init_resources = arm_smmu_init_resources
 };
 
-static __init
-const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
+static __init const struct iort_dev_config *iort_get_dev_cfg(
+			struct acpi_iort_node *node)
 {
 	switch (node->type) {
 	case ACPI_IORT_NODE_SMMU_V3:
-- 
2.4.12

[PATCH 2/3] ACPI/IORT: Improve functions return type/storage class specifier indentation

[Hanjun Guo]

On 2017/10/3 21:53, Lorenzo Pieralisi wrote:
> Some functions definition indentations are using a style that is frowned
> upon with return value type/storage class specifier in a separate line.
>
> Reindent the function definitions to fix them.

Shall we fix the functions which guarded with #ifdef CONFIG_IOMMU_API
as well?

Thanks
Hanjun

>
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Hanjun Guo <hanjun.guo@linaro.org>
> Cc: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/acpi/arm64/iort.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 69f21f3..f4775e7 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -88,8 +88,8 @@ static inline int iort_set_fwnode(struct acpi_iort_node *iort_node,
>   *
>   * Returns: fwnode_handle pointer on success, NULL on failure
>   */
> -static inline
> -struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
> +static inline struct fwnode_handle *iort_get_fwnode(
> +			struct acpi_iort_node *node)
>  {
>  	struct iort_fwnode *curr;
>  	struct fwnode_handle *fwnode = NULL;
> @@ -306,9 +306,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
>  	return 0;
>  }
>  
> -static
> -struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> -					u32 *id_out, int index)
> +static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> +					       u32 *id_out, int index)
>  {
>  	struct acpi_iort_node *parent;
>  	struct acpi_iort_id_mapping *map;
> @@ -392,10 +391,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
>  	return NULL;
>  }
>  
> -static
> -struct acpi_iort_node *iort_node_map_platform_id(struct acpi_iort_node *node,
> -						 u32 *id_out, u8 type_mask,
> -						 int index)
> +static struct acpi_iort_node *iort_node_map_platform_id(
> +		struct acpi_iort_node *node, u32 *id_out, u8 type_mask,
> +		int index)
>  {
>  	struct acpi_iort_node *parent;
>  	u32 id;
> @@ -1077,8 +1075,8 @@ static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
>  	.iommu_init_resources = arm_smmu_init_resources
>  };
>  
> -static __init
> -const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
> +static __init const struct iort_dev_config *iort_get_dev_cfg(
> +			struct acpi_iort_node *node)
>  {
>  	switch (node->type) {
>  	case ACPI_IORT_NODE_SMMU_V3:

[PATCH 2/3] ACPI/IORT: Improve functions return type/storage class specifier indentation

[Hanjun Guo]

On 2017/10/10 16:03, Hanjun Guo wrote:
...

>> -static __init
>> -const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
>> +static __init const struct iort_dev_config *iort_get_dev_cfg(

Seems patch rebase issue, this part should be in patch 3/3 :)

Thanks
Hanjun

[PATCH 2/3] ACPI/IORT: Improve functions return type/storage class specifier indentation

[Lorenzo Pieralisi]

On Tue, Oct 10, 2017 at 05:03:40PM +0800, Hanjun Guo wrote:
> On 2017/10/10 16:03, Hanjun Guo wrote:
> ...
> 
> >> -static __init
> >> -const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
> >> +static __init const struct iort_dev_config *iort_get_dev_cfg(
> 
> Seems patch rebase issue, this part should be in patch 3/3 :)

True, good catch, updated, thanks !

Lorenzo

[PATCH 2/3] ACPI/IORT: Improve functions return type/storage class specifier indentation

[Lorenzo Pieralisi]

On Tue, Oct 10, 2017 at 04:03:34PM +0800, Hanjun Guo wrote:
> On 2017/10/3 21:53, Lorenzo Pieralisi wrote:
> > Some functions definition indentations are using a style that is frowned
> > upon with return value type/storage class specifier in a separate line.
> >
> > Reindent the function definitions to fix them.
> 
> Shall we fix the functions which guarded with #ifdef CONFIG_IOMMU_API
> as well?

Yes I have a patch for that, I shall see if I can manage to squeeze it
in for 4.15.

Thanks,
Lorenzo

> Thanks
> Hanjun
> 
> >
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Hanjun Guo <hanjun.guo@linaro.org>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> >  drivers/acpi/arm64/iort.c | 20 +++++++++-----------
> >  1 file changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 69f21f3..f4775e7 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -88,8 +88,8 @@ static inline int iort_set_fwnode(struct acpi_iort_node *iort_node,
> >   *
> >   * Returns: fwnode_handle pointer on success, NULL on failure
> >   */
> > -static inline
> > -struct fwnode_handle *iort_get_fwnode(struct acpi_iort_node *node)
> > +static inline struct fwnode_handle *iort_get_fwnode(
> > +			struct acpi_iort_node *node)
> >  {
> >  	struct iort_fwnode *curr;
> >  	struct fwnode_handle *fwnode = NULL;
> > @@ -306,9 +306,8 @@ static int iort_id_map(struct acpi_iort_id_mapping *map, u8 type, u32 rid_in,
> >  	return 0;
> >  }
> >  
> > -static
> > -struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> > -					u32 *id_out, int index)
> > +static struct acpi_iort_node *iort_node_get_id(struct acpi_iort_node *node,
> > +					       u32 *id_out, int index)
> >  {
> >  	struct acpi_iort_node *parent;
> >  	struct acpi_iort_id_mapping *map;
> > @@ -392,10 +391,9 @@ static struct acpi_iort_node *iort_node_map_id(struct acpi_iort_node *node,
> >  	return NULL;
> >  }
> >  
> > -static
> > -struct acpi_iort_node *iort_node_map_platform_id(struct acpi_iort_node *node,
> > -						 u32 *id_out, u8 type_mask,
> > -						 int index)
> > +static struct acpi_iort_node *iort_node_map_platform_id(
> > +		struct acpi_iort_node *node, u32 *id_out, u8 type_mask,
> > +		int index)
> >  {
> >  	struct acpi_iort_node *parent;
> >  	u32 id;
> > @@ -1077,8 +1075,8 @@ static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
> >  	.iommu_init_resources = arm_smmu_init_resources
> >  };
> >  
> > -static __init
> > -const struct iort_iommu_config *iort_get_iommu_cfg(struct acpi_iort_node *node)
> > +static __init const struct iort_dev_config *iort_get_dev_cfg(
> > +			struct acpi_iort_node *node)
> >  {
> >  	switch (node->type) {
> >  	case ACPI_IORT_NODE_SMMU_V3:
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] ACPI/IORT: Make platform devices initialization code SMMU agnostic

[Lorenzo Pieralisi]

The way current IORT code initializes platform devices for SMMU nodes
is somewhat tied (mostly for naming convention) to the SMMU nodes
themselves but it need not be in that it is completely generic and
can easily be made so by structures renaming and code reshuffling.

Rework IORT platform devices initialization code to make the functions
and data structures SMMU agnostic.

No functional changes intended.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Hanjun Guo <hanjun.guo@linaro.org>
Cc: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/acpi/arm64/iort.c | 61 ++++++++++++++++++++++-------------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index f4775e7..271ec1f 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1049,30 +1049,30 @@ static bool __init arm_smmu_is_coherent(struct acpi_iort_node *node)
 	return smmu->flags & ACPI_IORT_SMMU_COHERENT_WALK;
 }
 
-struct iort_iommu_config {
+struct iort_dev_config {
 	const char *name;
-	int (*iommu_init)(struct acpi_iort_node *node);
-	bool (*iommu_is_coherent)(struct acpi_iort_node *node);
-	int (*iommu_count_resources)(struct acpi_iort_node *node);
-	void (*iommu_init_resources)(struct resource *res,
+	int (*dev_init)(struct acpi_iort_node *node);
+	bool (*dev_is_coherent)(struct acpi_iort_node *node);
+	int (*dev_count_resources)(struct acpi_iort_node *node);
+	void (*dev_init_resources)(struct resource *res,
 				     struct acpi_iort_node *node);
-	void (*iommu_set_proximity)(struct device *dev,
+	void (*dev_set_proximity)(struct device *dev,
 				    struct acpi_iort_node *node);
 };
 
-static const struct iort_iommu_config iort_arm_smmu_v3_cfg __initconst = {
+static const struct iort_dev_config iort_arm_smmu_v3_cfg __initconst = {
 	.name = "arm-smmu-v3",
-	.iommu_is_coherent = arm_smmu_v3_is_coherent,
-	.iommu_count_resources = arm_smmu_v3_count_resources,
-	.iommu_init_resources = arm_smmu_v3_init_resources,
-	.iommu_set_proximity = arm_smmu_v3_set_proximity,
+	.dev_is_coherent = arm_smmu_v3_is_coherent,
+	.dev_count_resources = arm_smmu_v3_count_resources,
+	.dev_init_resources = arm_smmu_v3_init_resources,
+	.dev_set_proximity = arm_smmu_v3_set_proximity,
 };
 
-static const struct iort_iommu_config iort_arm_smmu_cfg __initconst = {
+static const struct iort_dev_config iort_arm_smmu_cfg __initconst = {
 	.name = "arm-smmu",
-	.iommu_is_coherent = arm_smmu_is_coherent,
-	.iommu_count_resources = arm_smmu_count_resources,
-	.iommu_init_resources = arm_smmu_init_resources
+	.dev_is_coherent = arm_smmu_is_coherent,
+	.dev_count_resources = arm_smmu_count_resources,
+	.dev_init_resources = arm_smmu_init_resources
 };
 
 static __init const struct iort_dev_config *iort_get_dev_cfg(
@@ -1089,31 +1089,28 @@ static __init const struct iort_dev_config *iort_get_dev_cfg(
 }
 
 /**
- * iort_add_smmu_platform_device() - Allocate a platform device for SMMU
- * @node: Pointer to SMMU ACPI IORT node
+ * iort_add_platform_device() - Allocate a platform device for IORT node
+ * @node: Pointer to device ACPI IORT node
  *
  * Returns: 0 on success, <0 failure
  */
-static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
+static int __init iort_add_platform_device(struct acpi_iort_node *node,
+					   const struct iort_dev_config *ops)
 {
 	struct fwnode_handle *fwnode;
 	struct platform_device *pdev;
 	struct resource *r;
 	enum dev_dma_attr attr;
 	int ret, count;
-	const struct iort_iommu_config *ops = iort_get_iommu_cfg(node);
-
-	if (!ops)
-		return -ENODEV;
 
 	pdev = platform_device_alloc(ops->name, PLATFORM_DEVID_AUTO);
 	if (!pdev)
 		return -ENOMEM;
 
-	if (ops->iommu_set_proximity)
-		ops->iommu_set_proximity(&pdev->dev, node);
+	if (ops->dev_set_proximity)
+		ops->dev_set_proximity(&pdev->dev, node);
 
-	count = ops->iommu_count_resources(node);
+	count = ops->dev_count_resources(node);
 
 	r = kcalloc(count, sizeof(*r), GFP_KERNEL);
 	if (!r) {
@@ -1121,7 +1118,7 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
 		goto dev_put;
 	}
 
-	ops->iommu_init_resources(r, node);
+	ops->dev_init_resources(r, node);
 
 	ret = platform_device_add_resources(pdev, r, count);
 	/*
@@ -1156,8 +1153,8 @@ static int __init iort_add_smmu_platform_device(struct acpi_iort_node *node)
 
 	pdev->dev.fwnode = fwnode;
 
-	attr = ops->iommu_is_coherent(node) ?
-			     DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
+	attr = ops->dev_is_coherent && ops->dev_is_coherent(node) ?
+			DEV_DMA_COHERENT : DEV_DMA_NON_COHERENT;
 
 	/* Configure DMA for the page table walker */
 	acpi_dma_configure(&pdev->dev, attr);
@@ -1214,6 +1211,7 @@ static void __init iort_init_platform_devices(void)
 	struct fwnode_handle *fwnode;
 	int i, ret;
 	bool acs_enabled = false;
+	const struct iort_dev_config *ops;
 
 	/*
 	 * iort_table and iort both point to the start of IORT table, but
@@ -1236,16 +1234,15 @@ static void __init iort_init_platform_devices(void)
 		if (!acs_enabled)
 			acs_enabled = iort_enable_acs(iort_node);
 
-		if ((iort_node->type == ACPI_IORT_NODE_SMMU) ||
-			(iort_node->type == ACPI_IORT_NODE_SMMU_V3)) {
-
+		ops = iort_get_dev_cfg(iort_node);
+		if (ops) {
 			fwnode = acpi_alloc_fwnode_static();
 			if (!fwnode)
 				return;
 
 			iort_set_fwnode(iort_node, fwnode);
 
-			ret = iort_add_smmu_platform_device(iort_node);
+			ret = iort_add_platform_device(iort_node, ops);
 			if (ret) {
 				iort_delete_fwnode(iort_node);
 				acpi_free_fwnode_static(fwnode);
-- 
2.4.12

[PATCH 3/3] ACPI/IORT: Make platform devices initialization code SMMU agnostic

[Hanjun Guo]

On 2017/10/3 21:53, Lorenzo Pieralisi wrote:
> The way current IORT code initializes platform devices for SMMU nodes
> is somewhat tied (mostly for naming convention) to the SMMU nodes
> themselves but it need not be in that it is completely generic and
> can easily be made so by structures renaming and code reshuffling.
>
> Rework IORT platform devices initialization code to make the functions
> and data structures SMMU agnostic.
>
> No functional changes intended.

This will be good preparation for PMCG driver support, with my comments
in patch 2/3 fixed,

Acked-by: Hanjun Guo <hanjun.guo@linaro.org>

Thanks
Hanjun