Re: [PATCH RESEND] ACPI/IORT: Fix build error when IOMMU_SUPPORT disabled

[Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>]

On Wed, May 15, 2019 at 01:46:28PM +0100, Robin Murphy wrote:
> On 15/05/2019 13:06, Lorenzo Pieralisi wrote:
> > [+Joerg, Robin]
> > 
> > On Wed, May 15, 2019 at 11:15:34AM +0100, Will Deacon wrote:
> > > On Wed, May 15, 2019 at 11:42:53AM +0800, Kefeng Wang wrote:
> > > > drivers/acpi/arm64/iort.c: In function iort_iommu_configure:
> > > > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags
> > > >      dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
> > > >                       ^~
> > > > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS
> > > > undeclared (first use in this function)
> > > >      dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
> > > >                                  ^~~~~~~~~~~~~~~~~~~~~~~
> > > > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in
> > > > 
> > > > If IOMMU_SUPPORT not enabled, struct iommu_fwspec without members and
> > > > IOMMU_FWSPEC_PCI_RC_ATS not defined, add new iommu_fwspec_set_ats_flags()
> > > > to set IOMMU_FWSPEC_PCI_RC_ATS flags to solve build error.
> > > > 
> > > > Cc: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
> > > > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Reported-by: Hulk Robot <hulkci@huawei.com>
> > > 
> > > What's this "Hulk Robot"? Does it report things publicly?
> > 
> > AFAIK it is Huawei CI system, I had some doubts about leaving
> > that tag for other patches I dealt with, I am not sure it is
> > really useful (other than giving Hulk some credit).
> > 
> > > Anyway, I can pick this up if Lorenzo acks it.
> > 
> > Actually I think we should compile out iort_iommu_configure() for
> > !IOMMU_SUPPORT unless I am missing something, so that we are
> > removing this source of errors.
> 
> Indeed, much as I like the compile coverage of not having things #ifdefed
> out in general, this particular case seems like a fairly atypical
> configuration so I'm not sure all the extra fluff of struct accessors is
> really worth it.

Ok, I will send a patch shortly to address this issue, if Kefeng
does not object I will drop the CI bot as reporter and add him
instead.

Thanks,
Lorenzo

> 
> Robin.
> 
> > If that does not cut it, for this patch:
> > 
> > iommu_fwspec_set_ats_flags() should be iommu_fwspec_set_ats_flag()
> > 
> > The commit log should be fixed (it is pretty weird to start with the
> > build error and then explain it):
> > 
> > "If IOMMU_SUPPORT is not enabled, struct iommu_fwspec is an empty
> > struct and IOMMU_FWSPEC_PCI_RC_ATS is not defined, resulting in
> > the following build error:
> > 
> > drivers/acpi/arm64/iort.c: In function iort_iommu_configure:
> > drivers/acpi/arm64/iort.c:1079:21: error: struct iommu_fwspec has no member named flags
> >      dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
> >                       ^~
> > drivers/acpi/arm64/iort.c:1079:32: error: IOMMU_FWSPEC_PCI_RC_ATS
> > undeclared (first use in this function)
> >      dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_RC_ATS;
> >                                  ^~~~~~~~~~~~~~~~~~~~~~~
> > drivers/acpi/arm64/iort.c:1079:32: note: each undeclared identifier is reported only once for each function it appears in
> > 
> > Add a iommu_fwspec_set_ats_flag() helper function (that is an empty
> > stub on !IOMMU_SUPPORT) to fix it."
> >