On 16/07/18 19:56, Thor Thayer wrote:
[...]
@@ -332,6 +342,36 @@
altr,modrst-offset = <0x20>;
};
+ smmu: iommu@fa000000 {
+ compatible = "arm,mmu-500", "arm,smmu-v2";
+ reg = <0xfa000000 0x40000>;
+ #global-interrupts = <9>;
+ #iommu-cells = <1>;
+ clocks = <&clkmgr STRATIX10_L4_MAIN_CLK>;
+ clock-names = "smmu_clk";
+ interrupt-parent = <&intc>;
+ interrupts = <0 128 4>, /* Global Secure Fault */
+ <0 129 4>, /* Global Non-secure Fault */
+ <0 130 4>, /* FPGA Performance Counter */
+ <0 131 4>, /* DMA Performance Counter */
+ <0 132 4>, /* EMAC Performance Counter */
+ <0 133 4>, /* IO Performance Counter */
+ <0 134 4>, /* SDM Performance Counter */
Note that there isn't much benefit to adding the secure or PMU
interrupts here other than to document the hardware - FWIW I have
actually been working on a PMU driver, and needless to say it turns
out not to be sufficient just having those munged into the SMMU global
fault handler.
Thanks for pointing this out. I was following other SMMU-500 device
trees. Just to clarify, how should I simplify this? Should I replace all
the above with the following?
<0 129 4>, /* Global Non-secure Fault */
Or will your upcoming PMU driver need the PMU units? It sounded like
using the just Global fault was not sufficient.
No, you had it right - what I meant was it's only worth including the
actual global fault signals themselves here (there's no harm in leaving
the secure one in if you like, it just won't do anything if the GIC is
configured correctly). What I found was that the current SMMU binding
leaves no reasonable way to encode the PMU interrupts in this interrupt
list that doesn't break at least one of the possible old/new driver/DT
combinations, so whatever happens they will need to be specified
separately once a PMU binding is defined.
+ <0 136 4>, /* Non-secure Combined Interrupt */
+ <0 137 4>, /* Secure Combined Interrupt */
Similarly the combined interrupt; that's literally just all these
other interrupt lines ORed together at the SMMU end, and would
generally only be useful if you *didn't* have the individual lines
wired up. As it stands with everything listed, any event will also
generate a spurious global fault IRQ, which isn't ideal (not that you
should get many interrupts during normal operation, but still...)
And I'd remove both of these then, right?
Indeed.
Thanks for the review and helpful comments (and also pointing out the
existing clock patch in my patch series summary)!
Cheers,
Robin.
--
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