Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
On Wed, Sep 30, 2020 at 08:58:50AM +0300, Dmitry Osipenko wrote: > 30.09.2020 08:29, Nicolin Chen пишет: > > Hi Dmitry, > > > > On Wed, Sep 30, 2020 at 08:10:07AM +0300, Dmitry Osipenko wrote: > >> 30.09.2020 03:30, Nicolin Chen пишет: > >>> - group->group = iommu_group_alloc(); > >>> + group->group = pci ? pci_device_group(dev) : iommu_group_alloc(); > >> > >> This will be nicer to write as: > >> > >> if (dev_is_pci(dev)) > >> group->group = pci_device_group(dev); > >> else > >> group->group = generic_device_group(dev); > > > > Why is that nicer? I don't feel mine is hard to read at all... > > > > Previously you said that you're going to add USB support, so I assume > there will be something about USB. I was hoping that it'd work for USB. Yet USB doesn't seem to have an different function for device_group(). > It's also a kinda common style that majority of Tegra drivers use in > upstream kernel. But yours variant is good too if there won't be a need > to change it later on. Okay..I'll use yours then. Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
On Wed, Sep 30, 2020 at 09:01:09AM +0300, Dmitry Osipenko wrote: > 30.09.2020 08:34, Nicolin Chen пишет: > > On Wed, Sep 30, 2020 at 08:10:35AM +0300, Dmitry Osipenko wrote: > >> 30.09.2020 03:30, Nicolin Chen пишет: > >>> void tegra_smmu_remove(struct tegra_smmu *smmu) > >>> { > >>> + bus_set_iommu(_bus_type, NULL); > >> > >> Why only platform_bus? Is this really needed at all? > > > > I see qcom_iommu.c file set to NULL in remove(), Probably should > > have added pci_bus_type too though. > > > > Or are you sure that there's no need at all? > > > > It wasn't here before this patch and platform_bus is unrelated to the > topic of this patch. But it probably should be there. > > On the other hand, the tegra_smmu_remove() is unused and maybe it could > be better to get rid of this unused function at all. I will drop this line then, since no one is calling it. And maybe we can submit a clean up patch later. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
30.09.2020 08:34, Nicolin Chen пишет: > On Wed, Sep 30, 2020 at 08:10:35AM +0300, Dmitry Osipenko wrote: >> 30.09.2020 03:30, Nicolin Chen пишет: >>> void tegra_smmu_remove(struct tegra_smmu *smmu) >>> { >>> + bus_set_iommu(_bus_type, NULL); >> >> Why only platform_bus? Is this really needed at all? > > I see qcom_iommu.c file set to NULL in remove(), Probably should > have added pci_bus_type too though. > > Or are you sure that there's no need at all? > It wasn't here before this patch and platform_bus is unrelated to the topic of this patch. But it probably should be there. On the other hand, the tegra_smmu_remove() is unused and maybe it could be better to get rid of this unused function at all. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
30.09.2020 08:29, Nicolin Chen пишет: > Hi Dmitry, > > On Wed, Sep 30, 2020 at 08:10:07AM +0300, Dmitry Osipenko wrote: >> 30.09.2020 03:30, Nicolin Chen пишет: >>> - group->group = iommu_group_alloc(); >>> + group->group = pci ? pci_device_group(dev) : iommu_group_alloc(); >> >> This will be nicer to write as: >> >> if (dev_is_pci(dev)) >> group->group = pci_device_group(dev); >> else >> group->group = generic_device_group(dev); > > Why is that nicer? I don't feel mine is hard to read at all... > Previously you said that you're going to add USB support, so I assume there will be something about USB. It's also a kinda common style that majority of Tegra drivers use in upstream kernel. But yours variant is good too if there won't be a need to change it later on. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
On Wed, Sep 30, 2020 at 08:10:35AM +0300, Dmitry Osipenko wrote: > 30.09.2020 03:30, Nicolin Chen пишет: > > void tegra_smmu_remove(struct tegra_smmu *smmu) > > { > > + bus_set_iommu(_bus_type, NULL); > > Why only platform_bus? Is this really needed at all? I see qcom_iommu.c file set to NULL in remove(), Probably should have added pci_bus_type too though. Or are you sure that there's no need at all? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
Hi Dmitry, On Wed, Sep 30, 2020 at 08:10:07AM +0300, Dmitry Osipenko wrote: > 30.09.2020 03:30, Nicolin Chen пишет: > > - group->group = iommu_group_alloc(); > > + group->group = pci ? pci_device_group(dev) : iommu_group_alloc(); > > This will be nicer to write as: > > if (dev_is_pci(dev)) > group->group = pci_device_group(dev); > else > group->group = generic_device_group(dev); Why is that nicer? I don't feel mine is hard to read at all... ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
On Wed, Sep 30, 2020 at 08:10:00AM +0300, Dmitry Osipenko wrote: > 30.09.2020 03:30, Nicolin Chen пишет: > ... > > +#ifdef CONFIG_PCI > > + if (!iommu_present(_bus_type)) { > > Could you please explain why this check is needed? That's referencing what's in the arm-smmu.c file, since it does not hurt to have a check. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
30.09.2020 03:30, Nicolin Chen пишет: > void tegra_smmu_remove(struct tegra_smmu *smmu) > { > + bus_set_iommu(_bus_type, NULL); Why only platform_bus? Is this really needed at all? > iommu_device_unregister(>iommu); > iommu_device_sysfs_remove(>iommu); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
30.09.2020 03:30, Nicolin Chen пишет: ... > +#ifdef CONFIG_PCI > + if (!iommu_present(_bus_type)) { Could you please explain why this check is needed? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
30.09.2020 03:30, Nicolin Chen пишет: > - group->group = iommu_group_alloc(); > + group->group = pci ? pci_device_group(dev) : iommu_group_alloc(); This will be nicer to write as: if (dev_is_pci(dev)) group->group = pci_device_group(dev); else group->group = generic_device_group(dev); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 3/3] iommu/tegra-smmu: Add PCI support
This patch simply adds support for PCI devices. Also reverses bus_set_iommu in tegra_smmu_remove(). Signed-off-by: Nicolin Chen --- Changelog v1->v2 * Added error-out labels in tegra_smmu_probe() * Dropped pci_request_acs() since IOMMU core would call it. drivers/iommu/tegra-smmu.c | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index cf4981369828..74d84908679e 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -856,6 +857,7 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) const struct tegra_smmu_group_soc *soc; unsigned int swgroup = fwspec->ids[0]; struct tegra_smmu_group *group; + bool pci = dev_is_pci(dev); struct iommu_group *grp; /* Find group_soc associating with swgroup */ @@ -882,7 +884,7 @@ static struct iommu_group *tegra_smmu_device_group(struct device *dev) group->smmu = smmu; group->soc = soc; - group->group = iommu_group_alloc(); + group->group = pci ? pci_device_group(dev) : iommu_group_alloc(); if (IS_ERR(group->group)) { devm_kfree(smmu->dev, group); mutex_unlock(>lock); @@ -1079,26 +1081,39 @@ struct tegra_smmu *tegra_smmu_probe(struct device *dev, iommu_device_set_fwnode(>iommu, dev->fwnode); err = iommu_device_register(>iommu); - if (err) { - iommu_device_sysfs_remove(>iommu); - return ERR_PTR(err); - } + if (err) + goto err_sysfs; err = bus_set_iommu(_bus_type, _smmu_ops); - if (err < 0) { - iommu_device_unregister(>iommu); - iommu_device_sysfs_remove(>iommu); - return ERR_PTR(err); + if (err < 0) + goto err_unregister; + +#ifdef CONFIG_PCI + if (!iommu_present(_bus_type)) { + err = bus_set_iommu(_bus_type, _smmu_ops); + if (err < 0) + goto err_bus_set; } +#endif if (IS_ENABLED(CONFIG_DEBUG_FS)) tegra_smmu_debugfs_init(smmu); return smmu; + +err_bus_set: + bus_set_iommu(_bus_type, NULL); +err_unregister: + iommu_device_unregister(>iommu); +err_sysfs: + iommu_device_sysfs_remove(>iommu); + + return ERR_PTR(err); } void tegra_smmu_remove(struct tegra_smmu *smmu) { + bus_set_iommu(_bus_type, NULL); iommu_device_unregister(>iommu); iommu_device_sysfs_remove(>iommu); -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu