On Wed, Apr 29, 2015 at 11:56:58AM +0300, Antony Pavlov wrote:
> Since 2011 barebox' of_device_id struct uses unsigned long type for data field:
>
> struct of_device_id {
> char *compatible;
> unsigned long data;
> };
>
> Almost always struct of_device_id.data field are used as pointer
> and need 'unsigned long' casting.
>
> E.g. see 'git grep -A 4 of_device_id drivers/' output:
>
> drivers/ata/sata-imx.c:static __maybe_unused struct of_device_id imx_sata_dt_ids[] = {
> drivers/ata/sata-imx.c- {
> drivers/ata/sata-imx.c- .compatible = "fsl,imx6q-ahci",
> drivers/ata/sata-imx.c- .data = (unsigned long)&data_imx6,
> drivers/ata/sata-imx.c- }, {
>
> Here is of_device_id struct in linux kernel v4.0:
>
> struct of_device_id {
> char name[32];
> char type[32];
> char compatible[128];
> const void *data;
> };
>
> Changing of_device_id.data type to 'const void *data' will increase
> barebox' linux kernel compatibility and decrease number of 'unsigned
> long' casts.
>
> Part of the patch was done using the 'coccinelle' tool with the
> following semantic patch:
>
> @rule1@
> identifier dev;
> identifier type;
> identifier func;
> @@
> func(...) {
> <...
> - dev_get_drvdata(dev, (unsigned long *)&type)
> + dev_get_drvdata(dev, (const void **)&type)
> ...>
> }
> @rule2@
> identifier dev;
> identifier type;
> identifier func;
> identifier data;
> @@
> func(...) {
> <...
> - dev_get_drvdata(dev, (unsigned long *)&type->data)
> + dev_get_drvdata(dev, (const void **)&type->data)
> ...>
> }
>
> Signed-off-by: Antony Pavlov <antonynpavlov@xxxxxxxxx>
[...]
> diff --git a/arch/arm/mach-imx/clocksource.c b/arch/arm/mach-imx/clocksource.c
> index eba04a3..06d2fba 100644
> --- a/arch/arm/mach-imx/clocksource.c
> +++ b/arch/arm/mach-imx/clocksource.c
> @@ -99,7 +99,7 @@ static int imx_gpt_probe(struct device_d *dev)
> if (timer_base)
> return 0;
>
> - ret = dev_get_drvdata(dev, (unsigned long *)®s);
> + ret = dev_get_drvdata(dev, (const void **)®s);
I applied this to -next to get some compile coverage. The one thing I
don't like about this patch is this explicit cast to const void ** that
is needed. This makes the compiler happy even when the pointer passed in
is not const.
The following may be a way to fix this, but maybe there is a more
elegant way. However, since most of the data passed into dev_get_drvdata
is not const at the moment this requires more patches anyway. So I think
your patch is fine the way it is.
Sascha
-----------------------------8<-----------------------
diff --git a/drivers/base/driver.c b/drivers/base/driver.c
index da020d8..e55aa44 100644
--- a/drivers/base/driver.c
+++ b/drivers/base/driver.c
@@ -404,7 +404,7 @@ void devices_shutdown(void)
}
}
-int dev_get_drvdata(struct device_d *dev, const void **data)
+int __dev_get_drvdata(struct device_d *dev, const void **data)
{
if (dev->of_id_entry) {
*data = dev->of_id_entry->data;
diff --git a/include/driver.h b/include/driver.h
index b8a94e4..f182e01 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -512,7 +512,15 @@ int devfs_create_partitions(const char *devname,
#define DRV_OF_COMPAT(compat) \
IS_ENABLED(CONFIG_OFDEVICE) ? (compat) : NULL
-int dev_get_drvdata(struct device_d *dev, const void **data);
+int __dev_get_drvdata(struct device_d *dev, const void **data);
+
+#define dev_get_drvdata(dev, data) ({ \
+ const void *d; \
+ int ret; \
+ ret = __dev_get_drvdata(dev, &d); \
+ *(data) = d; \
+ ret; \
+})
int device_match_of_modalias(struct device_d *dev, struct driver_d *drv);
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
