Messages in this thread | | | Subject | Re: [PATCH] platform/surface: aggregator: shut up clang -Wconstantn-conversion warning | From | Hans de Goede <> | Date | Fri, 14 May 2021 21:41:07 +0200 |
| |
Hi,
On 5/14/21 9:38 PM, Hans de Goede wrote: > Hi, > > On 5/14/21 4:21 PM, Maximilian Luz wrote: >> On 5/14/21 4:05 PM, Arnd Bergmann wrote: >>> From: Arnd Bergmann <arnd@arndb.de> >>> >>> Clang complains about the assignment of SSAM_ANY_IID to >>> ssam_device_uid->instance: >>> >>> drivers/platform/surface/surface_aggregator_registry.c:478:25: error: implicit conversion from 'int' to '__u8' (aka 'unsigned char') changes value from 65535 to 255 [-Werror,-Wconstant-conversion] >>> { SSAM_VDEV(HUB, 0x02, SSAM_ANY_IID, 0x00) }, >>> ~ ^~~~~~~~~~~~ >>> include/linux/surface_aggregator/device.h:71:23: note: expanded from macro 'SSAM_ANY_IID' >>> #define SSAM_ANY_IID 0xffff >>> ^~~~~~ >>> include/linux/surface_aggregator/device.h:126:63: note: expanded from macro 'SSAM_VDEV' >>> SSAM_DEVICE(SSAM_DOMAIN_VIRTUAL, SSAM_VIRTUAL_TC_##cat, tid, iid, fun) >>> ^~~ >>> include/linux/surface_aggregator/device.h:102:41: note: expanded from macro 'SSAM_DEVICE' >>> .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0, \ >>> ^~~ >>> >>> The assignment doesn't actually happen, but clang checks the type limits >>> before checking whether this assignment is reached. Shut up the warning >>> using an explicit type cast. >> >> I'm not too happy about this fix as (I believe) it will also shut up any >> valid GCC error message in case those macros are used with non-u8 (and >> non-SSAM_ANY_xxx) values. > > Since you're the maintainer of this code, I'll go with your judgement here,
p.s.
I just went to patchwork to drop this patch from the queue:
https://patchwork.kernel.org/projecat/platform-driver-x86/list/
But it never got added there because platform-driver-x86@vger.kernel.org was missing from the Cc even though get_maintainer.pl lists it.
Regards,
Hans
>> I'll let others judge and decide what's preferred, however. >> >> In case you're deciding to apply this, feel free to add: >> >> Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com> >> >> Thanks, >> Max >> >>> Fixes: eb0e90a82098 ("platform/surface: aggregator: Add dedicated bus and device type") >>> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >>> --- >>> include/linux/surface_aggregator/device.h | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/include/linux/surface_aggregator/device.h b/include/linux/surface_aggregator/device.h >>> index 4441ad667c3f..90df092ed565 100644 >>> --- a/include/linux/surface_aggregator/device.h >>> +++ b/include/linux/surface_aggregator/device.h >>> @@ -98,9 +98,9 @@ struct ssam_device_uid { >>> | (((fun) != SSAM_ANY_FUN) ? SSAM_MATCH_FUNCTION : 0), \ >>> .domain = d, \ >>> .category = cat, \ >>> - .target = ((tid) != SSAM_ANY_TID) ? (tid) : 0, \ >>> - .instance = ((iid) != SSAM_ANY_IID) ? (iid) : 0, \ >>> - .function = ((fun) != SSAM_ANY_FUN) ? (fun) : 0 \ >>> + .target = ((tid) != SSAM_ANY_TID) ? (u8)(tid) : 0, \ >>> + .instance = ((iid) != SSAM_ANY_IID) ? (u8)(iid) : 0, \ >>> + .function = ((fun) != SSAM_ANY_FUN) ? (u8)(fun) : 0 \ >>> /** >>> * SSAM_VDEV() - Initialize a &struct ssam_device_id as virtual device with >>> >>
| |