Subject: [PATCH v2 03/22] ARM: tegra: Create a DT header defining swgroups ID

From: [email-masked] (Stephen Warren)
Date: Mon, 29 Jul 2013 11:45:15 -0600

On 07/29/2013 04:53 AM, Hiroshi Doyu wrote:
> Stephen Warren <[email-masked]> wrote @ Wed, 17 Jul 2013 01:07:12 +0200:
> 
>> On 07/05/2013 04:44 AM, Hiroshi Doyu wrote:
>>> Create a header file to define the swgroup IDs used by the IOMMU(SMMU)
>>> binding. "swgroup" is a group of H/W clients in Tegra SoC from S/W POV.
>>>
>>> This will allow the same header to be used by both device tree files,
>>> and drivers implementing this binding, which guarantees that the two
>>> stay in sync. This also makes device trees more readable by using names
>>> instead of magic numbers.
>>
>> Why does the driver need these constants; shouldn't it simply support
>> 0..n SW group IDs that all work the same way? Or, is there some reason
>> for it to know the identities?
>>
>>> diff --git a/include/dt-bindings/memory/tegra-swgroup.h b/include/dt-bindings/memory/tegra-swgroup.h
>>
>> s/memory/iommu/?
> 
> This info would be used in PMC code as well, which H/W belongs to
> which swgroup. So I use the term "memory(-client)" instead of "iommu".

Why does the PMC care? An explanation of exactly what SW needs to know
about these groups and why would be a good idea.

>> s/tegra-swgroup.h/tegra-smmu.h/?
> 
> Is "tegra-memory-clients.h" preferable, then?

Perhaps, although aren't these IDs specific to the connection between
modules and the SMMU? Or, are these IDs some more global concept; the HW
modules in question pass the same IDs to both the SMMU and to other HW
modules for some reason?

>>> +#define TEGRA_SWGROUP_BIT(x)	(1ULL << TEGRA_SWGROUP_##x)
>>> +
>>> +#define TEGRA30_SWGROUP_ALL	<0x00000000 0x000779ff>
>>> +#define TEGRA114_SWGROUP_ALL	<0x00000000 0x01b659fe>
>>
>> Are those 3 values needed by DT files, or just the driver?
> 
> Only from DT.

If the values are only needed to write the DT content, I'd tend to
prefer not defining them in a header in <dt-bindings/>; I consider that
directory a definition of the constants that define the ABI itself, not
as a random place to put useful things for DT. You can include headers
in the *.dts directory itself.