On 29/03/2019 10:51, Marc Gonzalez wrote:
On 28/03/2019 18:05, Marc Gonzalez wrote:ANOC1 SMMU filters PCIe accesses.I'm not sure this description is entirely accurate... ANOC likely stands for "Application Network-On-Chip"arch/arm64/boot/dts/qcom/msm8998.dtsi | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi index f9a922fdae75..5a1c0961b281 100644 --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi @@ -606,6 +606,21 @@ #thermal-sensor-cells = <1>; };+ anoc1_smmu: arm,smmu@1680000 {+ compatible = "qcom,msm8996-smmu-v2", "qcom,smmu-v2";Maybe I should instead use "qcom,msm8998-smmu-v2", "qcom,smmu-v2"; and define "qcom,msm8998-smmu-v2" in Documentation/devicetree/bindings/iommu/arm,smmu.txt ?
Yes please.
(Would the Documentation change need to be in a separate patch?)
That's generally preferred, yes.
+ reg = <0x01680000 0x10000>; + #iommu-cells = <1>;I'm not sure about this property. IIRC, Robin said <0> is not valid, but I don't have any iommus prop, only iommu-map.
You have to join the dots between the various bindings a little, but the iommu-base part of each iommu-map entry is an IOMMU specifier, and IOMMU specifiers are #iommu-cells long.
To cut a long story short, 1 is definitely appropriate, because arm-smmu's definition of a 2-cell specifier wouldn't make much sense in the iommu-map context (and the current code for parsing iommu-map actually just assumes 1 anyway).
+ + #global-interrupts = <0>; + interrupts = + <GIC_SPI 364 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 365 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 366 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 367 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 368 IRQ_TYPE_EDGE_RISING>, + <GIC_SPI 369 IRQ_TYPE_EDGE_RISING>; + }; +The rest of the node looks fairly straight-forward.
You should probably have some "bus" and "iface" clocks too, per the requirement of "qcom,smmu-v2". Maybe Vivek might know what's relevant for MSM8998?
DT code was adapted from: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm-arm-smmu-8998.dtsi?h=LE.UM.1.3.r3.25#n18 I left out the so-called implementation-defined init: https://source.codeaurora.org/quic/la/kernel/msm-4.4/tree/arch/arm/boot/dts/qcom/msm-arm-smmu-impl-defs-8998.dtsi?h=LE.UM.1.3.r3.25#n123 Should I try to merge this part in mainline? (I don't have any documentation for it though.)
"pls no :("
I'm not sure what route the path takes from "DT describes the platform"
to get to "DT lists opaque register addresses and magic data to write
into them", but I imagine it might involve getting hit in the head along
the way.
Robin.
