Message ID | 20240103210604.16877-1-duje.mihanovic@skole.hr |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7301:6f82:b0:100:9c79:88ff with SMTP id tb2csp5268970dyb; Wed, 3 Jan 2024 13:15:23 -0800 (PST) X-Google-Smtp-Source: AGHT+IGR6Jrs+N14X0s+Pd8AeH+UnpUk4t30pTSaLSCcN8lRifEU4SVDKmhfdr0Wuh0gXnZhGFRv X-Received: by 2002:a05:6808:e8d:b0:3bc:1346:9c38 with SMTP id k13-20020a0568080e8d00b003bc13469c38mr3876336oil.94.1704316523457; Wed, 03 Jan 2024 13:15:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1704316523; cv=none; d=google.com; s=arc-20160816; b=IihCwf/+zDqWC4H+yLUg5+9FZt7kDrHZwqtMt1LD30gIaP6sGkggv3wGi004OvF3Mt j7gJhedfdBt5bUfHH7yx/2J7+nvjC53E8XG0CijH2JVDYSYS7ZI8fnR8DAZ122cNAu1o ou7a20ryFli5O1jRcnasHGsnK5eBNz4anvhtqVc/C5QPEk3bY81lHhCSBNIPfala7ylZ J7PTX7UQGym6G2kdiXDyV4kE4irIptb7XeTRVQj51PFYrFQtI6PA+oXE0oNdoALluVIa 4YFFWKhBsbEqa08MRETfPPT0mKAcoDHlfuO0IpALXn298zkGot9pYdmotOn+idRQ6BmV V1Mw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from; bh=G4AM82oZYIXq2htDHH8TV1zOK+hr62MWik7xdTn6Olk=; fh=x3hY/hasug7bFu3R6sJrbhJoXBu9uvntF68lTXY4A4o=; b=XT4dB1/jK4M0/Ah4nzPV+UcZtdaohggg8UfQ1gas3FfWzWAUHh2j05SO4BkLN+Z4iL qFPODiUR/ciVbevyV+w7fEgBvzeOA5FDxRnlS0mNIksqJhQnzy80QzuAASGn/uxSmoEj tKHNekCC/Ey+imo4P8QK5/nVtSpc8Q0fWz42b23sZyDm0IFhOpjZpgsiHYFjVBOq6W/q 5hIWi1e9QgyjgF2xQdAHXY7m2ik/XlEuuf+3ifAsIcgCTt8ySD3hrC2ony0z32GCPKps erMi0COKBmpY7zvQ+MumvNafbyGoUVepEvSKEsIjWluk1VxHyxfnKj1ko01c/mUOJozo SWBQ== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org" Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [147.75.199.223]) by mx.google.com with ESMTPS id m9-20020a0cf189000000b0067f6f3aeff7si31438545qvl.0.2024.01.03.13.15.23 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 03 Jan 2024 13:15:23 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) smtp.mailfrom="linux-kernel+bounces-16011-ouuuleilei=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ny.mirrors.kernel.org (Postfix) with ESMTPS id 4044B1C24671 for <ouuuleilei@gmail.com>; Wed, 3 Jan 2024 21:15:23 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id B33031D6BC; Wed, 3 Jan 2024 21:15:11 +0000 (UTC) X-Original-To: linux-kernel@vger.kernel.org Received: from mx.skole.hr (mx1.hosting.skole.hr [161.53.165.185]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5A8031D681 for <linux-kernel@vger.kernel.org>; Wed, 3 Jan 2024 21:15:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=skole.hr Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=skole.hr Received: from mx1.hosting.skole.hr (localhost.localdomain [127.0.0.1]) by mx.skole.hr (mx.skole.hr) with ESMTP id 88FFB83D74; Wed, 3 Jan 2024 22:08:57 +0100 (CET) From: =?utf-8?q?Duje_Mihanovi=C4=87?= <duje.mihanovic@skole.hr> To: Arnd Bergmann <arnd@arndb.de>, Robert Jarzmik <robert.jarzmik@free.fr> Cc: =?utf-8?q?Duje_Mihanovi=C4=87?= <duje.mihanovic@skole.hr>, Lubomir Rintel <lkundrak@v3.sk>, =?utf-8?q?Uwe_Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>, zhang songyi <zhang.songyi@zte.com.cn>, soc@kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH RFC RESEND] soc: pxa: ssp: Cast to enum pxa_ssp_type instead of int Date: Wed, 3 Jan 2024 22:06:03 +0100 Message-ID: <20240103210604.16877-1-duje.mihanovic@skole.hr> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1787105402444321596 X-GMAIL-MSGID: 1787105402444321596 |
Series |
[RFC,RESEND] soc: pxa: ssp: Cast to enum pxa_ssp_type instead of int
|
|
Commit Message
Duje Mihanović
Jan. 3, 2024, 9:06 p.m. UTC
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(-)
Comments
[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. Best regards Uwe
On Thu, Jan 4, 2024, at 10:08, Uwe Kleine-König wrote: > [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. I would just change the cast to (uintptr_t), which is what we have elsewhere. > 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. I think this is a nice idea in general, but I would keep it separate from the bugfix, as we might want to do this on a wider scale, or run into problems. In particular, removing tons of casts to (kernel_ulong_t) in other subsystems is probably more valuable than removing casts to (void *) for of_device_id, since kernel_ulong_t is particularly confusing, with a definition that is completely unrelated to the similarly named __kernel_ulong_t. Arnd
Hello Arnd, On Thu, Jan 04, 2024 at 11:03:41AM +0100, Arnd Bergmann wrote: > On Thu, Jan 4, 2024, at 10:08, Uwe Kleine-König wrote: > > [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. > > I would just change the cast to (uintptr_t), which is what we > have elsewhere. > > > 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. > > I think this is a nice idea in general, but I would keep > it separate from the bugfix, as we might want to do this on > a wider scale, or run into problems. Sure, I didn't intend to put the diff into a commit as is. Before doing that change it would probably be sensible to check how this field is used, I guess most drivers use an integer value and not a pointer there. Also while touching that making the same change with the same names for the other *_id structs might be nice. Currently we have (from include/linux/mod_devicetable.h): pci_device_id kernel_ulong_t driver_data ieee1394_device_id kernel_ulong_t driver_data usb_device_id kernel_ulong_t driver_info hid_device_id kernel_ulong_t driver_data ccw_device_id kernel_ulong_t driver_info ap_device_id kernel_ulong_t driver_info css_device_id kernel_ulong_t driver_data acpi_device_id kernel_ulong_t driver_data pnp_device_id kernel_ulong_t driver_data pnp_card_device_id kernel_ulong_t driver_data serio_device_id - hda_device_id unsigned long driver_data sdw_device_id kernel_ulong_t driver_data of_device_id const void *data vio_device_id - pcmcia_device_id kernel_ulong_t driver_info input_device_id kernel_ulong_t driver_info eisa_device_id kernel_ulong_t driver_data parisc_device_id - sdio_device_id kernel_ulong_t driver_data ssb_device_id - bcma_device_id - virtio_device_id - hv_vmbus_device_id kernel_ulong_t driver_data rpmsg_device_id kernel_ulong_t driver_data i2c_device_id kernel_ulong_t driver_data pci_epf_device_id kernel_ulong_t driver_data i3c_device_id const void *data spi_device_id kernel_ulong_t driver_data slim_device_id - apr_device_id kernel_ulong_t driver_data spmi_device_id kernel_ulong_t driver_data dmi_system_id void *driver_data platform_device_id kernel_ulong_t driver_data mdio_device_id - zorro_device_id kernel_ulong_t driver_data isapnp_device_id kernel_ulong_t driver_data amba_id void *data mips_cdmm_device_id - x86_cpu_id kernel_ulong_t driver_data ipack_device_id - mei_cl_device_id kernel_ulong_t driver_info rio_device_id - mcb_device_id kernel_ulong_t driver_data ulpi_device_id kernel_ulong_t driver_data fsl_mc_device_id - tb_service_id kernel_ulong_t driver_data typec_device_id kernel_ulong_t driver_data tee_client_device_id - wmi_device_id const void *context mhi_device_id kernel_ulong_t driver_data auxiliary_device_id kernel_ulong_t driver_data ssam_device_id kernel_ulong_t driver_data dfl_device_id kernel_ulong_t driver_data ishtp_device_id kernel_ulong_t driver_data cdx_device_id - vchiq_device_id - Note driver_data vs driver_info which is probably not worth to unify. > In particular, removing tons of casts to (kernel_ulong_t) > in other subsystems is probably more valuable than removing > casts to (void *) for of_device_id, since kernel_ulong_t > is particularly confusing, with a definition that is > completely unrelated to the similarly named __kernel_ulong_t. I'll add that to my list of ideas for big projects :-) Best regards Uwe
Hello Uwe and Arnd, for v2 I will change the first cast to (uintptr_t) as Arnd suggested and drop the second cast completely as Uwe suggested. As a long-term solution (at least for this particular driver), the few PXA platforms and drivers still depending on or using this driver at first seem trivial to convert to pxa2xx-spi, while the navpoint driver seemingly could be removed entirely as all boards using it have been removed. If there are no objections I am willing to do this conversion. -- Regards, Duje
Hello Duje, On Thu, Jan 04, 2024 at 03:23:09PM +0100, Duje Mihanović wrote: > for v2 I will change the first cast to (uintptr_t) as Arnd suggested and drop > the second cast completely as Uwe suggested. > > As a long-term solution (at least for this particular driver), the few PXA > platforms and drivers still depending on or using this driver at first seem > trivial to convert to pxa2xx-spi, while the navpoint driver seemingly could be > removed entirely as all boards using it have been removed. If there are no > objections I am willing to do this conversion. In case you need encouragement: Sounds like a nice plan; no objections from my side. Best regards Uwe
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; } 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; /* PXA2xx/3xx SSP ports starts from 1 and the internal pdev->id * starts from 0, do a translation here