Re: [PATCH v2 9/9] iommu/arm-smmu: add support for non-pci devices

[Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>]

On 09/07/15 02:56, leizhen wrote:
[...]
>>> @@ -1928,9 +1929,35 @@ static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
[...]
>>> +        for (i = 0; i < args->args_count; i++) {
>>
>> I'm dubious of the value of looping here - having n>1 #iommu-cells per phandle means that every platform device behind one SMMU must have the same number of stream IDs, or you still have to have
>> repeated phandles for every device with some greater multiple of n stream IDs each, but ruling out any device with <n. I'm not sure how realistic that is, and whether there's any real benefit beyond
>> saving a handful of bytes in the DTB.
>
> As you mentioned before, a master with two streamIDs can be written as below(This is also mentioned in Documentation\devicetree\bindings\iommu\iommu.txt):
> #iommu-cells = <1>;
> iommus = <&{/iommu} 23>, <&{/iommu} 24>;
>
> On my hardware platform, a master only have one streamID. But I tested two cases:
> #iommu-cells = <1>;
> 1. iommus = <&smmu0 good-sid>, <&smmu0 bad-sid>;
> 2. iommus = <&smmu0 bad-sid>, <&smmu0 good-sid>;
> All of these two cases worked well.
>
> Well, if each master contains two streamIDs(or more but equal), this for loop is needed.

My point is that args_count == #iommu-cells here. If you mandate 
#iommu-cells == 1 as per your example (which in my opinion *is* the 
right thing to do), then looping over something which is guaranteed to 
be a single item is pointless.

You'd only need the loop if args_count > 1, for example:

	#iommu-cells = <2>;
	1: iommus = <&smmu0 sid1 sid2>;
	2: iommus = <&smmu0 sid1 sid2>, <&smmu0 sid3 sid4>;

but then you'd be also stuck describing any device with an odd number of 
stream IDs on that SMMU:

	3: iommus = <&smmu0 sid1 what-do-I-put-here?>;

which is why I don't think it's worth trying to accommodate anything 
other than #iommu-cells == 1, and for that case you know you will only 
ever have to deal with args[0] in each of_xlate() call.

Robin.