Hello all, Linus is right, I should remove all debug prints and keep whit-spacing conventions.
As for the real part of the patch, it is correct that it is only the tmp reuse problem, and I couldn't agree more that it is a bad idea to reuse a variable, or name it tmp. It is also correct that the same problem exists at the other if statements in this function. However, these other places don't need the previous value of tmp, so the problem is masked away. I tried to get the HW specs from the company, but the product was discontinued and they provide no support for it. Therefor I also don’t know if maybe the original code was intentional. So, if any of you could shed some light on the problem, it would be a great help. Any documentation or specs you may have on the PLX NET2280 hardware would be very much appreciated. As for the patch Linus sent- it solves the problem as it does exactly what my patch does, but with much cleaner code. Looking forward to your input, Raz Manor -----Original Message----- From: linus...@gmail.com [mailto:linus...@gmail.com] On Behalf Of Linus Torvalds Sent: Monday, December 26, 2016 1:47 AM To: Raz Manor <raz.ma...@valens.com>; Felipe Balbi <ba...@kernel.org>; Jussi Kivilinna <jussi.kivili...@haltian.com>; Colin Ian King <colin.k...@canonical.com>; Tim Harvey <thar...@gateworks.com>; Heikki Krogerus <heikki.kroge...@linux.intel.com> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>; USB list <linux-usb@vger.kernel.org> Subject: Re: net2280 Driver Bug On Sun, Dec 25, 2016 at 8:09 AM, Raz Manor <raz.ma...@valens.com> 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 N�����r��y����b�X��ǧv�^�){.n�+����{������^n�r���z���h�����&�� �G���h�(�階�ݢj"���m������z�ޖ���f���h���~�m�
