[PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Justin Stitt]

When building with clang 18 I see the following warning:
|       drivers/i2c/busses/i2c-pxa.c:1267:15: warning: cast to smaller integer
|       type 'enum pxa_i2c_types' from 'const void *' [-Wvoid-pointer-to-enum-cast]
|        1267 |         *i2c_types = (enum pxa_i2c_types)(of_id->data);

This is due to the fact that `of_id->data` is a void* while `enum pxa_i2c_types`
has the size of an int.

Cast `of_id->data` to a uintptr_t to silence the above warning for clang
builds using W=1

Link: https://github.com/ClangBuiltLinux/linux/issues/1910
Reported-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Note: I think something like this may be more readable:
| 	*i2c_types = (enum pxa_i2c_types)(uintptr_t)of_id->data;

Thoughts on this approach against the one present in this patch?
---
 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 937f7eebe906..20d1132d3d69 100644
--- a/drivers/i2c/busses/i2c-pxa.c
+++ b/drivers/i2c/busses/i2c-pxa.c
@@ -1264,7 +1264,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)(of_id->data);
+	*i2c_types = (uintptr_t)of_id->data;
 
 	return 0;
 }

---
base-commit: 2ccdd1b13c591d306f0401d98dedc4bdcd02b421
change-id: 20230816-void-drivers-i2c-busses-i2c-pxa-aaf94f5c39e0

Best regards,
--
Justin Stitt <justinstitt@google.com>

Re: [PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 462 bytes --]


> Note: I think something like this may be more readable:
> | 	*i2c_types = (enum pxa_i2c_types)(uintptr_t)of_id->data;
> 
> Thoughts on this approach against the one present in this patch?

On the one hand, I think this is more explicit and, thus, more readable.
On the other hand, we still have the loss of precision, between the
first and the second cast. Which gives it a bit of a "let's hide it
somewhat so the compiler will be happy" feeling?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Justin Stitt]

On Fri, Aug 25, 2023 at 3:17 PM Wolfram Sang <wsa@kernel.org> wrote:
>
>
> > Note: I think something like this may be more readable:
> > |     *i2c_types = (enum pxa_i2c_types)(uintptr_t)of_id->data;
> >
> > Thoughts on this approach against the one present in this patch?
>
> On the one hand, I think this is more explicit and, thus, more readable.
> On the other hand, we still have the loss of precision, between the
> first and the second cast. Which gives it a bit of a "let's hide it
> somewhat so the compiler will be happy" feeling?
>
There was some discussion [1] wherein it was ultimately decided that
this warning should probably be turned off (contrary to what the title
of the GitHub issue says).

The state of these patches [2] is in some sort of limbo until I get a
patch in to disable the warning from W=1 (keep in mind GCC doesn't
even support this specific warning). I want to make the patch but am
seeking some guidance about what exactly is to be done -- I feel a
simply _demotion_ from W=1 to W=2 would suffice as CI robots aren't
testing w/ that AFAIK.

Nick, do you have anything to add here as we had previously discussed
this off-list/IRL.

[1]: https://github.com/ClangBuiltLinux/linux/issues/1910
[2]: https://lore.kernel.org/all/?q=b%3Apointer-to-enum-cast

Re: [PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Nick Desaulniers]

On Fri, Aug 25, 2023 at 3:49 PM Justin Stitt <justinstitt@google.com> wrote:
>
> On Fri, Aug 25, 2023 at 3:17 PM Wolfram Sang <wsa@kernel.org> wrote:
> >
> >
> > > Note: I think something like this may be more readable:
> > > |     *i2c_types = (enum pxa_i2c_types)(uintptr_t)of_id->data;
> > >
> > > Thoughts on this approach against the one present in this patch?
> >
> > On the one hand, I think this is more explicit and, thus, more readable.
> > On the other hand, we still have the loss of precision, between the
> > first and the second cast. Which gives it a bit of a "let's hide it
> > somewhat so the compiler will be happy" feeling?
> >
> There was some discussion [1] wherein it was ultimately decided that
> this warning should probably be turned off (contrary to what the title
> of the GitHub issue says).
>
> The state of these patches [2] is in some sort of limbo until I get a
> patch in to disable the warning from W=1 (keep in mind GCC doesn't
> even support this specific warning). I want to make the patch but am
> seeking some guidance about what exactly is to be done -- I feel a
> simply _demotion_ from W=1 to W=2 would suffice as CI robots aren't
> testing w/ that AFAIK.
>
> Nick, do you have anything to add here as we had previously discussed
> this off-list/IRL.

Mostly that we should make -fsanitize=enum not totally suck (i.e.
actually do anything for C code, then check for bad conversions from
values that aren't valid enumeration values including truncations),
then disable this warning in favor of folks testing with that
sanitizer enabled.

>
> [1]: https://github.com/ClangBuiltLinux/linux/issues/1910
> [2]: https://lore.kernel.org/all/?q=b%3Apointer-to-enum-cast



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 436 bytes --]


> There was some discussion [1] wherein it was ultimately decided that
> this warning should probably be turned off (contrary to what the title
> of the GitHub issue says).

I totally agree with your last comment in [1]. So, I also vote for
disabling the warning. Thus, I will reject these patches, but still
thank you for looking into such issues and trying to solve them!

> [1]: https://github.com/ClangBuiltLinux/linux/issues/1910

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] i2c: pxa: fix clang -Wvoid-pointer-to-enum-cast warning

[Andi Shyti]

Hi,

> > There was some discussion [1] wherein it was ultimately decided that
> > this warning should probably be turned off (contrary to what the title
> > of the GitHub issue says).
> 
> I totally agree with your last comment in [1]. So, I also vote for
> disabling the warning. Thus, I will reject these patches, but still
> thank you for looking into such issues and trying to solve them!

yes, unfortunately this is the trend and most of the patches
follow this approach and they are getting merged.

I don't like pointers storing values and this fix whould be
completely taken from another side. In any case, given the trend,
I will not oppose.

Andi