diff mbox

[v2] iommu/arm-smmu: Make use of phandle iterators in device-tree device-tree parsing

Message ID 20160323114707.GA17838@8bytes.org (mailing list archive)
State New, archived
Headers show

Commit Message

Joerg Roedel March 23, 2016, 11:47 a.m. UTC 
On Tue, Mar 22, 2016 at 01:38:06PM -0500, Rob Herring wrote:
> > +       struct of_phandle_iterator it;
> > +       struct arm_smmu_phandle_args masterspec;
> 
> Isn't this a bit big to put on the stack being ~512 bytes?

Yeah, you might be right. I havn't seen any problems booting with this
being allocated on the stack, but to be on the safe side I changed the
patch to allocate the masterspec with kmalloc, patch below.


	Joerg

From 85995d1edea7f61aae3c31e0fbd3258622d0b5ae Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel@suse.de>
Date: Wed, 16 Mar 2016 17:10:10 +0100
Subject: [PATCH] iommu/arm-smmu: Make use of phandle iterators in device-tree
 parsing

Remove the usage of of_parse_phandle_with_args() and replace
it by the phandle-iterator implementation so that we can
parse out all of the potentially present 128 stream-ids.

Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
 drivers/iommu/arm-smmu.c | 41 +++++++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 8 deletions(-)

Comments

kernel test robot March 23, 2016, 3:18 p.m. UTC | #1 
Hi Joerg,

[auto build test ERROR on iommu/next]
[also build test ERROR on v4.5 next-20160323]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Joerg-Roedel/iommu-arm-smmu-Make-use-of-phandle-iterators-in-device-tree-device-tree-parsing/20160323-194824
base:   https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git next
config: arm64-defconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm64 

All errors (new ones prefixed by >>):

   drivers/iommu/arm-smmu.c: In function 'arm_smmu_device_dt_probe':
   drivers/iommu/arm-smmu.c:1746:29: error: storage size of 'it' isn't known
     struct of_phandle_iterator it;
                                ^
