| Message ID | 20190903024212.20300-1-yuehaibing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] iommu/arm-smmu-v3: Fix build error without CONFIG_PCI_ATS | expand |
On Tue, Sep 03, 2019 at 10:42:12AM +0800, YueHaibing wrote: > If CONFIG_PCI_ATS is not set, building fails: > > drivers/iommu/arm-smmu-v3.c: In function arm_smmu_ats_supported: > drivers/iommu/arm-smmu-v3.c:2325:35: error: struct pci_dev has no member named ats_cap; did you mean msi_cap? > return !pdev->untrusted && pdev->ats_cap; > ^~~~~~~ > > ats_cap should only used when CONFIG_PCI_ATS is defined, > so use #ifdef block to guard this. > > Fixes: bfff88ec1afe ("iommu/arm-smmu-v3: Rework enabling/disabling of ATS for PCI masters") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > drivers/iommu/arm-smmu-v3.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index 66bf641..44ac9ac 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -2313,7 +2313,7 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master) > > static bool arm_smmu_ats_supported(struct arm_smmu_master *master) > { > - struct pci_dev *pdev; > + struct pci_dev *pdev __maybe_unused; > struct arm_smmu_device *smmu = master->smmu; > struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev); > > @@ -2321,8 +2321,10 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master) > !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS) || pci_ats_disabled()) > return false; > > +#ifdef CONFIG_PCI_ATS > pdev = to_pci_dev(master->dev); > return !pdev->untrusted && pdev->ats_cap; > +#endif > } Hmm, I really don't like the missing return statement here, even though we never get this far thanks to the feature not getting set during ->probe(). I'd actually prefer just to duplicate the function: #ifndef CONFIG_PCI_ATS static bool arm_smmu_ats_supported(struct arm_smmu_master *master) { return false; } #else <current code here> #endif Can you send a v2 like that, please? Will
On 2019/9/3 14:30, Will Deacon wrote: > On Tue, Sep 03, 2019 at 10:42:12AM +0800, YueHaibing wrote: >> If CONFIG_PCI_ATS is not set, building fails: >> >> drivers/iommu/arm-smmu-v3.c: In function arm_smmu_ats_supported: >> drivers/iommu/arm-smmu-v3.c:2325:35: error: struct pci_dev has no member named ats_cap; did you mean msi_cap? >> return !pdev->untrusted && pdev->ats_cap; >> ^~~~~~~ >> >> ats_cap should only used when CONFIG_PCI_ATS is defined, >> so use #ifdef block to guard this. >> >> Fixes: bfff88ec1afe ("iommu/arm-smmu-v3: Rework enabling/disabling of ATS for PCI masters") >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> >> --- >> drivers/iommu/arm-smmu-v3.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index 66bf641..44ac9ac 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -2313,7 +2313,7 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master) >> >> static bool arm_smmu_ats_supported(struct arm_smmu_master *master) >> { >> - struct pci_dev *pdev; >> + struct pci_dev *pdev __maybe_unused; >> struct arm_smmu_device *smmu = master->smmu; >> struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev); >> >> @@ -2321,8 +2321,10 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master) >> !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS) || pci_ats_disabled()) >> return false; >> >> +#ifdef CONFIG_PCI_ATS >> pdev = to_pci_dev(master->dev); >> return !pdev->untrusted && pdev->ats_cap; >> +#endif >> } > > Hmm, I really don't like the missing return statement here, even though we > never get this far thanks to the feature not getting set during ->probe(). > I'd actually prefer just to duplicate the function: > > #ifndef CONFIG_PCI_ATS > static bool > arm_smmu_ats_supported(struct arm_smmu_master *master) { return false; } > #else > <current code here> > #endif > > Can you send a v2 like that, please? Ok, will send v2 as your suggestion. > > Will > > . >
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 66bf641..44ac9ac 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -2313,7 +2313,7 @@ static void arm_smmu_install_ste_for_dev(struct arm_smmu_master *master) static bool arm_smmu_ats_supported(struct arm_smmu_master *master) { - struct pci_dev *pdev; + struct pci_dev *pdev __maybe_unused; struct arm_smmu_device *smmu = master->smmu; struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev); @@ -2321,8 +2321,10 @@ static bool arm_smmu_ats_supported(struct arm_smmu_master *master) !(fwspec->flags & IOMMU_FWSPEC_PCI_RC_ATS) || pci_ats_disabled()) return false; +#ifdef CONFIG_PCI_ATS pdev = to_pci_dev(master->dev); return !pdev->untrusted && pdev->ats_cap; +#endif } static void arm_smmu_enable_ats(struct arm_smmu_master *master)
If CONFIG_PCI_ATS is not set, building fails: drivers/iommu/arm-smmu-v3.c: In function arm_smmu_ats_supported: drivers/iommu/arm-smmu-v3.c:2325:35: error: struct pci_dev has no member named ats_cap; did you mean msi_cap? return !pdev->untrusted && pdev->ats_cap; ^~~~~~~ ats_cap should only used when CONFIG_PCI_ATS is defined, so use #ifdef block to guard this. Fixes: bfff88ec1afe ("iommu/arm-smmu-v3: Rework enabling/disabling of ATS for PCI masters") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/iommu/arm-smmu-v3.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)