On Sun, Dec 25, 2016 at 8:09 AM, Raz Manor <Raz.Manor@xxxxxxxxxx> wrote:
>
> I had a problem with the net2280 driver- reading from an endpoint resulted
> with a wrong read size in some cases.
>
> I found the problem and fixed it, and I wanted to publish the fix. However,
> I have no push access, and I couldn't find who is the maintainer of the
> file.
>
> Attached is the patch to fix the problem, hopefully you could forward it, or
> connect me with the maintainer.
That patch is really hard to read due to the whitespace changes and
all the debugging code.
The only real change seems to be that you changed the "tmp" use in
reading the scan_dma_completions() to be another variable.
That seems to be because the re-use of "tmp" there corrupts the
previous value of "tmp" that is then later used for "dma_done()", and
you used a different variable for the "readl(&ep->dma->dmacount)".
That makes sense. But I note that the exact same thing seems to happen
in the other if-statement too.
IOW, maybe you meant something like the attached? Does that fix the
problem for you too?
I don't know the hardware. Maybe overwriting "tmp" was intentional.
Regardless, the use of "tmp" as a variable name for something that
clearly is *not* temporary is bad.
Adding the proper people to the recipients list so that they can
hopefully take a more informed look at this.
Linus
drivers/usb/gadget/udc/net2280.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
index 85504419ab31..39bbbb88a48a 100644
--- a/drivers/usb/gadget/udc/net2280.c
+++ b/drivers/usb/gadget/udc/net2280.c
@@ -1163,8 +1163,8 @@ static int scan_dma_completions(struct net2280_ep *ep)
*/
if (unlikely(req->td->dmadesc == 0)) {
/* paranoia */
- tmp = readl(&ep->dma->dmacount);
- if (tmp & DMA_BYTE_COUNT_MASK)
+ u32 ep_dmacount = readl(&ep->dma->dmacount);
+ if (ep_dmacount & DMA_BYTE_COUNT_MASK)
break;
/* single transfer mode */
dma_done(ep, req, tmp, 0);
@@ -1173,24 +1173,24 @@ static int scan_dma_completions(struct net2280_ep *ep)
} else if (!ep->is_in &&
(req->req.length % ep->ep.maxpacket) &&
!(ep->dev->quirks & PLX_PCIE)) {
+ u32 ep_stat = readl(&ep->regs->ep_stat);
- tmp = readl(&ep->regs->ep_stat);
/* AVOID TROUBLE HERE by not issuing short reads from
* your gadget driver. That helps avoids errata 0121,
* 0122, and 0124; not all cases trigger the warning.
*/
- if ((tmp & BIT(NAK_OUT_PACKETS)) == 0) {
+ if ((ep_stat & BIT(NAK_OUT_PACKETS)) == 0) {
ep_warn(ep->dev, "%s lost packet sync!\n",
ep->ep.name);
req->req.status = -EOVERFLOW;
} else {
- tmp = readl(&ep->regs->ep_avail);
- if (tmp) {
+ u32 ep_avail = readl(&ep->regs->ep_avail);
+ if (ep_avail) {
/* fifo gets flushed later */
ep->out_overflow = 1;
ep_dbg(ep->dev,
"%s dma, discard %d len %d\n",
- ep->ep.name, tmp,
+ ep->ep.name, ep_avail,
req->req.length);
req->req.status = -EOVERFLOW;
}
