Re: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp_type instead of int

Uwe Kleine-König
2024-01-04T09:09:16Z

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

[adding lakml to Cc for wider audience]

On Wed, Jan 03, 2024 at 10:06:03PM +0100, Duje Mihanović wrote:
> On ARM64 platforms, id->data is a 64-bit value and casting it to a
> 32-bit integer causes build errors. Cast it to the corresponding enum
> instead.
>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> This patch is necessary for my Marvell PXA1908 series to compile successfully
> with allyesconfig:
> https://lore.kernel.org/all/20231102-pxa1908-lkml-v7-0-cabb1a0cb52b@skole.hr/
> ---
> drivers/soc/pxa/ssp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/pxa/ssp.c b/drivers/soc/pxa/ssp.c
> index a1e8a07f7275..e2ffd8fd7e13 100644
> --- a/drivers/soc/pxa/ssp.c
> +++ b/drivers/soc/pxa/ssp.c
> @@ -152,11 +152,11 @@ static int pxa_ssp_probe(struct platform_device *pdev)
> if (dev->of_node) {
> const struct of_device_id *id =
> of_match_device(of_match_ptr(pxa_ssp_of_ids), dev);
> - ssp->type = (int) id->data;
> + ssp->type = (enum pxa_ssp_type) id->data;

I wonder if this is a long-term fix. The error that the compiler throws
is:

drivers/soc/pxa/ssp.c:155:29: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
155 | ssp->type = (int) id->data;
| ^

enum pxa_ssp_type is an integer type, too, and its width could be
smaller than 64 bit, too.

The following would also help:

diff --git a/drivers/soc/pxa/ssp.c b/drivers/soc/pxa/ssp.c
index a1e8a07f7275..095d997eb886 100644
--- a/drivers/soc/pxa/ssp.c
+++ b/drivers/soc/pxa/ssp.c
@@ -96,13 +96,13 @@ EXPORT_SYMBOL(pxa_ssp_free);

#ifdef CONFIG_OF
static const struct of_device_id pxa_ssp_of_ids[] = {
- { .compatible = "mrvl,pxa25x-ssp", .data = (void *) PXA25x_SSP },
- { .compatible = "mvrl,pxa25x-nssp", .data = (void *) PXA25x_NSSP },
- { .compatible = "mrvl,pxa27x-ssp", .data = (void *) PXA27x_SSP },
- { .compatible = "mrvl,pxa3xx-ssp", .data = (void *) PXA3xx_SSP },
- { .compatible = "mvrl,pxa168-ssp", .data = (void *) PXA168_SSP },
- { .compatible = "mrvl,pxa910-ssp", .data = (void *) PXA910_SSP },
- { .compatible = "mrvl,ce4100-ssp", .data = (void *) CE4100_SSP },
+ { .compatible = "mrvl,pxa25x-ssp", .driver_data = PXA25x_SSP },
+ { .compatible = "mvrl,pxa25x-nssp", .driver_data = PXA25x_NSSP },
+ { .compatible = "mrvl,pxa27x-ssp", .driver_data = PXA27x_SSP },
+ { .compatible = "mrvl,pxa3xx-ssp", .driver_data = PXA3xx_SSP },
+ { .compatible = "mvrl,pxa168-ssp", .driver_data = PXA168_SSP },
+ { .compatible = "mrvl,pxa910-ssp", .driver_data = PXA910_SSP },
+ { .compatible = "mrvl,ce4100-ssp", .driver_data = CE4100_SSP },
{ },
};
MODULE_DEVICE_TABLE(of, pxa_ssp_of_ids);
@@ -152,7 +152,7 @@ static int pxa_ssp_probe(struct platform_device *pdev)
if (dev->of_node) {
const struct of_device_id *id =
of_match_device(of_match_ptr(pxa_ssp_of_ids), dev);
- ssp->type = (int) id->data;
+ ssp->type = id->driver_data;
} else {
const struct platform_device_id *id =
platform_get_device_id(pdev);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index f458469c5ce5..fbe16089e4bb 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -283,7 +283,10 @@ struct of_device_id {
char name[32];
char type[32];
char compatible[128];
- const void *data;
+ union {
+ const void *data;
+ kernel_ulong_t driver_data;
+ };
};

/* VIO */

For this driver the change would be nice, as several casts can be
dropped.

> } else {
> const struct platform_device_id *id =
> platform_get_device_id(pdev);
> - ssp->type = (int) id->driver_data;
> + ssp->type = (enum pxa_ssp_type) id->driver_data;

This one isn't problematic in my build configuration and you could just
drop the cast completely.