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
