On 10/31/2013 02:12 AM, Hiroshi Doyu wrote:
> Stephen Warren <swarren@xxxxxxxxxxxxx> wrote @ Wed, 30 Oct 2013 22:58:58 +0100:
>
>> On 10/25/2013 03:11 AM, Thierry Reding wrote:
>> ...
>>> So my proposed solution for the IOMMU case is to treat it the same
>>> as any other resources. Perhaps resource isn't the right word, but
>>> at the core the issue is the same. A device requires the services
>>> of an IOMMU so that it can be put into the correct address space.
>>> If the IOMMU is not available yet it cannot do that, so we simply
>>> return -EPROBE_DEFER and cause the probe to be retried later.
>>
>> Personally, I view deferred probe as being used when one device
>> requires either a resource /or/ a service provided by another, not
>> /just/ when there's a resource dependency. Hence, I think it fits
>> perfectly here.
>>
>> So I agree with Thierry: In other words, I think the solution is for
>> all devices that are affected by an IOMMU to have a property such as:
>>
>> iommu = <&iommu_phandle iommu_specifier>;
>
> Agree
>
>> (and the DT node for the IOMMU will contain e.g. an #iommu-cells property)
>
> As explained in another mail[1], "#iommu-cells" could vary, depending
> on a device since it's arbitral number of arguments(IDs) right
> now. This could be a fixed size of bitmap(64), 2 cells since we know
> "64" would be enough in the future as well as below.
So, a device can generate transactions using n different IDs, yet those
IDs all fit into some specific number of bits if represented as a
bitmask rather than a list. The size of that bitmask would be a good
candidate for #iommu-cells.
...
> smmu: iommu {
> #iommu-cells = <2>;
> };
>
> deviceA {
> iommu = <&smmu 0x00000000 0x00000040>;
> };
>
> deviceB {
> iommu = <&smmu 0x00000000 0x00000104>;
> };
So yes, that seems like a reasonable representation.
> But then we cannot use SWGROUP ID "macros" in DT files since DTC
> cannot perse OR("|") operations as below. Or can we?
...
> deviceB {
> iommu = <&smmu SWGROUP_ID_A | SWGROUP_ID_B>;
> };
dtc now supports math expressions. You need to wrap expressions in ()
for them to be recognized, so if you write the above as:
iommu = <&smmu (SWGROUP_ID_A | SWGROUP_ID_B)>;
... it should work fine.
> So I thought that arbitral length of arguments may be easier to read as
> below:
...
> deviceB {
> iommu = <&smmu SWGROUP_ID_A
> SWGROUP_ID_B>;
> };
The problem with that is that it doesn't allow for multiple IOMMUs to be
represented by one property. You need a fixed number (e.g. #iommu-cells)
of cells after the phandle in order to know where one IOMMU specifier
ends, and the next phandle/specifier starts. While multiple IOMMUs is
likely quite rare, I see no reason to force a lack of support for that
scenario by ignoring the standard phandle/fixed-length-specifier
property format.
>> ... and for the driver to explicitly parse that property, and wait
>> until the driver for iommu_phandle is ready. Exactly the same as any
>> other resource/service dependency.
>>
>> That will solve all the problems.
>>
>> The only downside is that every driver needs to contain code to parse
>> that property. However, I think that's just one function call; the
>> actual implementation of that function can be unified somewhere inside
>> core code in drivers/iommu/.
>
> Yes, but only missing part now is that, we could do this with
> "bus_notifier", but the current bus_notifier doesn't have the feature
> to return error(-EPROBE_DEFER). This could be modified so that
> bus_notifier could return (-EPROBE_DEFER) to postpone
> probing. Alternatively this could be done in some core probe code as
> well as Thierry pointed out.
>
> [1] In the reply of "[PATCHv3 14/19] iommu/tegra: smmu: Get "nvidia,memory-clients" from DT"
I think this should be done explicitly in drivers. It's much simpler,
and doesn't encode any knowledge of driver-specific bindings into some
common bus notifier code.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