>> drivers/iommu/arm-smmu.c:1815:2: error: implicit declaration of function 'of_for_each_phandle' [-Werror=implicit-function-declaration]
     of_for_each_phandle(&it, err, dev->of_node,
     ^
>> drivers/iommu/arm-smmu.c:1816:46: error: expected ';' before '{' token
           "mmu-masters", "#stream-id-cells", 0) {
                                                 ^
   drivers/iommu/arm-smmu.c:1746:29: warning: unused variable 'it' [-Wunused-variable]
     struct of_phandle_iterator it;
                                ^
   drivers/iommu/arm-smmu.c: At top level:
   drivers/iommu/arm-smmu.c:473:12: warning: 'register_smmu_master' defined but not used [-Wunused-function]
    static int register_smmu_master(struct arm_smmu_device *smmu,
               ^
   cc1: some warnings being treated as errors

vim +/of_for_each_phandle +1815 drivers/iommu/arm-smmu.c

  1740	{
  1741		const struct of_device_id *of_id;
  1742		struct resource *res;
  1743		struct arm_smmu_device *smmu;
  1744		struct device *dev = &pdev->dev;
  1745		struct rb_node *node;
> 1746		struct of_phandle_iterator it;
  1747		struct arm_smmu_phandle_args *masterspec;
  1748		int num_irqs, i, err;
  1749	
  1750		smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
  1751		if (!smmu) {
  1752			dev_err(dev, "failed to allocate arm_smmu_device\n");
  1753			return -ENOMEM;
  1754		}
  1755		smmu->dev = dev;
  1756	
  1757		of_id = of_match_node(arm_smmu_of_match, dev->of_node);
  1758		smmu->version = (enum arm_smmu_arch_version)of_id->data;
  1759	
  1760		res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
  1761		smmu->base = devm_ioremap_resource(dev, res);
  1762		if (IS_ERR(smmu->base))
  1763			return PTR_ERR(smmu->base);
  1764		smmu->size = resource_size(res);
  1765	
  1766		if (of_property_read_u32(dev->of_node, "#global-interrupts",
  1767					 &smmu->num_global_irqs)) {
  1768			dev_err(dev, "missing #global-interrupts property\n");
  1769			return -ENODEV;
  1770		}
  1771	
  1772		num_irqs = 0;
  1773		while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
  1774			num_irqs++;
  1775			if (num_irqs > smmu->num_global_irqs)
  1776				smmu->num_context_irqs++;
  1777		}
  1778	
  1779		if (!smmu->num_context_irqs) {
  1780			dev_err(dev, "found %d interrupts but expected at least %d\n",
  1781				num_irqs, smmu->num_global_irqs + 1);
  1782			return -ENODEV;
  1783		}
  1784	
  1785		smmu->irqs = devm_kzalloc(dev, sizeof(*smmu->irqs) * num_irqs,
  1786					  GFP_KERNEL);
  1787		if (!smmu->irqs) {
  1788			dev_err(dev, "failed to allocate %d irqs\n", num_irqs);
  1789			return -ENOMEM;
  1790		}
  1791	
  1792		for (i = 0; i < num_irqs; ++i) {
  1793			int irq = platform_get_irq(pdev, i);
  1794	
  1795			if (irq < 0) {
  1796				dev_err(dev, "failed to get irq index %d\n", i);
  1797				return -ENODEV;
  1798			}
  1799			smmu->irqs[i] = irq;
  1800		}
  1801	
  1802		err = arm_smmu_device_cfg_probe(smmu);
  1803		if (err)
  1804			return err;
  1805	
  1806		i = 0;
  1807		smmu->masters = RB_ROOT;
  1808	
  1809		err = -ENOMEM;
  1810		/* No need to zero the memory for masterspec */
  1811		masterspec = kmalloc(sizeof(*masterspec), GFP_KERNEL);
  1812		if (!masterspec)
  1813			goto out_put_masters;
  1814	
> 1815		of_for_each_phandle(&it, err, dev->of_node,
> 1816				    "mmu-masters", "#stream-id-cells", 0) {
  1817			int count = of_phandle_iterator_args(&it, masterspec->args,
  1818							     MAX_MASTER_STREAMIDS);
  1819			masterspec->np		= of_node_get(it.node);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Joerg Roedel April 4, 2016, 2:25 p.m. UTC | #2 
On Wed, Mar 23, 2016 at 11:18:25PM +0800, kbuild test robot wrote:
>    drivers/iommu/arm-smmu.c: In function 'arm_smmu_device_dt_probe':
>    drivers/iommu/arm-smmu.c:1746:29: error: storage size of 'it' isn't known
>      struct of_phandle_iterator it;
>                                 ^
> >> drivers/iommu/arm-smmu.c:1815:2: error: implicit declaration of function 'of_for_each_phandle' [-Werror=implicit-function-declaration]
>      of_for_each_phandle(&it, err, dev->of_node,
>      ^
> >> drivers/iommu/arm-smmu.c:1816:46: error: expected ';' before '{' token
>            "mmu-masters", "#stream-id-cells", 0) {
>                                                  ^
>    drivers/iommu/arm-smmu.c:1746:29: warning: unused variable 'it' [-Wunused-variable]
>      struct of_phandle_iterator it;
>                                 ^
>    drivers/iommu/arm-smmu.c: At top level:
>    drivers/iommu/arm-smmu.c:473:12: warning: 'register_smmu_master' defined but not used [-Wunused-function]
>     static int register_smmu_master(struct arm_smmu_device *smmu,
>                ^
>    cc1: some warnings being treated as errors

Looks like this patch got compiled without the other patch in this set,
right? Because the config builds perfectly fine here for me.


	Joerg
diff mbox

Patch

diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 59ee4b8..fa9b98c 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,7 +48,7 @@ 
 #include "io-pgtable.h"
 
 /* Maximum number of stream IDs assigned to a single device */
-#define MAX_MASTER_STREAMIDS		MAX_PHANDLE_ARGS
+#define MAX_MASTER_STREAMIDS		128
 
 /* Maximum number of context banks per SMMU */
 #define ARM_SMMU_MAX_CBS		128
@@ -349,6 +349,12 @@  struct arm_smmu_domain {
 	struct iommu_domain		domain;
 };
 
+struct arm_smmu_phandle_args {
+	struct device_node *np;
+	int args_count;
+	uint32_t args[MAX_MASTER_STREAMIDS];
+};
+
 static struct iommu_ops arm_smmu_ops;
 
 static DEFINE_SPINLOCK(arm_smmu_devices_lock);
@@ -458,7 +464,7 @@  static int insert_smmu_master(struct arm_smmu_device *smmu,
 
 static int register_smmu_master(struct arm_smmu_device *smmu,
 				struct device *dev,
-				struct of_phandle_args *masterspec)
+				struct arm_smmu_phandle_args *masterspec)
 {
 	int i;
 	struct arm_smmu_master *master;
@@ -1716,7 +1722,8 @@  static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 	struct arm_smmu_device *smmu;
 	struct device *dev = &pdev->dev;
 	struct rb_node *node;
-	struct of_phandle_args masterspec;
+	struct of_phandle_iterator it;
+	struct arm_smmu_phandle_args *masterspec;
 	int num_irqs, i, err;
 
 	smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL);
@@ -1777,20 +1784,38 @@  static int arm_smmu_device_dt_probe(struct platform_device *pdev)
 
 	i = 0;
 	smmu->masters = RB_ROOT;
-	while (!of_parse_phandle_with_args(dev->of_node, "mmu-masters",
-					   "#stream-id-cells", i,
-					   &masterspec)) {
-		err = register_smmu_master(smmu, dev, &masterspec);
+
+	err = -ENOMEM;
+	/* No need to zero the memory for masterspec */
+	masterspec = kmalloc(sizeof(*masterspec), GFP_KERNEL);
+	if (!masterspec)
+		goto out_put_masters;
+
+	of_for_each_phandle(&it, err, dev->of_node,
+			    "mmu-masters", "#stream-id-cells", 0) {
+		int count = of_phandle_iterator_args(&it, masterspec->args,
+						     MAX_MASTER_STREAMIDS);
+		masterspec->np		= of_node_get(it.node);
+		masterspec->args_count	= count;
+
+		err = register_smmu_master(smmu, dev, masterspec);
 		if (err) {
 			dev_err(dev, "failed to add master %s\n",
-				masterspec.np->name);
+				masterspec->np->name);
+			kfree(masterspec);
 			goto out_put_masters;
 		}
 
 		i++;
 	}
+
+	if (i == 0)
+		goto out_put_masters;
+
 	dev_notice(dev, "registered %d master devices\n", i);
 
+	kfree(masterspec);
+
 	parse_driver_options(smmu);
 
 	if (smmu->version > ARM_SMMU_V1 &&