* [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning
@ 2025-11-26 18:22 Krzysztof Kozlowski
2025-11-26 18:22 ` [PATCH 2/3] i2c: pxa: " Krzysztof Kozlowski
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-26 18:22 UTC (permalink / raw)
To: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Cc: Krzysztof Kozlowski
'type' is an enum, thus cast of pointer on 64-bit compile test with
clang and W=1 causes:
i2c-bcm-iproc.c:1102:3: error: cast to smaller integer type 'enum bcm_iproc_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
One of the discussions in 2023 on LKML suggested warning is not suitable
for kernel. Nothing changed in this regard since that time, so assume
the warning will stay and we want to have warnings-free builds.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
kernel_ulong_t is the preferred cast for such cases.
---
drivers/i2c/busses/i2c-bcm-iproc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
index e418a4f23f15..b5629cffe99b 100644
--- a/drivers/i2c/busses/i2c-bcm-iproc.c
+++ b/drivers/i2c/busses/i2c-bcm-iproc.c
@@ -1098,8 +1098,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, iproc_i2c);
iproc_i2c->device = &pdev->dev;
- iproc_i2c->type =
- (enum bcm_iproc_i2c_type)of_device_get_match_data(&pdev->dev);
+ iproc_i2c->type = (kernel_ulong_t)of_device_get_match_data(&pdev->dev);
init_completion(&iproc_i2c->done);
iproc_i2c->base = devm_platform_ioremap_resource(pdev, 0);
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] i2c: pxa: Fix Wvoid-pointer-to-enum-cast warning
2025-11-26 18:22 [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning Krzysztof Kozlowski
@ 2025-11-26 18:22 ` Krzysztof Kozlowski
2025-11-26 18:23 ` [PATCH 3/3] i2c: rcar: " Krzysztof Kozlowski
2025-12-03 17:38 ` [PATCH 1/3] i2c: bcm-iproc: " Andi Shyti
2 siblings, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-26 18:22 UTC (permalink / raw)
To: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Cc: Krzysztof Kozlowski
'i2c_types' is an enum, thus cast of pointer on 64-bit compile test with
clang and W=1 causes:
i2c-pxa.c:1269:15: error: cast to smaller integer type 'enum pxa_i2c_types' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
One of the discussions in 2023 on LKML suggested warning is not suitable
for kernel. Nothing changed in this regard since that time, so assume
the warning will stay and we want to have warnings-free builds.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-pxa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-pxa.c b/drivers/i2c/busses/i2c-pxa.c
index 968a8b8794da..09af3b3625f1 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1266,7 +1266,7 @@ static int i2c_pxa_probe_dt(struct platform_device *pdev, struct pxa_i2c *i2c,
i2c->use_pio = of_property_read_bool(np, "mrvl,i2c-polling");
i2c->fast_mode = of_property_read_bool(np, "mrvl,i2c-fast-mode");
- *i2c_types = (enum pxa_i2c_types)device_get_match_data(&pdev->dev);
+ *i2c_types = (kernel_ulong_t)device_get_match_data(&pdev->dev);
return 0;
}
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-26 18:22 [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning Krzysztof Kozlowski
2025-11-26 18:22 ` [PATCH 2/3] i2c: pxa: " Krzysztof Kozlowski
@ 2025-11-26 18:23 ` Krzysztof Kozlowski
2025-11-27 9:02 ` Geert Uytterhoeven
2025-12-03 17:38 ` [PATCH 1/3] i2c: bcm-iproc: " Andi Shyti
2 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-26 18:23 UTC (permalink / raw)
To: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Cc: Krzysztof Kozlowski
'i2c_types' is an enum, thus cast of pointer on 64-bit compile test with
clang and W=1 causes:
i2c-rcar.c:1144:18: error: cast to smaller integer type 'enum rcar_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
One of the discussions in 2023 on LKML suggested warning is not suitable
for kernel. Nothing changed in this regard since that time, so assume
the warning will stay and we want to have warnings-free builds.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/i2c/busses/i2c-rcar.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index d51884ab99f4..5ce8f8e4856f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
if (IS_ERR(priv->io))
return PTR_ERR(priv->io);
- priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
+ priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
init_waitqueue_head(&priv->wait);
adap = &priv->adap;
--
2.48.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-26 18:23 ` [PATCH 3/3] i2c: rcar: " Krzysztof Kozlowski
@ 2025-11-27 9:02 ` Geert Uytterhoeven
2025-11-27 11:42 ` Krzysztof Kozlowski
0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2025-11-27 9:02 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Hi Krzysztof,
On Wed, 26 Nov 2025 at 19:23, Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
> 'i2c_types' is an enum, thus cast of pointer on 64-bit compile test with
> clang and W=1 causes:
>
> i2c-rcar.c:1144:18: error: cast to smaller integer type 'enum rcar_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
>
> One of the discussions in 2023 on LKML suggested warning is not suitable
> for kernel. Nothing changed in this regard since that time, so assume
> the warning will stay and we want to have warnings-free builds.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Thanks for your patch!
=
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> if (IS_ERR(priv->io))
> return PTR_ERR(priv->io);
>
> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
The former seems to be the least common option.
FWIW, the most common option is "uintptr_t", which torvalds doesn't like...
> init_waitqueue_head(&priv->wait);
>
> adap = &priv->adap;
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 9:02 ` Geert Uytterhoeven
@ 2025-11-27 11:42 ` Krzysztof Kozlowski
2025-11-27 11:48 ` Krzysztof Kozlowski
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 11:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
On 27/11/2025 10:02, Geert Uytterhoeven wrote:
> Hi Krzysztof,
>
> On Wed, 26 Nov 2025 at 19:23, Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>> 'i2c_types' is an enum, thus cast of pointer on 64-bit compile test with
>> clang and W=1 causes:
>>
>> i2c-rcar.c:1144:18: error: cast to smaller integer type 'enum rcar_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
>>
>> One of the discussions in 2023 on LKML suggested warning is not suitable
>> for kernel. Nothing changed in this regard since that time, so assume
>> the warning will stay and we want to have warnings-free builds.
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> Thanks for your patch!
>
> =
>> --- a/drivers/i2c/busses/i2c-rcar.c
>> +++ b/drivers/i2c/busses/i2c-rcar.c
>> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>> if (IS_ERR(priv->io))
>> return PTR_ERR(priv->io);
>>
>> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
>> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
>
> Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
> The former seems to be the least common option.
As I wrote in the first patch, because to my knowledge it is the
preferred form for holding driver data which are in general pointers. We
do not store pointers as unsigned long. It is also already used for the
driver data types - see include/linux/mod_devicetable.h.
> FWIW, the most common option is "uintptr_t", which torvalds doesn't like...
Because it is discouraged in the kernel.
https://lore.kernel.org/all/2023081004-lapped-handbag-0324@gregkh/
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 11:42 ` Krzysztof Kozlowski
@ 2025-11-27 11:48 ` Krzysztof Kozlowski
2025-11-27 12:52 ` Geert Uytterhoeven
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 11:48 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
On 27/11/2025 12:42, Krzysztof Kozlowski wrote:
>>
>> =
>>> --- a/drivers/i2c/busses/i2c-rcar.c
>>> +++ b/drivers/i2c/busses/i2c-rcar.c
>>> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>>> if (IS_ERR(priv->io))
>>> return PTR_ERR(priv->io);
>>>
>>> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
>>> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
>>
>> Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
>> The former seems to be the least common option.
>
> As I wrote in the first patch, because to my knowledge it is the
> preferred form for holding driver data which are in general pointers. We
> do not store pointers as unsigned long. It is also already used for the
> driver data types - see include/linux/mod_devicetable.h.
... and in case it is still not obvious: I do not cast here to final
type, because that cast is the reason for the warning.
I cast to an intermediary type to help compiler suppressing the warning
- to integer representing the pointer. Unsigned long is not representing
pointers in the kernel. Integer type representing the pointer is
kernel_ulong_t, I think.
Once you have integer type representing the pointer, you do not need
further casts to enum.
>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 11:48 ` Krzysztof Kozlowski
@ 2025-11-27 12:52 ` Geert Uytterhoeven
2025-11-27 13:42 ` Krzysztof Kozlowski
0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2025-11-27 12:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Hi Krzysztof,
On Thu, 27 Nov 2025 at 12:48, Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
> On 27/11/2025 12:42, Krzysztof Kozlowski wrote:
> >>> --- a/drivers/i2c/busses/i2c-rcar.c
> >>> +++ b/drivers/i2c/busses/i2c-rcar.c
> >>> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> >>> if (IS_ERR(priv->io))
> >>> return PTR_ERR(priv->io);
> >>>
> >>> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
> >>> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
> >>
> >> Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
> >> The former seems to be the least common option.
> >
> > As I wrote in the first patch, because to my knowledge it is the
I don't see that written in the first patch; it has the same patch description
as the other patches in the series (mutatis mutandis)?
> > preferred form for holding driver data which are in general pointers. We
> > do not store pointers as unsigned long. It is also already used for the
> > driver data types - see include/linux/mod_devicetable.h.
>
> ... and in case it is still not obvious: I do not cast here to final
> type, because that cast is the reason for the warning.
>
> I cast to an intermediary type to help compiler suppressing the warning
I know...
> - to integer representing the pointer. Unsigned long is not representing
> pointers in the kernel. Integer type representing the pointer is
"unsigned long" indeed does not represent a pointer, but in the Linux
kernel, it is guaranteed to be the same size as a pointer.
'"unsigned long" is superior'
https://lore.kernel.org/CAHk-=wj2OHy-5e+srG1fy+ZU00TmZ1NFp6kFLbVLMXHe7A1d-g@mail.gmail.com
(he doesn't seem to have said anything about kernel_ulong_t)
> kernel_ulong_t, I think.
include/linux/mod_devicetable.h:typedef unsigned long kernel_ulong_t;
IIRC, it was introduced originally because "unsigned long" might have
a different size in userspace. Nowadays (for x32), uapi uses
__kernel_ulong_t, though.
> Once you have integer type representing the pointer, you do not need
> further casts to enum.
True.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 12:52 ` Geert Uytterhoeven
@ 2025-11-27 13:42 ` Krzysztof Kozlowski
2025-11-27 13:46 ` Geert Uytterhoeven
0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-27 13:42 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
On 27/11/2025 13:52, Geert Uytterhoeven wrote:
> Hi Krzysztof,
>
> On Thu, 27 Nov 2025 at 12:48, Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>> On 27/11/2025 12:42, Krzysztof Kozlowski wrote:
>>>>> --- a/drivers/i2c/busses/i2c-rcar.c
>>>>> +++ b/drivers/i2c/busses/i2c-rcar.c
>>>>> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>>>>> if (IS_ERR(priv->io))
>>>>> return PTR_ERR(priv->io);
>>>>>
>>>>> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
>>>>> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
>>>>
>>>> Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
>>>> The former seems to be the least common option.
>>>
>>> As I wrote in the first patch, because to my knowledge it is the
>
> I don't see that written in the first patch; it has the same patch description
> as the other patches in the series (mutatis mutandis)?
https://lore.kernel.org/all/20251126182257.157439-4-krzysztof.kozlowski@oss.qualcomm.com/
I meant that brief statement in the patch changelog.
>
>>> preferred form for holding driver data which are in general pointers. We
>>> do not store pointers as unsigned long. It is also already used for the
>>> driver data types - see include/linux/mod_devicetable.h.
>>
>> ... and in case it is still not obvious: I do not cast here to final
>> type, because that cast is the reason for the warning.
>>
>> I cast to an intermediary type to help compiler suppressing the warning
>
> I know...
>
>> - to integer representing the pointer. Unsigned long is not representing
>> pointers in the kernel. Integer type representing the pointer is
>
> "unsigned long" indeed does not represent a pointer, but in the Linux
> kernel, it is guaranteed to be the same size as a pointer.
>
> '"unsigned long" is superior'
> https://lore.kernel.org/CAHk-=wj2OHy-5e+srG1fy+ZU00TmZ1NFp6kFLbVLMXHe7A1d-g@mail.gmail.com
> (he doesn't seem to have said anything about kernel_ulong_t)
Yes, that was again comparing to uintptr_t, so we both agree.
>
>> kernel_ulong_t, I think.
>
> include/linux/mod_devicetable.h:typedef unsigned long kernel_ulong_t;
>
> IIRC, it was introduced originally because "unsigned long" might have
> a different size in userspace. Nowadays (for x32), uapi uses
> __kernel_ulong_t, though.
Maybe, but if you look at the data structures all have kernel_ulong_t,
so this fits the logic I was following here - I cast to the type which
is both representing pointer and is already used for driver data,
because this match data serves similar purpose as mentioned driver data.
I don't mind casting via unsigned long, but:
1. these are old and trivial issues,
2. they are quite annoying when people want to compile test with W=1,
3. I was trying to fix them (although not sure if for I2C) already,
4. and some others probably as well were trying to fix them,
so how many people need to send them and debate before we fix them finally?
unsigned long vs kernel_ulong_t is a nit-style discussion IMO, unless we
have here more concrete arguments, e.g. statement from Linus that
kernel_ulong_t is wrong.
If maintainers of this code want unsigned long, please apply the patch
and fix directly, but for the sake, let's get it merged...
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 13:42 ` Krzysztof Kozlowski
@ 2025-11-27 13:46 ` Geert Uytterhoeven
2025-11-28 8:32 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2025-11-27 13:46 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Hi Krzysztof,
On Thu, 27 Nov 2025 at 14:42, Krzysztof Kozlowski
<krzysztof.kozlowski@oss.qualcomm.com> wrote:
> On 27/11/2025 13:52, Geert Uytterhoeven wrote:
> > On Thu, 27 Nov 2025 at 12:48, Krzysztof Kozlowski
> > <krzysztof.kozlowski@oss.qualcomm.com> wrote:
> >> On 27/11/2025 12:42, Krzysztof Kozlowski wrote:
> >>>>> --- a/drivers/i2c/busses/i2c-rcar.c
> >>>>> +++ b/drivers/i2c/busses/i2c-rcar.c
> >>>>> @@ -1141,7 +1141,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> >>>>> if (IS_ERR(priv->io))
> >>>>> return PTR_ERR(priv->io);
> >>>>>
> >>>>> - priv->devtype = (enum rcar_i2c_type)of_device_get_match_data(dev);
> >>>>> + priv->devtype = (kernel_ulong_t)of_device_get_match_data(dev);
> >>>>
> >>>> Any specific reason you picked "kernel_ulong_t" instead of "unsigned long"?
> >>>> The former seems to be the least common option.
> >>>
> >>> As I wrote in the first patch, because to my knowledge it is the
> >
> > I don't see that written in the first patch; it has the same patch description
> > as the other patches in the series (mutatis mutandis)?
>
> https://lore.kernel.org/all/20251126182257.157439-4-krzysztof.kozlowski@oss.qualcomm.com/
>
> I meant that brief statement in the patch changelog.
Sorry, I had missed that, as I don't read patches for foreign hardware,
and had just b4-am'ed the series, and compared the two commits.
> >>> preferred form for holding driver data which are in general pointers. We
> >>> do not store pointers as unsigned long. It is also already used for the
> >>> driver data types - see include/linux/mod_devicetable.h.
> >>
> >> ... and in case it is still not obvious: I do not cast here to final
> >> type, because that cast is the reason for the warning.
> >>
> >> I cast to an intermediary type to help compiler suppressing the warning
> >
> > I know...
> >
> >> - to integer representing the pointer. Unsigned long is not representing
> >> pointers in the kernel. Integer type representing the pointer is
> >
> > "unsigned long" indeed does not represent a pointer, but in the Linux
> > kernel, it is guaranteed to be the same size as a pointer.
> >
> > '"unsigned long" is superior'
> > https://lore.kernel.org/CAHk-=wj2OHy-5e+srG1fy+ZU00TmZ1NFp6kFLbVLMXHe7A1d-g@mail.gmail.com
> > (he doesn't seem to have said anything about kernel_ulong_t)
>
> Yes, that was again comparing to uintptr_t, so we both agree.
>
> >> kernel_ulong_t, I think.
> >
> > include/linux/mod_devicetable.h:typedef unsigned long kernel_ulong_t;
> >
> > IIRC, it was introduced originally because "unsigned long" might have
> > a different size in userspace. Nowadays (for x32), uapi uses
> > __kernel_ulong_t, though.
>
> Maybe, but if you look at the data structures all have kernel_ulong_t,
> so this fits the logic I was following here - I cast to the type which
> is both representing pointer and is already used for driver data,
> because this match data serves similar purpose as mentioned driver data.
>
> I don't mind casting via unsigned long, but:
> 1. these are old and trivial issues,
> 2. they are quite annoying when people want to compile test with W=1,
> 3. I was trying to fix them (although not sure if for I2C) already,
> 4. and some others probably as well were trying to fix them,
> so how many people need to send them and debate before we fix them finally?
>
> unsigned long vs kernel_ulong_t is a nit-style discussion IMO, unless we
> have here more concrete arguments, e.g. statement from Linus that
> kernel_ulong_t is wrong.
>
> If maintainers of this code want unsigned long, please apply the patch
> and fix directly, but for the sake, let's get it merged...
Sure, let's get it fixed!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] i2c: rcar: Fix Wvoid-pointer-to-enum-cast warning
2025-11-27 13:46 ` Geert Uytterhoeven
@ 2025-11-28 8:32 ` Arnd Bergmann
0 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2025-11-28 8:32 UTC (permalink / raw)
To: Geert Uytterhoeven, Krzysztof Kozlowski
Cc: Andi Shyti, Ray Jui, Scott Branden,
Broadcom internal kernel review list, Wolfram Sang,
Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, Linux-Renesas, llvm
On Thu, Nov 27, 2025, at 14:46, Geert Uytterhoeven wrote:
> On Thu, 27 Nov 2025 at 14:42, Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> wrote:
>>
>> >> kernel_ulong_t, I think.
>> >
>> > include/linux/mod_devicetable.h:typedef unsigned long kernel_ulong_t;
>> >
>> > IIRC, it was introduced originally because "unsigned long" might have
>> > a different size in userspace. Nowadays (for x32), uapi uses
>> > __kernel_ulong_t, though.
>>
>> Maybe, but if you look at the data structures all have kernel_ulong_t,
>> so this fits the logic I was following here - I cast to the type which
>> is both representing pointer and is already used for driver data,
>> because this match data serves similar purpose as mentioned driver data.
It is rather inconsistent: The __kernel_ulong_t type is used in
include/uapi/ in places where x32 uses the 64-bit ABI, as the idea
was to not have to do (much) type conversion there. This of course
failed overall because any ioctl still has to be converted.
The kernel_ulong_t as far as I can tell was only meant to be
used in include/linux/mod_devicetable.h, with the idea of being
able to interpret 64-bit kernel modules from a 32-bit module
loader in userspace. I don't think the latest kmod actually uses
it that way any more.
>> I don't mind casting via unsigned long, but:
>> 1. these are old and trivial issues,
>> 2. they are quite annoying when people want to compile test with W=1,
>> 3. I was trying to fix them (although not sure if for I2C) already,
>> 4. and some others probably as well were trying to fix them,
>> so how many people need to send them and debate before we fix them finally?
>>
>> unsigned long vs kernel_ulong_t is a nit-style discussion IMO, unless we
>> have here more concrete arguments, e.g. statement from Linus that
>> kernel_ulong_t is wrong.
>>
>> If maintainers of this code want unsigned long, please apply the patch
>> and fix directly, but for the sake, let's get it merged...
>
> Sure, let's get it fixed!
Agreed. I don't mind the use of kernel_ulong_t here either, since it's
adjacent to the module device table entries, but I would also pick
'unsigned long' or 'uintptr_t' instead.
Arnd
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning
2025-11-26 18:22 [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning Krzysztof Kozlowski
2025-11-26 18:22 ` [PATCH 2/3] i2c: pxa: " Krzysztof Kozlowski
2025-11-26 18:23 ` [PATCH 3/3] i2c: rcar: " Krzysztof Kozlowski
@ 2025-12-03 17:38 ` Andi Shyti
2 siblings, 0 replies; 11+ messages in thread
From: Andi Shyti @ 2025-12-03 17:38 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Ray Jui, Scott Branden, Broadcom internal kernel review list,
Wolfram Sang, Geert Uytterhoeven, Magnus Damm, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt, linux-i2c,
linux-arm-kernel, linux-kernel, linux-renesas-soc, llvm
Hi Krzk,
All three patches merged to i2c/i2c-host.
Thanks,
Andi
On Wed, Nov 26, 2025 at 07:22:58PM +0100, Krzysztof Kozlowski wrote:
> 'type' is an enum, thus cast of pointer on 64-bit compile test with
> clang and W=1 causes:
>
> i2c-bcm-iproc.c:1102:3: error: cast to smaller integer type 'enum bcm_iproc_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast]
>
> One of the discussions in 2023 on LKML suggested warning is not suitable
> for kernel. Nothing changed in this regard since that time, so assume
> the warning will stay and we want to have warnings-free builds.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> kernel_ulong_t is the preferred cast for such cases.
> ---
> drivers/i2c/busses/i2c-bcm-iproc.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c
> index e418a4f23f15..b5629cffe99b 100644
> --- a/drivers/i2c/busses/i2c-bcm-iproc.c
> +++ b/drivers/i2c/busses/i2c-bcm-iproc.c
> @@ -1098,8 +1098,7 @@ static int bcm_iproc_i2c_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, iproc_i2c);
> iproc_i2c->device = &pdev->dev;
> - iproc_i2c->type =
> - (enum bcm_iproc_i2c_type)of_device_get_match_data(&pdev->dev);
> + iproc_i2c->type = (kernel_ulong_t)of_device_get_match_data(&pdev->dev);
> init_completion(&iproc_i2c->done);
>
> iproc_i2c->base = devm_platform_ioremap_resource(pdev, 0);
> --
> 2.48.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-12-03 17:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-26 18:22 [PATCH 1/3] i2c: bcm-iproc: Fix Wvoid-pointer-to-enum-cast warning Krzysztof Kozlowski
2025-11-26 18:22 ` [PATCH 2/3] i2c: pxa: " Krzysztof Kozlowski
2025-11-26 18:23 ` [PATCH 3/3] i2c: rcar: " Krzysztof Kozlowski
2025-11-27 9:02 ` Geert Uytterhoeven
2025-11-27 11:42 ` Krzysztof Kozlowski
2025-11-27 11:48 ` Krzysztof Kozlowski
2025-11-27 12:52 ` Geert Uytterhoeven
2025-11-27 13:42 ` Krzysztof Kozlowski
2025-11-27 13:46 ` Geert Uytterhoeven
2025-11-28 8:32 ` Arnd Bergmann
2025-12-03 17:38 ` [PATCH 1/3] i2c: bcm-iproc: " Andi Shyti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox