Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
Hi Joerg, On 22/03/16 17:58, Joerg Roedel wrote: From: Joerg Roedel 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. In a stream-matching implementation, a device may quite legitimately own anything up to _all_ of the stream IDs (32768, or 65536 if we ever implement support for the SMMUv2 EXID extension), so this is only a genuine limit for stream indexing (and if anyone ever actually made one of those, I don't think they're running mainline on it). Alternatively, how straightforward is it to change the DT on your machine? I'll be getting a v2 of [1] out in a couple of weeks (after imminent holidays), which already gets rid of MAX_MASTER_STREAMIDS altogether, and might also have grown proper SMR support by then. Robin. [1]:http://thread.gmane.org/gmane.linux.kernel.iommu/12454 Signed-off-by: Joerg Roedel --- drivers/iommu/arm-smmu.c | 28 ++-- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 59ee4b8..413bd64 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,9 +1784,14 @@ 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)) { + + 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", @@ -1789,6 +1801,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev) i++; } + + if (i == 0) + goto out_put_masters; + dev_notice(dev, "registered %d master devices\n", i); parse_driver_options(smmu); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/6] of: Implement iterator for phandles
On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel wrote: > Hi, > > here is an implementation of the iterator over phandles > concept which Rob Herring suggested to me some time ago. My > approach is a little bit different from what the diff showed > back then, but it gets rid of the allocation and 'struct > 'struct of_phandle_args' misuse. > > I also converted the arm-smmu driver to make use of the > iterator. The resulting kernel boots on my AMD Seattle > system and fixes the warning triggered there. The patches > now also pass all dt-unittests in my kvm environment. > > Please review. Patches are based on v4.5. Other than my one comment, this looks good to me. For the series: Acked-by: Rob Herring ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
On Tue, Mar 22, 2016 at 12:58 PM, Joerg Roedel wrote: > From: Joerg Roedel > > 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 > --- > drivers/iommu/arm-smmu.c | 28 ++-- > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index 59ee4b8..413bd64 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; Isn't this a bit big to put on the stack being ~512 bytes? > int num_irqs, i, err; > > smmu = devm_kzalloc(dev, sizeof(*smmu), GFP_KERNEL); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 6/6] iommu/arm-smmu: Make use of phandle iterators in device-tree parsing
From: Joerg Roedel 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 --- drivers/iommu/arm-smmu.c | 28 ++-- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 59ee4b8..413bd64 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,9 +1784,14 @@ 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)) { + + 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", @@ -1789,6 +1801,10 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev) i++; } + + if (i == 0) + goto out_put_masters; + dev_notice(dev, "registered %d master devices\n", i); parse_driver_options(smmu); -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/6] of: Remove counting special case from __of_parse_phandle_with_args()
From: Joerg Roedel The index = -1 case in __of_parse_phandle_with_args() is used to just return the number of phandles. That special case needs extra handling, so move it to the place where it is needed: of_count_phandle_with_args(). This allows to further simplify __of_parse_phandle_with_args() later on. Signed-off-by: Joerg Roedel --- drivers/of/base.c | 19 ++- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 4036512..15593cd 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1583,10 +1583,7 @@ static int __of_parse_phandle_with_args(const struct device_node *np, * Unlock node before returning result; will be one of: * -ENOENT : index is for empty phandle * -EINVAL : parsing error on data -* [1..n] : Number of phandle (count mode; when index = -1) */ - if (rc == -ENOENT && index < 0) - rc = cur_index; err: if (it.node) @@ -1722,8 +1719,20 @@ EXPORT_SYMBOL(of_parse_phandle_with_fixed_args); int of_count_phandle_with_args(const struct device_node *np, const char *list_name, const char *cells_name) { - return __of_parse_phandle_with_args(np, list_name, cells_name, 0, -1, - NULL); + struct of_phandle_iterator it; + int rc, cur_index = 0; + + rc = of_phandle_iterator_init(&it, np, list_name, cells_name, 0); + if (rc) + return rc; + + while ((rc = of_phandle_iterator_next(&it)) == 0) + cur_index += 1; + + if (rc != -ENOENT) + return rc; + + return cur_index; } EXPORT_SYMBOL(of_count_phandle_with_args); -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/6] of: Introduce of_phandle_iterator_args()
From: Joerg Roedel This helper function can be used to copy the arguments of a phandle to an array. Signed-off-by: Joerg Roedel --- drivers/of/base.c | 29 +++-- include/linux/of.h | 10 ++ 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 471d3d9..008988b 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1533,6 +1533,23 @@ err: return -EINVAL; } +int of_phandle_iterator_args(struct of_phandle_iterator *it, +uint32_t *args, +int size) +{ + int i, count; + + count = it->cur_count; + + if (WARN_ON(size < count)) + count = size; + + for (i = 0; i < count; i++) + args[i] = be32_to_cpup(it->cur++); + + return count; +} + static int __of_parse_phandle_with_args(const struct device_node *np, const char *list_name, const char *cells_name, @@ -1556,13 +1573,13 @@ static int __of_parse_phandle_with_args(const struct device_node *np, goto err; if (out_args) { - int i; - if (WARN_ON(it.cur_count > MAX_PHANDLE_ARGS)) - it.cur_count = MAX_PHANDLE_ARGS; + int c; + + c = of_phandle_iterator_args(&it, +out_args->args, +MAX_PHANDLE_ARGS); out_args->np = it.node; - out_args->args_count = it.cur_count; - for (i = 0; i < it.cur_count; i++) - out_args->args[i] = be32_to_cpup(it.cur++); + out_args->args_count = c; } else { of_node_put(it.node); } diff --git a/include/linux/of.h b/include/linux/of.h index 05e38ed..c91f8b1 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -359,6 +359,9 @@ extern int of_phandle_iterator_init(struct of_phandle_iterator *it, int cell_count); extern int of_phandle_iterator_next(struct of_phandle_iterator *it); +extern int of_phandle_iterator_args(struct of_phandle_iterator *it, + uint32_t *args, + int size); extern void of_alias_scan(void * (*dt_alloc)(u64 size, u64 align)); extern int of_alias_get_id(struct device_node *np, const char *stem); @@ -648,6 +651,13 @@ static inline int of_phandle_iterator_next(struct of_phandle_iterator *it) return -ENOSYS; } +static inline int of_phandle_iterator_args(struct of_phandle_iterator *it, + uint32_t *args, + int size) +{ + return 0; +} + static inline int of_alias_get_id(struct device_node *np, const char *stem) { return -ENOSYS; -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/6] of: Introduce of_for_each_phandle() helper macro
From: Joerg Roedel With this macro any user can easily iterate over a list of phandles. The patch also converts __of_parse_phandle_with_args() to make use of the macro. The of_count_phandle_with_args() function is not converted, because the macro hides the return value of of_phandle_iterator_init(), which is needed in there. Signed-off-by: Joerg Roedel --- drivers/of/base.c | 7 +-- include/linux/of.h | 6 ++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 15593cd..471d3d9 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1542,13 +1542,8 @@ static int __of_parse_phandle_with_args(const struct device_node *np, struct of_phandle_iterator it; int rc, cur_index = 0; - rc = of_phandle_iterator_init(&it, np, list_name, - cells_name, cell_count); - if (rc) - return rc; - /* Loop over the phandles until all the requested entry is found */ - while ((rc = of_phandle_iterator_next(&it)) == 0) { + of_for_each_phandle(&it, rc, np, list_name, cells_name, cell_count) { /* * All of the error cases bail out of the loop, so at * this point, the parsing is successful. If the requested diff --git a/include/linux/of.h b/include/linux/of.h index d94388b0..05e38ed 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -908,6 +908,12 @@ static inline int of_property_read_s32(const struct device_node *np, return of_property_read_u32(np, propname, (u32*) out_value); } +#define of_for_each_phandle(it, err, np, ln, cn, cc) \ + for (of_phandle_iterator_init((it), (np), (ln), (cn), (cc)),\ +err = of_phandle_iterator_next(it);\ +err == 0; \ +err = of_phandle_iterator_next(it)) + #define of_property_for_each_u32(np, propname, prop, p, u) \ for (prop = of_find_property(np, propname, NULL), \ p = of_prop_next_u32(prop, NULL, &u); \ -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/6] of: Move phandle walking to of_phandle_iterator_next()
From: Joerg Roedel Move the code to walk over the phandles out of the loop in __of_parse_phandle_with_args() to a separate function that just works with the iterator handle: of_phandle_iterator_next(). Signed-off-by: Joerg Roedel --- drivers/of/base.c | 130 ++--- include/linux/of.h | 7 +++ 2 files changed, 81 insertions(+), 56 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index bfdb09b..4036512 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1464,6 +1464,75 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it, return 0; } +int of_phandle_iterator_next(struct of_phandle_iterator *it) +{ + uint32_t count = 0; + + if (it->node) { + of_node_put(it->node); + it->node = NULL; + } + + if (!it->cur || it->phandle_end >= it->list_end) + return -ENOENT; + + it->cur = it->phandle_end; + + /* If phandle is 0, then it is an empty entry with no arguments. */ + it->phandle = be32_to_cpup(it->cur++); + + if (it->phandle) { + + /* +* Find the provider node and parse the #*-cells property to +* determine the argument length. +*/ + it->node = of_find_node_by_phandle(it->phandle); + + if (it->cells_name) { + if (!it->node) { + pr_err("%s: could not find phandle\n", + it->parent->full_name); + goto err; + } + + if (of_property_read_u32(it->node, it->cells_name, +&count)) { + pr_err("%s: could not get %s for %s\n", + it->parent->full_name, + it->cells_name, + it->node->full_name); + goto err; + } + } else { + count = it->cell_count; + } + + /* +* Make sure that the arguments actually fit in the remaining +* property data length +*/ + if (it->cur + count > it->list_end) { + pr_err("%s: arguments longer than property\n", + it->parent->full_name); + goto err; + } + } + + it->phandle_end = it->cur + count; + it->cur_count = count; + + return 0; + +err: + if (it->node) { + of_node_put(it->node); + it->node = NULL; + } + + return -EINVAL; +} + static int __of_parse_phandle_with_args(const struct device_node *np, const char *list_name, const char *cells_name, @@ -1479,59 +1548,9 @@ static int __of_parse_phandle_with_args(const struct device_node *np, return rc; /* Loop over the phandles until all the requested entry is found */ - while (it.cur < it.list_end) { - rc = -EINVAL; - it.cur_count = 0; - - /* -* If phandle is 0, then it is an empty entry with no -* arguments. Skip forward to the next entry. -*/ - it.phandle = be32_to_cpup(it.cur++); - if (it.phandle) { - /* -* Find the provider node and parse the #*-cells -* property to determine the argument length. -* -* This is not needed if the cell count is hard-coded -* (i.e. cells_name not set, but cell_count is set), -* except when we're going to return the found node -* below. -*/ - if (it.cells_name || cur_index == index) { - it.node = of_find_node_by_phandle(it.phandle); - if (!it.node) { - pr_err("%s: could not find phandle\n", - it.parent->full_name); - goto err; - } - } - - if (it.cells_name) { - if (of_property_read_u32(it.node, it.cells_name, -&it.cur_count)) { - pr_err("%s: could not get %s for %s\n", - it.parent->full_name, it.cells_name, - it.node->full_name); -
[PATCH 1/6] of: Introduce struct of_phandle_iterator
From: Joerg Roedel This struct carrys all necessary information to iterate over a list of phandles and extract the arguments. Add an init-function for the iterator and make use of it in __of_parse_phandle_with_args(). Signed-off-by: Joerg Roedel --- drivers/of/base.c | 99 +- include/linux/of.h | 33 ++ 2 files changed, 93 insertions(+), 39 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 017dd94..bfdb09b 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -1439,35 +1439,56 @@ void of_print_phandle_args(const char *msg, const struct of_phandle_args *args) printk("\n"); } +int of_phandle_iterator_init(struct of_phandle_iterator *it, + const struct device_node *np, + const char *list_name, + const char *cells_name, + int cell_count) +{ + const __be32 *list; + int size; + + memset(it, 0, sizeof(*it)); + + list = of_get_property(np, list_name, &size); + if (!list) + return -ENOENT; + + it->cells_name = cells_name; + it->cell_count = cell_count; + it->parent = np; + it->list_end = list + size / sizeof(*list); + it->phandle_end = list; + it->cur = list; + + return 0; +} + static int __of_parse_phandle_with_args(const struct device_node *np, const char *list_name, const char *cells_name, int cell_count, int index, struct of_phandle_args *out_args) { - const __be32 *list, *list_end; - int rc = 0, size, cur_index = 0; - uint32_t count = 0; - struct device_node *node = NULL; - phandle phandle; + struct of_phandle_iterator it; + int rc, cur_index = 0; - /* Retrieve the phandle list property */ - list = of_get_property(np, list_name, &size); - if (!list) - return -ENOENT; - list_end = list + size / sizeof(*list); + rc = of_phandle_iterator_init(&it, np, list_name, + cells_name, cell_count); + if (rc) + return rc; /* Loop over the phandles until all the requested entry is found */ - while (list < list_end) { + while (it.cur < it.list_end) { rc = -EINVAL; - count = 0; + it.cur_count = 0; /* * If phandle is 0, then it is an empty entry with no * arguments. Skip forward to the next entry. */ - phandle = be32_to_cpup(list++); - if (phandle) { + it.phandle = be32_to_cpup(it.cur++); + if (it.phandle) { /* * Find the provider node and parse the #*-cells * property to determine the argument length. @@ -1477,34 +1498,34 @@ static int __of_parse_phandle_with_args(const struct device_node *np, * except when we're going to return the found node * below. */ - if (cells_name || cur_index == index) { - node = of_find_node_by_phandle(phandle); - if (!node) { + if (it.cells_name || cur_index == index) { + it.node = of_find_node_by_phandle(it.phandle); + if (!it.node) { pr_err("%s: could not find phandle\n", - np->full_name); + it.parent->full_name); goto err; } } - if (cells_name) { - if (of_property_read_u32(node, cells_name, -&count)) { + if (it.cells_name) { + if (of_property_read_u32(it.node, it.cells_name, +&it.cur_count)) { pr_err("%s: could not get %s for %s\n", - np->full_name, cells_name, - node->full_name); + it.parent->full_name, it.cells_name, + it.node->full_name); goto err; } } else { - count = cell_count; + it.cur_count = it.cell_count; }
[PATCH 0/6] of: Implement iterator for phandles
Hi, here is an implementation of the iterator over phandles concept which Rob Herring suggested to me some time ago. My approach is a little bit different from what the diff showed back then, but it gets rid of the allocation and 'struct 'struct of_phandle_args' misuse. I also converted the arm-smmu driver to make use of the iterator. The resulting kernel boots on my AMD Seattle system and fixes the warning triggered there. The patches now also pass all dt-unittests in my kvm environment. Please review. Patches are based on v4.5. Thanks, Joerg Changes since RFC post: * Reordered members of 'struct of_phandle_iterator' and did some renaming * Removed index counting from the iterator * Split up iterator implementation into multiple patches * Fixed the code to survive all dt-unittests, tested with each patch in this series * Re-added and updated some comments which got lost during the conversion. * Added of_for_each_phandle macro for easier handling * Moved the counting special-case from __of_parse_phandle_with_args directly to of_count_phandle_with_args for code simplification * Removed some iterator helper functions * Formatting and style changes Joerg Roedel (6): of: Introduce struct of_phandle_iterator of: Move phandle walking to of_phandle_iterator_next() of: Remove counting special case from __of_parse_phandle_with_args() of: Introduce of_for_each_phandle() helper macro of: Introduce of_phandle_iterator_args() iommu/arm-smmu: Make use of phandle iterators in device-tree parsing drivers/iommu/arm-smmu.c | 28 +-- drivers/of/base.c| 206 ++- include/linux/of.h | 56 + 3 files changed, 211 insertions(+), 79 deletions(-) -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] of: Implement iterator for phandles
Hi Rob, On Fri, Mar 18, 2016 at 10:54:57AM -0500, Rob Herring wrote: > This mostly looks fine to me, but it is kind of a lot of functions > just for this one thing. For example, I think the caller can track the > index themselves if they care about it. I'd also like to see a more > standard style for_each type iterator define rather than open coded > while loops. Thanks for your feedback. I think I worked everything into the patches and am about to send a new version. > > +struct of_phandle_iterator { > > + const struct device_node *np; > > + const __be32 *list; > > + const __be32 *list_end; > > + const __be32 *phandle_end; > > + phandle phandle; > > + struct device_node *node; > > np and node? If you need both, name them based on what they point to. Yes, 'np' is the parent node, while 'node' is the current node the iterator points to. The parent node is needed for the error messages later. > > +static inline struct device_node *of_phandle_iterator_node(struct > > of_phandle_iterator *it) > > +{ > > + if (!it->node) > > + it->node = of_find_node_by_phandle(it->phandle); > > + > > + if (it->node) > > + of_node_get(it->node); > > The above function may have already done the get. Not sure offhand. Yes, it does. But when the iterator moves on the node will be put again. So when it is returned here we get an extra reference. But it doesn't matter anymore as I changed the code to unconditionally get the node in of_phandle_iterator_next now. Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] arm64/dma-mapping: Add DMA_ATTR_ALLOC_SINGLE_PAGES support
Will, On Mon, Mar 21, 2016 at 11:01 AM, Will Deacon wrote: > On Thu, Mar 03, 2016 at 02:54:26AM +0800, Yong Wu wrote: >> Sometimes it is not worth for the iommu allocating big chunks. >> Here we enable DMA_ATTR_ALLOC_SINGLE_PAGES which could help avoid to >> allocate big chunks while iommu allocating buffer. >> >> More information about this attribute, please check Doug's commit[1]. >> >> [1]: https://lkml.org/lkml/2016/1/11/720 >> >> Cc: Robin Murphy >> Suggested-by: Douglas Anderson >> Signed-off-by: Yong Wu >> --- >> >> Our video drivers may soon use this. >> >> arch/arm64/mm/dma-mapping.c | 4 ++-- >> drivers/iommu/dma-iommu.c | 14 ++ >> include/linux/dma-iommu.h | 4 ++-- >> 3 files changed, 14 insertions(+), 8 deletions(-) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 331c4ca..3225e3ca 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -562,8 +562,8 @@ static void *__iommu_alloc_attrs(struct device *dev, >> size_t size, >> struct page **pages; >> pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, coherent); >> >> - pages = iommu_dma_alloc(dev, iosize, gfp, ioprot, handle, >> - flush_page); >> + pages = iommu_dma_alloc(dev, iosize, gfp, ioprot, attrs, >> + handle, flush_page); >> if (!pages) >> return NULL; >> >> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c >> index 72d6182..3569cb6 100644 >> --- a/drivers/iommu/dma-iommu.c >> +++ b/drivers/iommu/dma-iommu.c >> @@ -190,7 +190,8 @@ static void __iommu_dma_free_pages(struct page **pages, >> int count) >> kvfree(pages); >> } >> >> -static struct page **__iommu_dma_alloc_pages(unsigned int count, gfp_t gfp) >> +static struct page **__iommu_dma_alloc_pages(unsigned int count, gfp_t gfp, >> + struct dma_attrs *attrs) >> { >> struct page **pages; >> unsigned int i = 0, array_size = count * sizeof(*pages); >> @@ -203,6 +204,10 @@ static struct page **__iommu_dma_alloc_pages(unsigned >> int count, gfp_t gfp) >> if (!pages) >> return NULL; >> >> + /* Go straight to 4K chunks if caller says it's OK. */ >> + if (dma_get_attr(DMA_ATTR_ALLOC_SINGLE_PAGES, attrs)) >> + order = 0; > > I have a slight snag with this, in that you don't consult the IOMMU > pgsize_bitmap at any point, and assume that it can map pages at the > same granularity as the CPU. The documentation for > DMA_ATTR_ALLOC_SINGLE_PAGES seems to be weaker than that. Interesting. Is that something that exists in the real world? ...or something you think is coming soon? I'd argue that such a case existed in the real world then probably we're already broken. Unless I'm misreading, existing code will already fall all the way back to order 0 allocations. ...so while existing code might could work if it was called on a totally unfragmented system, it would already break once some fragmentation was introduced. I'm not saying that we shouldn't fix the code to handle this, I'm just saying that Yong Wu's patch doesn't appear to break any code that wasn't already broken. That might be reason to land his code first, then debate the finer points of whether IOMMUs with less granularity are likely to exist and whether we need to add complexity to the code to handle them (or just detect this case and return an error). >From looking at a WIP patch provided to me by Yong Wu, it looks as if he thinks several more functions need to change to handle this need for IOMMUs that can't handle small pages. That seems to be further evidence that the work should be done in a separate patch. Doug ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/rockchip: Only log stall errors when attaching
Move the logging of timeouts when stalling the MMU to rk_iommu_attach_device, as it's expected that sometimes the MMU won't get stalled when detaching a device, and it's not a real problem that would need to be communicated to the user. Signed-off-by: Tomeu Vizoso --- drivers/iommu/rockchip-iommu.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c index 0253ab35c33b..65325b6742e6 100644 --- a/drivers/iommu/rockchip-iommu.c +++ b/drivers/iommu/rockchip-iommu.c @@ -346,13 +346,7 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu) rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL); - ret = rk_wait_for(rk_iommu_is_stall_active(iommu), 1); - if (ret) - for (i = 0; i < iommu->num_mmu; i++) - dev_err(iommu->dev, "Enable stall request timed out, status: %#08x\n", - rk_iommu_read(iommu->bases[i], RK_MMU_STATUS)); - - return ret; + return rk_wait_for(rk_iommu_is_stall_active(iommu), 1); } static int rk_iommu_disable_stall(struct rk_iommu *iommu) @@ -798,8 +792,12 @@ static int rk_iommu_attach_device(struct iommu_domain *domain, return 0; ret = rk_iommu_enable_stall(iommu); - if (ret) + if (ret) { + for (i = 0; i < iommu->num_mmu; i++) + dev_err(iommu->dev, "Enable stall request timed out, status: %#08x\n", + rk_iommu_read(iommu->bases[i], RK_MMU_STATUS)); return ret; + } ret = rk_iommu_force_reset(iommu); if (ret) -- 2.5.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 0/7] PAMU driver update
Hi, Anyone has any comments on this patch-set? Please share your thoughts. Thanks and best regards, Codrin > -Original Message- > From: Codrin Ciubotariu [mailto:codrin.ciubota...@nxp.com] > Sent: Monday, 07 March, 2016 5:34 PM > To: iommu@lists.linux-foundation.org > Cc: scottw...@freescale.com; varun.se...@freescale.com; linuxppc- > d...@lists.ozlabs.org; Codrin Constantin Ciubotariu > Subject: [PATCH 0/7] PAMU driver update > > This patchset addresses a few issues found on PAMU IOMMU and small changes to > enable power management and to support the > L3 cache controller on some newer boards. > > The series starts with a clean-up patch, followed by two errata fixes: > A-007907 > and A-005982. It continues with two fixes for PCIe support. The last two > patches > add support for power management and compatible strings for new L3 controller > device-tree nodes. > > Codrin Ciubotariu (2): > iommu/fsl: Fix most checkpatch warnings and typos > iommu/fsl: Work around erratum A-007907 > > Varun Sethi (5): > iommu/fsl: Enable OMT cache, before invalidating PAACT and SPAACT > cache > iommu/fsl: Factor out PCI specific code > iommu/fsl: Enable default DMA window for PCIe devices once detached > from domain > iommu/fsl: PAMU power management support > iommu/fsl: Added cache controller compatible strings for SOCs > > drivers/iommu/fsl_pamu.c| 322 > > drivers/iommu/fsl_pamu.h| 30 ++-- > drivers/iommu/fsl_pamu_domain.c | 160 +--- > drivers/iommu/fsl_pamu_domain.h | 2 +- > 4 files changed, 381 insertions(+), 133 deletions(-) > > -- > 1.9.3 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[git pull] IOMMU Updates for Linux v4.6
Hi Linus, The following changes since commit b562e44f507e863c6792946e4e1b1449fbbac85d: Linux 4.5 (2016-03-13 21:28:54 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git tags/iommu-updates-v4.6 for you to fetch changes up to 70cf769c5ba283483a42c46f3734202b55dd3041: Merge branches 'arm/rockchip', 'arm/exynos', 'arm/smmu', 'arm/mediatek', 'arm/io-pgtable', 'arm/renesas' and 'core' into next (2016-03-21 14:58:47 +0100) IOMMU Updates for Linux v4.6 This time with: * Updates for the Exynos IOMMU driver to make use of default domains and to add support for the SYSMMU v5 * New Mediatek IOMMU driver * Support for the ARMv7 short descriptor format in the io-pgtable code * Default domain support for the ARM SMMU * Couple of other small fixes all over the place Andrzej Hajda (1): iommu/mediatek: Fix handling of of_count_phandle_with_args result Anup Patel (1): of: iommu: Increment DT node refcount in of_iommu_set_ops() Arnd Bergmann (3): iommu/exynos: Pointers are nto physical addresses iommu/mediatek: Select ARM_DMA_USE_IOMMU iommu/mediatek: Mark PM functions as __maybe_unused Dan Carpenter (1): iommu/mediatek: Check for NULL instead of IS_ERR() Joerg Roedel (3): Merge branch 'for-joerg/io-pgtable' of git://git.kernel.org/.../will/linux into arm/io-pgtable Merge branch 'for-joerg/arm-smmu/updates' of git://git.kernel.org/.../will/linux into arm/smmu Merge branches 'arm/rockchip', 'arm/exynos', 'arm/smmu', 'arm/mediatek', 'arm/io-pgtable', 'arm/renesas' and 'core' into next Magnus Damm (1): iommu/ipmmu-vmsa: Add r8a7795 DT binding Marek Szyprowski (14): iommu/exynos: Rework iommu group initialization iommu/exynos: Add support for IOMMU_DOMAIN_DMA domain type iommu/exynos: Remove ARM-specific cache flush interface iommu/exynos: Simplify master clock operations iommu/exynos: Refactor code (no direct register access) iommu/exynos: Refactor fault handling code iommu/exynos: Refactor init config code iommu/exynos: Unify code for fldp cache invalidation iommu/exynos: Add support for SYSMMU controller with bogus version reg iommu/exynos: Update device tree documentation iommu/exynos: Add support for v5 SYSMMU iommu/exynos: Add Maintainers entry for Exynos SYSMMU driver iommu/exynos: Support multiple attach_device calls iommu/exynos: Use proper readl/writel register interface Robin Murphy (9): iommu/io-pgtable: Add ARMv7 short descriptor support iommu/io-pgtable: Add helper functions for TLB ops iommu/io-pgtable: Avoid redundant TLB syncs iommu/io-pgtable: Rationalise quirk handling iommu/arm-smmu: Treat all device transactions as unprivileged iommu/arm-smmu: Support DMA-API domains iommu/arm-smmu: Allow disabling unmatched stream bypass iommu/dma: Fix NEED_SG_DMA_LENGTH dependency iommu/io-pgtable-armv7s: Fix kmem_cache_alloc() flags Simon Horman (1): iommu/ipmmu-vmsa: Use ARCH_RENESAS Will Deacon (3): MAINTAINERS: update ARM SMMU entry iommu/arm-smmu: Don't fail device attach if already attached to a domain iommu/arm-smmu: Treat IOMMU_DOMAIN_DMA as bypass for now Yong Wu (5): dt-bindings: iommu: Add binding for mediatek IOMMU dt-bindings: mediatek: Add smi dts binding memory: mediatek: Add SMI driver iommu/mediatek: Add mt8173 IOMMU driver dts: mt8173: Add iommu/smi nodes for mt8173 Yoshihiro Shimoda (1): iommu: Fix second argument of trace_map() to report correct paddr ZhengShunQian (1): iommu/rockchip: Reconstruct to support multi slaves .../devicetree/bindings/iommu/mediatek,iommu.txt | 68 ++ .../bindings/iommu/renesas,ipmmu-vmsa.txt | 15 +- .../devicetree/bindings/iommu/samsung,sysmmu.txt | 22 +- .../memory-controllers/mediatek,smi-common.txt | 24 + .../memory-controllers/mediatek,smi-larb.txt | 25 + MAINTAINERS| 8 + arch/arm64/boot/dts/mediatek/mt8173.dtsi | 81 ++ drivers/iommu/Kconfig | 42 +- drivers/iommu/Makefile | 2 + drivers/iommu/arm-smmu-v3.c| 50 +- drivers/iommu/arm-smmu.c | 79 +- drivers/iommu/exynos-iommu.c | 608 +-- drivers/iommu/io-pgtable-arm-v7s.c | 846 + drivers/iommu/io-pgtable-arm.c | 34 +- drivers/iommu/io-pgtable.c | 5 +- drivers/iommu/io-pgtable.h | 53 +- drivers/iommu/iommu.c | 3 +-