linux-kernel - Re: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rj7ijuyy47jrffi6sk7wikqo3rnutz2swkdrznyegalylacawz@jdncegf3elct>
Date: Thu, 4 Jan 2024 10:08:58 +0100
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Duje Mihanović <duje.mihanovic@...le.hr>
Cc: Arnd Bergmann <arnd@...db.de>, Robert Jarzmik <robert.jarzmik@...e.fr>, 
	Lubomir Rintel <lkundrak@...sk>, zhang songyi <zhang.songyi@....com.cn>, soc@...nel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp_type
 instead of int

[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@...le.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.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)