On Wed, Sep 30, 2020 at 08:59:45AM +0300, Dmitry Osipenko wrote: > 30.09.2020 08:41, Nicolin Chen пишет: > > On Wed, Sep 30, 2020 at 08:39:54AM +0300, Dmitry Osipenko wrote: > >> 30.09.2020 03:30, Nicolin Chen пишет: > >>> static int tegra_smmu_attach_dev(struct iommu_domain *domain, > >>> struct device *dev) > >>> { > >>> + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev); > >>> struct tegra_smmu *smmu = dev_iommu_priv_get(dev); > >>> struct tegra_smmu_as *as = to_smmu_as(domain); > >>> - struct device_node *np = dev->of_node; > >>> - struct of_phandle_args args; > >>> unsigned int index = 0; > >>> int err = 0; > >>> > >>> - while (!of_parse_phandle_with_args(np, "iommus", "#iommu-cells", index, > >>> - &args)) { > >>> - unsigned int swgroup = args.args[0]; > >>> - > >>> - if (args.np != smmu->dev->of_node) { > >>> - of_node_put(args.np); > >>> - continue; > >>> - } > >>> - > >>> - of_node_put(args.np); > >>> + if (!fwspec || fwspec->ops != &tegra_smmu_ops) > >>> + return -ENOENT; > >> > >> s/&tegra_smmu_ops/smmu->iommu.ops/ > >> > >> Secondly, is it even possible that fwspec could be NULL here or that > >> fwspec->ops != smmu->ops? > > > > I am following what's in the arm-smmu driver, as I think it'd be > > a common practice to do such a check in such a way. > > > > Please check whether it's really needed. It looks like it was needed > sometime ago, but that's not true anymore. Given that most iommu drivers have ->ops check, I'd like to have it also for safety. If someday that's not true anymore, I'd expect someone to update all existing drivers.