| Message ID | 20230809-cbl-1903-v1-1-df9d66a3ba3e@google.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | i2c: fix -Wvoid-pointer-to-enum-cast warnings | expand |
Hi Justin, > @@ -1036,7 +1036,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); > + (unsigned long) of_device_get_match_data(&pdev->dev); I think this should be uintptr_t, as defined in types.h: typedef unsigned long uintptr_t; (I'm a bit puzzled to see a void *data cast to a 0/1 value.) Andi
Hi Andi, On Thu, Aug 10, 2023 at 10:56:32AM +0200, Andi Shyti wrote: > Hi Justin, > > > @@ -1036,7 +1036,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); > > + (unsigned long) of_device_get_match_data(&pdev->dev); > > I think this should be uintptr_t, as defined in types.h: > > typedef unsigned long uintptr_t; Yes, that is a valid comment, although I will say that I have seen some developers/maintainers prefer 'unsigned long' over 'uintptr_t', so it is really a crapshoot which one to pick :) > (I'm a bit puzzled to see a void *data cast to a 0/1 value.) It is an odd pattern but as far as I understand it, the kernel uses it a lot to pass platform specific data around (just grepping for '.data = (void *)' shows a lot of instances. Cheers, Nathan
diff --git a/drivers/i2c/busses/i2c-bcm-iproc.c b/drivers/i2c/busses/i2c-bcm-iproc.c index 2d8342fdc25d..3464f3a376a5 100644 --- a/drivers/i2c/busses/i2c-bcm-iproc.c +++ b/drivers/i2c/busses/i2c-bcm-iproc.c @@ -1036,7 +1036,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); + (unsigned long) of_device_get_match_data(&pdev->dev); init_completion(&iproc_i2c->done); res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
When building with W=1 we see the following warning: | drivers/i2c/busses/i2c-bcm-iproc.c:1039:3: error: cast to smaller \ | integer type 'enum bcm_iproc_i2c_type' from 'const void *' [-Werror,-Wvoid-pointer-to-enum-cast] | 1039 | (enum bcm_iproc_i2c_type)of_device_get_match_data(&pdev->dev); | | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ This is due to the fact that the `bcm_iproc_i2c_type` enum members are int-width and a cast from pointer-width down to int will cause truncation and possible data loss. Although in this case `bcm_iproc_i2c_type` has only a few enumerated fields and thus there is likely no data loss occurring. Nonetheless, this patch is necessary to the goal of promoting this warning out of W=1. Link: https://github.com/ClangBuiltLinux/linux/issues/1903 Signed-off-by: Justin Stitt <justinstitt@google.com> --- drivers/i2c/busses/i2c-bcm-iproc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)