On Tue, 19 Nov 2024 01:50:18 +0100 Andrew Lunn <andrew@xxxxxxx> wrote: > > This is not a new dt property, the "iommus" property is an existing property > > that is parsed by the Nvidia implementation of the arm-smmu driver. > > > > Here is a snippet from the device tree: > > > > smmu_niso0: iommu@12000000 { > > compatible = "nvidia,tegra234-smmu", "nvidia,smmu-500"; > > ... > > } > > > > /* MGBE0 */ > > ethernet@6800000 { > > compatible = "nvidia,tegra234-mgbe"; > > ... > > iommus = <&smmu_niso0 TEGRA234_SID_MGBE>; > > ... > > } > > > > /* MGBE1 */ > > ethernet@6900000 { > > compatible = "nvidia,tegra234-mgbe"; > > ... > > iommus = <&smmu_niso0 TEGRA234_SID_MGBE_VF1>; > > ... > > } > > What i was meaning does the nvidia,tegra234-mgbe binding allow iommus? > I just checked, yes it does. > > > If the iommus property is missing completely from the MGBE's device tree node it > > causes secure read/write errors which spam the kernel log and can cause crashes. > > > > I can add the fallback in V2 with a warning if that is preferred. > > The fact it crashed makes me think it is optional. Any existing users > must work, otherwise it would crash, and then be debugged. I guess you > are pushing the usage further, and so have come across this condition. > > Is the iommus a SoC property, or a board property? If it is a SoC > property, could you review all the SoC .dtsi files and fix up any > which are missing the property? > > Adding a warning is O.K, but ideally the missing property should be > added first. I think there is some confusion here. I will try to summarize: - Ihe iommu is supported by the Tegra SOC. - The way the mgbe driver is written the iommu DT property is REQUIRED. - "iommus" is a SOC DT property and is defined in tegra234.dtsi. - The mgbe device tree nodes in tegra234.dtsi DO have the iommus property. - There are no device tree changes required to to make this patch work. - This patch works fine with existing device trees. I will add the fallback however in case there is changes made to the iommu subsystem in the future. > The merge window is open now, so patches will need to wait two weeks. > Ok thanks, I will wait a couple weeks to resend. Parker