| 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
| ||
|
Message-ID: <ymq7tp3ps4grmmo6o7txpk2sswb2mzxoaplu7lih2xbmbtp6np@b6ycr5wmmbcc>
Date: Thu, 4 Jan 2024 13:02:10 +0100
From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To: Arnd Bergmann <arnd@...db.de>
Cc: Duje Mihanović <duje.mihanovic@...le.hr>,
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
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@...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.
>
> 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
--
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)
Powered by blists - more mailing lists