Hi Stewart, > On 15 Dec 2022, at 3:10 pm, Stewart Hildebrand <[email protected]> > wrote: > > On 12/15/22 09:51, Julien Grall wrote: >> Hi Stewart, >> >> On 15/12/2022 14:11, Stewart Hildebrand wrote: >>> On 12/15/22 06:34, Julien Grall wrote: >>>> Hi Stewart, >>>> >>>> I was about to commit this patch when I noticed the placement of the >>>> attribute doesn't match what we are usually doing in Xen. >>>> >>>> On 13/12/2022 18:18, Stewart Hildebrand wrote: >>>>> When building with clang 12 and CONFIG_ARM_SMMU_V3=y, we observe the >>>>> following build error: >>>>> >>>>> drivers/passthrough/arm/smmu-v3.c:1408:20: error: unused function >>>>> 'arm_smmu_disable_pasid' [-Werror,-Wunused-function] >>>>> static inline void arm_smmu_disable_pasid(struct arm_smmu_master *master) >>>>> { } >>>>> ^ >>>>> >>>>> arm_smmu_disable_pasid is not currently called from anywhere in Xen, but >>>>> it is inside a section of code guarded by CONFIG_PCI_ATS, which may be >>>>> helpful in the future if the PASID feature is to be implemented. Add the >>>>> attribute __maybe_unused to the function. >>>>> >>>>> Signed-off-by: Stewart Hildebrand <[email protected]> >>>>> --- >>>>> v1->v2: >>>>> Add __maybe_unused attribute instead of removing >>>>> --- >>>>> xen/drivers/passthrough/arm/smmu-v3.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c >>>>> b/xen/drivers/passthrough/arm/smmu-v3.c >>>>> index 9c9f4630090e..0cdc862f96d1 100644 >>>>> --- a/xen/drivers/passthrough/arm/smmu-v3.c >>>>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c >>>>> @@ -1376,6 +1376,7 @@ static int arm_smmu_enable_pasid(struct >>>>> arm_smmu_master *master) >>>>> return 0; >>>>> } >>>>> >>>>> +__maybe_unused >>>>> static void arm_smmu_disable_pasid(struct arm_smmu_master *master) >>>> >>>> The attribute should be placed after "void". I.e.: >>>> >>>> static void __maybe_unused arm_smmu_disable_pasid(...) >>> >>> I had initially tried placing it exactly where you suggest in the first >>> draft of v2 of this patch. However, the line would then exceed 72 >>> characters (actual 81 characters): >> >> This doesn't change the problem here but the limit is 80 characters per >> line rather than 72. >> >>> >>> static void __maybe_unused arm_smmu_disable_pasid(struct arm_smmu_master >>> *master) >>> >>> So I found myself juggling with how best to wrap it. How about a newline >>> after the __maybe_unused attribute? >>> >>> static void __maybe_unused >>> arm_smmu_disable_pasid(struct arm_smmu_master *master) >>> >>> and similarly for the 2nd occurrence: >>> >>> static inline void __maybe_unused >>> arm_smmu_disable_pasid(struct arm_smmu_master *master) { } >>> >>> There is precedent for this style of wrapping in xen/common/sched/credit2.c. >> >> Ah! I didn't realize the line would have been too long. In this case, >> the newline after __maybe_unused is the way to go. > > Ok, I will send a v3 with this change. > > Rahul - may I retain your R-b tag in v3?
Yes you can retain my R-b. Regards, Rahul
