* [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() [not found] <20200824142118.GA223827@mwanda> @ 2020-08-24 15:19 ` Alex Dewar 2020-08-24 15:57 ` Alex Dewar 0 siblings, 1 reply; 6+ messages in thread From: Alex Dewar @ 2020-08-24 15:19 UTC (permalink / raw) To: dan.carpenter, Greg Kroah-Hartman, Alex Dewar, Javier F. Arias, YueHaibing, Frank A. Cancio Bello, Saiyam Doshi, Magnus Damm, Simon Horman, devel, linux-kernel In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), though, strangely, NULL is passed as the struct device* argument. Pass the UDC's device instead. Build-tested on x86 only. Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> --- So I *think* this is the right fix, but I don't have the hardware so I've only been able to build-test it. My worry is that I could be passing in the wrong struct device* here, which would squelch the warning without fixing the breakage. Can someone cleverer than me tell me if this makes sense? - Alex --- drivers/staging/emxx_udc/emxx_udc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c index 03929b9d3a8b..09e91449c08c 100644 --- a/drivers/staging/emxx_udc/emxx_udc.c +++ b/drivers/staging/emxx_udc/emxx_udc.c @@ -2593,7 +2593,7 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep, if (req->unaligned) { if (!ep->virt_buf) - ep->virt_buf = dma_alloc_coherent(NULL, PAGE_SIZE, + ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE, &ep->phys_buf, GFP_ATOMIC | GFP_DMA); if (ep->epnum > 0) { -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() 2020-08-24 15:19 ` [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() Alex Dewar @ 2020-08-24 15:57 ` Alex Dewar 2020-08-25 7:37 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Alex Dewar @ 2020-08-24 15:57 UTC (permalink / raw) To: Alex Dewar Cc: dan.carpenter, Greg Kroah-Hartman, Javier F. Arias, YueHaibing, Frank A. Cancio Bello, Saiyam Doshi, Magnus Damm, Simon Horman, devel, linux-kernel On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote: > In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), > though, strangely, NULL is passed as the struct device* argument. Pass > the UDC's device instead. > > Build-tested on x86 only. > > Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > --- > > So I *think* this is the right fix, but I don't have the hardware so > I've only been able to build-test it. My worry is that I could be > passing in the wrong struct device* here, which would squelch the > warning without fixing the breakage. > > Can someone cleverer than me tell me if this makes sense? > > - Alex PS -- I meant to put an RFC in the subject line and an extra tag: Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/staging/emxx_udc/emxx_udc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c > index 03929b9d3a8b..09e91449c08c 100644 > --- a/drivers/staging/emxx_udc/emxx_udc.c > +++ b/drivers/staging/emxx_udc/emxx_udc.c > @@ -2593,7 +2593,7 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep, > > if (req->unaligned) { > if (!ep->virt_buf) > - ep->virt_buf = dma_alloc_coherent(NULL, PAGE_SIZE, > + ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE, > &ep->phys_buf, > GFP_ATOMIC | GFP_DMA); > if (ep->epnum > 0) { > -- > 2.28.0 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() 2020-08-24 15:57 ` Alex Dewar @ 2020-08-25 7:37 ` Dan Carpenter 2020-08-25 8:14 ` Alex Dewar 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2020-08-25 7:37 UTC (permalink / raw) To: Alex Dewar Cc: devel, Saiyam Doshi, Magnus Damm, Greg Kroah-Hartman, YueHaibing, linux-kernel, Javier F. Arias, Frank A. Cancio Bello, Simon Horman On Mon, Aug 24, 2020 at 04:57:12PM +0100, Alex Dewar wrote: > On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote: > > In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), > > though, strangely, NULL is passed as the struct device* argument. Pass > > the UDC's device instead. I think passing NULL was always wrong, but it used to not cause an Oops. This was changed a year or two ago. > > > > Build-tested on x86 only. > > > > Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") > > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > > --- > > > > So I *think* this is the right fix, but I don't have the hardware so > > I've only been able to build-test it. My worry is that I could be > > passing in the wrong struct device* here, which would squelch the > > warning without fixing the breakage. > > > > Can someone cleverer than me tell me if this makes sense? > > > > - Alex > > PS -- I meant to put an RFC in the subject line and an extra tag: > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > I don't know which dev pointer we're supposed to pass... It would be good to find someone to test the patch but if not then applying your patch is reasonable. But could you search through the file and update the rest as well. The dma_free_coherent() needs to be updated and there was a second dma_alloc_coherent() in the bug report. regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() 2020-08-25 7:37 ` Dan Carpenter @ 2020-08-25 8:14 ` Alex Dewar 2020-08-25 8:28 ` Dan Carpenter 0 siblings, 1 reply; 6+ messages in thread From: Alex Dewar @ 2020-08-25 8:14 UTC (permalink / raw) To: Dan Carpenter Cc: Alex Dewar, devel, Saiyam Doshi, Magnus Damm, Greg Kroah-Hartman, YueHaibing, linux-kernel, Javier F. Arias, Frank A. Cancio Bello, Simon Horman On Tue, Aug 25, 2020 at 10:37:13AM +0300, Dan Carpenter wrote: > On Mon, Aug 24, 2020 at 04:57:12PM +0100, Alex Dewar wrote: > > On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote: > > > In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), > > > though, strangely, NULL is passed as the struct device* argument. Pass > > > the UDC's device instead. > > I think passing NULL was always wrong, but it used to not cause an Oops. > This was changed a year or two ago. > > > > > > > Build-tested on x86 only. > > > > > > Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") > > > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > > > --- > > > > > > So I *think* this is the right fix, but I don't have the hardware so > > > I've only been able to build-test it. My worry is that I could be > > > passing in the wrong struct device* here, which would squelch the > > > warning without fixing the breakage. > > > > > > Can someone cleverer than me tell me if this makes sense? > > > > > > - Alex > > > > PS -- I meant to put an RFC in the subject line and an extra tag: > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > I don't know which dev pointer we're supposed to pass... It would be > good to find someone to test the patch but if not then applying your > patch is reasonable. > > But could you search through the file and update the rest as well. > The dma_free_coherent() needs to be updated and there was a second > dma_alloc_coherent() in the bug report. > > regards, > dan carpenter > I can only find the one instance of dma_alloc_coherent(). Was it by any chance a different warning about the same call....? ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() 2020-08-25 8:14 ` Alex Dewar @ 2020-08-25 8:28 ` Dan Carpenter 2020-08-25 9:19 ` [PATCH v2] " Alex Dewar 0 siblings, 1 reply; 6+ messages in thread From: Dan Carpenter @ 2020-08-25 8:28 UTC (permalink / raw) To: Alex Dewar Cc: devel, Saiyam Doshi, Magnus Damm, Greg Kroah-Hartman, YueHaibing, linux-kernel, Javier F. Arias, Frank A. Cancio Bello, Simon Horman On Tue, Aug 25, 2020 at 09:14:59AM +0100, Alex Dewar wrote: > On Tue, Aug 25, 2020 at 10:37:13AM +0300, Dan Carpenter wrote: > > On Mon, Aug 24, 2020 at 04:57:12PM +0100, Alex Dewar wrote: > > > On Mon, Aug 24, 2020 at 04:19:17PM +0100, Alex Dewar wrote: > > > > In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), > > > > though, strangely, NULL is passed as the struct device* argument. Pass > > > > the UDC's device instead. > > > > I think passing NULL was always wrong, but it used to not cause an Oops. > > This was changed a year or two ago. > > > > > > > > > > Build-tested on x86 only. > > > > > > > > Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") > > > > Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> > > > > --- > > > > > > > > So I *think* this is the right fix, but I don't have the hardware so > > > > I've only been able to build-test it. My worry is that I could be > > > > passing in the wrong struct device* here, which would squelch the > > > > warning without fixing the breakage. > > > > > > > > Can someone cleverer than me tell me if this makes sense? > > > > > > > > - Alex > > > > > > PS -- I meant to put an RFC in the subject line and an extra tag: > > > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > > > > I don't know which dev pointer we're supposed to pass... It would be > > good to find someone to test the patch but if not then applying your > > patch is reasonable. > > > > But could you search through the file and update the rest as well. > > The dma_free_coherent() needs to be updated and there was a second > > dma_alloc_coherent() in the bug report. > > > > regards, > > dan carpenter > > > > I can only find the one instance of dma_alloc_coherent(). Was it by any > chance a different warning about the same call....? Ah.... It was warning about the dma_free_coherent(). regards, dan carpenter ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() 2020-08-25 8:28 ` Dan Carpenter @ 2020-08-25 9:19 ` Alex Dewar 0 siblings, 0 replies; 6+ messages in thread From: Alex Dewar @ 2020-08-25 9:19 UTC (permalink / raw) To: dan.carpenter Cc: alex.dewar90, damm+renesas, devel, frank, gregkh, horms+renesas, jarias.linux, linux-kernel, saiyamdoshi.in, yuehaibing In nbu2ss_eq_queue() memory is allocated with dma_alloc_coherent(), though, strangely, NULL is passed as the struct device* argument. Pass the UDC's device instead. Fix up the corresponding call to dma_free_coherent() in the same way. Build-tested on x86 only. Fixes: 33aa8d45a4fe ("staging: emxx_udc: Add Emma Mobile USB Gadget driver") Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> --- v2: also fix call to dma_free_coherent() (Dan) @Magnus: I noticed you contributed this code back in 2014... I don't suppose you happen to have the hardware lying around to test this so we can get a Tested-by....? :) --- drivers/staging/emxx_udc/emxx_udc.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c index 03929b9d3a8bc..d0725bc8b48a4 100644 --- a/drivers/staging/emxx_udc/emxx_udc.c +++ b/drivers/staging/emxx_udc/emxx_udc.c @@ -2593,7 +2593,7 @@ static int nbu2ss_ep_queue(struct usb_ep *_ep, if (req->unaligned) { if (!ep->virt_buf) - ep->virt_buf = dma_alloc_coherent(NULL, PAGE_SIZE, + ep->virt_buf = dma_alloc_coherent(udc->dev, PAGE_SIZE, &ep->phys_buf, GFP_ATOMIC | GFP_DMA); if (ep->epnum > 0) { @@ -3148,7 +3148,7 @@ static int nbu2ss_drv_remove(struct platform_device *pdev) for (i = 0; i < NUM_ENDPOINTS; i++) { ep = &udc->ep[i]; if (ep->virt_buf) - dma_free_coherent(NULL, PAGE_SIZE, (void *)ep->virt_buf, + dma_free_coherent(udc->dev, PAGE_SIZE, (void *)ep->virt_buf, ep->phys_buf); } -- 2.28.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-08-25 9:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20200824142118.GA223827@mwanda>
2020-08-24 15:19 ` [PATCH] staging: emxx_udc: Fix passing of NULL to dma_alloc_coherent() Alex Dewar
2020-08-24 15:57 ` Alex Dewar
2020-08-25 7:37 ` Dan Carpenter
2020-08-25 8:14 ` Alex Dewar
2020-08-25 8:28 ` Dan Carpenter
2020-08-25 9:19 ` [PATCH v2] " Alex Dewar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).