Message ID | 20180914071111.4602-1-angelo@sysam.it (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | [-next,v2] dmaengine: mcf-edma: fix x86_64 allmodconfig compilation warning | expand |
Hi Angelo, Thanks for your patch! On Fri, Sep 14, 2018 at 9:12 AM Angelo Dureghello <angelo@sysam.it> wrote: > This patch fixes the compilation warning reported > during x86_64 allmodconfig build. Please quote the warning, so people know what it is about: drivers/dma/mcf-edma.c: In function 'mcf_edma_filter_fn': drivers/dma/mcf-edma.c:296:33: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] return (mcf_chan->slave_id == (u32)param); > Reported-By: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > --- > Changes for v2: > - added Reported-By > --- > drivers/dma/mcf-edma.c | 3 ++- > include/linux/platform_data/dma-mcf-edma.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c > index 4d30d5302649..e08e2409a102 100644 > --- a/drivers/dma/mcf-edma.c > +++ b/drivers/dma/mcf-edma.c > @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) > { > if (chan->device->dev->driver == &mcf_edma_driver.driver) { > struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan); > + unsigned int req = *(unsigned int *)param; This looks a bit hackish to me. > > - return (mcf_chan->slave_id == (u32)param); The recommended way to cast from pointers to integers is to use uintptr_t, which always has the same size as a pointer, i.e.: return (mcf_chan->slave_id == (uintptr_t)param); > + return (mcf_chan->slave_id == req); > } > > return false; > diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h > index d718ccfa3421..97cb79bda646 100644 > --- a/include/linux/platform_data/dma-mcf-edma.h > +++ b/include/linux/platform_data/dma-mcf-edma.h > @@ -21,7 +21,7 @@ struct dma_slave_map; > > bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); > > -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) > +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) }) > > /** > * struct mcf_edma_platform_data - platform specific data for eDMA engine Gr{oetje,eeting}s, Geert
On 14-09-18, 09:11, Angelo Dureghello wrote: > This patch fixes the compilation warning reported > during x86_64 allmodconfig build. How does it do so, I have no clue what to expect. Please describe the change done here in the log... Patch title also doesn't tell me anything about the fix. > > Reported-By: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > --- > Changes for v2: > - added Reported-By > --- > drivers/dma/mcf-edma.c | 3 ++- > include/linux/platform_data/dma-mcf-edma.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c > index 4d30d5302649..e08e2409a102 100644 > --- a/drivers/dma/mcf-edma.c > +++ b/drivers/dma/mcf-edma.c > @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) > { > if (chan->device->dev->driver == &mcf_edma_driver.driver) { > struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan); > + unsigned int req = *(unsigned int *)param; > > - return (mcf_chan->slave_id == (u32)param); > + return (mcf_chan->slave_id == req); > } > > return false; > diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h > index d718ccfa3421..97cb79bda646 100644 > --- a/include/linux/platform_data/dma-mcf-edma.h > +++ b/include/linux/platform_data/dma-mcf-edma.h > @@ -21,7 +21,7 @@ struct dma_slave_map; > > bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); > > -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) > +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) }) I dont think you answered me about this, why is this change required and in the context of current patch, what does it fix?
Hi Geert, On Fri, Sep 14, 2018 at 01:35:22PM +0200, Geert Uytterhoeven wrote: > Hi Angelo, > > Thanks for your patch! > > On Fri, Sep 14, 2018 at 9:12 AM Angelo Dureghello <angelo@sysam.it> wrote: > > This patch fixes the compilation warning reported > > during x86_64 allmodconfig build. > > Please quote the warning, so people know what it is about: > > drivers/dma/mcf-edma.c: In function 'mcf_edma_filter_fn': > drivers/dma/mcf-edma.c:296:33: warning: cast from pointer to > integer of different size [-Wpointer-to-int-cast] > return (mcf_chan->slave_id == (u32)param); > Ok done. > > Reported-By: Stephen Rothwell <sfr@canb.auug.org.au> > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > > --- > > Changes for v2: > > - added Reported-By > > --- > > drivers/dma/mcf-edma.c | 3 ++- > > include/linux/platform_data/dma-mcf-edma.h | 2 +- > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c > > index 4d30d5302649..e08e2409a102 100644 > > --- a/drivers/dma/mcf-edma.c > > +++ b/drivers/dma/mcf-edma.c > > @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) > > { > > if (chan->device->dev->driver == &mcf_edma_driver.driver) { > > struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan); > > + unsigned int req = *(unsigned int *)param; > > This looks a bit hackish to me. > > > > - return (mcf_chan->slave_id == (u32)param); > > The recommended way to cast from pointers to integers is to > use uintptr_t, which always has the same size as a pointer, i.e.: > slave_id is acually an u32. So, if i understsand, when compiled on x86_64 using uintptr_t, the cast should trigger the same issue. I preferred, as i.e. ti/omap dma driver is doing, to simply use a pointer to an int (int[]) as passed type in param, so i can then indirectly access the integer. I tested proper functionality in ColdFire and that there are no warnings anymore in both ColdFire and x86_64 allmodconfig so the patch, hopefully, should be legal. > return (mcf_chan->slave_id == (uintptr_t)param); > > > + return (mcf_chan->slave_id == req); > > } > > > > return false; > > diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h > > index d718ccfa3421..97cb79bda646 100644 > > --- a/include/linux/platform_data/dma-mcf-edma.h > > +++ b/include/linux/platform_data/dma-mcf-edma.h > > @@ -21,7 +21,7 @@ struct dma_slave_map; > > > > bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); > > > > -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) > > +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) }) > > > > /** > > * struct mcf_edma_platform_data - platform specific data for eDMA engine > > Gr{oetje,eeting}s, > > Geert > Regards, Angelo > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Geert, On Fri, Sep 14, 2018 at 02:13:50PM +0200, Angelo Dureghello wrote: > Hi Geert, > > On Fri, Sep 14, 2018 at 01:35:22PM +0200, Geert Uytterhoeven wrote: > > Hi Angelo, > > > > Thanks for your patch! > > > > On Fri, Sep 14, 2018 at 9:12 AM Angelo Dureghello <angelo@sysam.it> wrote: > > > This patch fixes the compilation warning reported > > > during x86_64 allmodconfig build. > > > > Please quote the warning, so people know what it is about: > > > > drivers/dma/mcf-edma.c: In function 'mcf_edma_filter_fn': > > drivers/dma/mcf-edma.c:296:33: warning: cast from pointer to > > integer of different size [-Wpointer-to-int-cast] > > return (mcf_chan->slave_id == (u32)param); > > > > Ok done. > > > > Reported-By: Stephen Rothwell <sfr@canb.auug.org.au> > > > Signed-off-by: Angelo Dureghello <angelo@sysam.it> > > > --- > > > Changes for v2: > > > - added Reported-By > > > --- > > > drivers/dma/mcf-edma.c | 3 ++- > > > include/linux/platform_data/dma-mcf-edma.h | 2 +- > > > 2 files changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c > > > index 4d30d5302649..e08e2409a102 100644 > > > --- a/drivers/dma/mcf-edma.c > > > +++ b/drivers/dma/mcf-edma.c > > > @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) > > > { > > > if (chan->device->dev->driver == &mcf_edma_driver.driver) { > > > struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan); > > > + unsigned int req = *(unsigned int *)param; > > > > This looks a bit hackish to me. > > > > > > - return (mcf_chan->slave_id == (u32)param); > > > > The recommended way to cast from pointers to integers is to > > use uintptr_t, which always has the same size as a pointer, i.e.: > > > > slave_id is acually an u32. So, if i understsand, when compiled > on x86_64 using uintptr_t, the cast should trigger the same issue. > I preferred, as i.e. ti/omap dma driver is doing, to simply > use a pointer to an int (int[]) as passed type in param, > so i can then indirectly access the integer. I tested > proper functionality in ColdFire and that there are no warnings > anymore in both ColdFire and x86_64 allmodconfig so the patch, > hopefully, should be legal. > I misunderstood uintptr_t, verified now that it works fine for this case, reducing the patch to a single line change. So switching to it. Thanks ! > > return (mcf_chan->slave_id == (uintptr_t)param); > > > > > + return (mcf_chan->slave_id == req); > > > } > > > > > > return false; > > > diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h > > > index d718ccfa3421..97cb79bda646 100644 > > > --- a/include/linux/platform_data/dma-mcf-edma.h > > > +++ b/include/linux/platform_data/dma-mcf-edma.h > > > @@ -21,7 +21,7 @@ struct dma_slave_map; > > > > > > bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); > > > > > > -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) > > > +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) }) > > > > > > /** > > > * struct mcf_edma_platform_data - platform specific data for eDMA engine > > > > Gr{oetje,eeting}s, > > > > Geert > > > Regards, > Angelo > > > -- > > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > > > In personal conversations with technical people, I call myself a hacker. But > > when I'm talking to journalists I just say "programmer" or something like that. > > -- Linus Torvalds Regards, Angelo
diff --git a/drivers/dma/mcf-edma.c b/drivers/dma/mcf-edma.c index 4d30d5302649..e08e2409a102 100644 --- a/drivers/dma/mcf-edma.c +++ b/drivers/dma/mcf-edma.c @@ -292,8 +292,9 @@ bool mcf_edma_filter_fn(struct dma_chan *chan, void *param) { if (chan->device->dev->driver == &mcf_edma_driver.driver) { struct fsl_edma_chan *mcf_chan = to_fsl_edma_chan(chan); + unsigned int req = *(unsigned int *)param; - return (mcf_chan->slave_id == (u32)param); + return (mcf_chan->slave_id == req); } return false; diff --git a/include/linux/platform_data/dma-mcf-edma.h b/include/linux/platform_data/dma-mcf-edma.h index d718ccfa3421..97cb79bda646 100644 --- a/include/linux/platform_data/dma-mcf-edma.h +++ b/include/linux/platform_data/dma-mcf-edma.h @@ -21,7 +21,7 @@ struct dma_slave_map; bool mcf_edma_filter_fn(struct dma_chan *chan, void *param); -#define MCF_EDMA_FILTER_PARAM(ch) ((void *)ch) +#define MCF_EDMA_FILTER_PARAM(ch) ((int[]) { (ch) }) /** * struct mcf_edma_platform_data - platform specific data for eDMA engine
This patch fixes the compilation warning reported during x86_64 allmodconfig build. Reported-By: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Angelo Dureghello <angelo@sysam.it> --- Changes for v2: - added Reported-By --- drivers/dma/mcf-edma.c | 3 ++- include/linux/platform_data/dma-mcf-edma.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-)