Re: [PATCHv3 14/19] iommu/tegra: smmu: Get "nvidia,memory-clients" from DT

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 10/31/2013 11:40 AM, Mark Rutland wrote:
> On Thu, Oct 31, 2013 at 08:18:08AM +0000, Hiroshi Doyu wrote:
>> Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote @ Wed, 30 Oct 2013 23:44:04 +0100:
>>
>>>> +	host1x {
>>>> +		compatible = "nvidia,tegra30-host1x", "simple-bus";
>>>> +		nvidia,memory-clients = <&smmu TEGRA_SWGROUP_HC>;
>>>> +		....
>>>> +		gr3d {
>>>> +			compatible = "nvidia,tegra30-gr3d";
>>>> +			nvidia,memory-clients = <&smmu TEGRA_SWGROUP_NV
>>>> +						       TEGRA_SWGROUP_NV2>;
>>>
>>> Why one cell for the host1x property, and two cells for the gr3d
>>> property; shouldn't they be the same length?
>>
>> That can vary. Even a single device node in DT can belong to multiple
>> software groups.
> 
> Same with ARM SMMU.
> 
>>
>>>> diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
>>>
>>>> +static u64 smmu_of_get_memory_client(struct device *dev)
>>>
>>>> +	np = of_parse_phandle(dev->of_node, propname, 0);
>>>> +	if (np != smmu_handle->dev->of_node)
>>>> +		return ~0;
>>>> +
>>>> +	prop = of_get_property(dev->of_node, propname, &bytes);
>>>> +	if (!prop || !bytes)
>>>> +		return ~0;
>>>
>>> of_parse_phandle_with_fixed_args() might be a good fit here, or perhaps
>>> require a property #smmu-cells in the SMMU node, thus allowing the
>>> non-fixed-length of_parse_phandle_with_args() to be used here.
>>
>> I once considered of_parse_phandle_with_args() but I couldn't use that
>> since "#smmu-cells" needed to be fixed in smmu node, not in client
>> node. In this case, the number of argument(number of swgroup IDs)
>> varies per client device.
> 
> For the ARM SMMU binding, each device has a #stream-id-cells property
> describing how many IDs it has, and then the SMMU node has a phandle+args
> linkage to each of the devices attached to it, describing their stream IDs.
> While this does have some limitations (a device can't be plugged into multiple
> SMMUs with a different number of IDs), it seems generally sane.

Hmm. That's interesting. I see that the ARM SMMU has a list of the
clients it affects, whereas this Tegra series puts information into each
client device about the SMMU(s) that affect it. Is it better to flip the
Tegra binding around to match the style of the ARM SMMU?