> >>> The existing iommu-map binding did not account for the situation where > >>> #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell > >>> of the IOMMU specifier being the SMR mask. The existing binding defines > >>> the mapping as: > >>> Any RID r in the interval [rid-base, rid-base + length) is associated with > >>> the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base). > >>> > >>> ...and that does not work if iommu-base is 2 cells, the second being the > >>> SMR mask. > >>> > >>> While this can be worked around by always having length=1, it seems we > >>> can get this cleaned up by updating the binding definition for iommu-map. > >> > >> To reiterate, I'm very much not keen on the pci-iommu binding having > >> knowledge of the decomposition of iommu-specifiers from other bindings. > > > > With the current definition of iommu-map we already have baked in an > > assumption that an iommu-specifier is a value that can be incremented > > by 1 to get to the next sequential specifier. So the binding already > > has "knowledge" that applies in most, but not all cases. > > > > The generic iommu binding also defines a case where #iommu-cells=4 > > for some IOMMUs. > > > > Since the ARM SMMU is a special case, perhaps the intepretation > > of an iommu-specifier in the context of iommu-map should be moved > > into the SMMU binding. > > > >> As mentioned previously, there's an intended interpretation [1] that we > >> need to fix up the pci-iommu binding with. With that, I don't think it's > >> even necessary to bodge iommu-cells = <1> AFAICT. > > > > You had said in the previous thread: > > > > > I had intended that the offset was added to the final cell of the > > > iommu-specifier (i.e. that the iommu-specifier was treated as a large > > > number). > > > > > You can handle this case by adding additional entries in the map table, > > > with a single entry length. > > > > I understand that, and it works, but I don't see why the definition has > > to be that the offset is added to the "final cell". > > Because the cells of a specifier form a single opaque big-endian value. > Were DT little-endian it would be the "first cell". To be pedantic, both > of those descriptions are technically wrong because they fail to account > for overflow and carry up into the next cell (in whichever direction). If it is really opaque how can we reliably add 1 to it? > > Why can't it be > > an iommu specific definition that makes sense for a given IOMMU? > > Because the implementation would then become a nightmarish rabbit-warren > of special-cases, largely defeating the point of a *generic* binding. At > the very least it'd have to chase every phandle and determine its > compatible just to work out what to do with any given specifier - merely > thinking about the complexity of the error handling for the number of > additional failure modes that introduces is enough to put me off. In order to decode an iommu-map at all we have to chase every phandle, no? Isn't that how we know how many cells an iommu-map entry has? I have not thought through the implementation, but my thought was that the code could use a callback to handle the IOMMU-specific case. If callback==NULL then the default case is what we have today. An IOMMU like the SMMU can implement a simple callback that can return the mapping. Not sure how this is that much harder than any other map, such as interrupt-map. Stuart -- 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
