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

[Kefeng Wang <wangkefeng.wang@huawei.com>]

On 2019/5/16 0:41, Lorenzo Pieralisi wrote:
> 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).

As Lorenzo said, it is Huawei CI system to improve the robustness of

our inner Linux kernel version, we also use it to test the latest Linux kernel.

The system is still being developed, and  still need to enhance and improve.

>>>
>>>> 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.
This is founded by randconfig built, it seems that no one use this kind of configuration
> 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.
Either way is ok, thanks.
>
> 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."
>>>
> .
>