* [U-Boot] [PATCH] net: asix: Fix AX88772B when used with DriverModel @ 2016-09-06 4:03 Joshua Scott 2016-09-06 19:15 ` Joe Hershberger 2016-09-09 18:57 ` [U-Boot] " Joe Hershberger 0 siblings, 2 replies; 16+ messages in thread From: Joshua Scott @ 2016-09-06 4:03 UTC (permalink / raw) To: u-boot A previous patch (net: asix: fix operation without eeprom) added a two-byte shift to the packet buffer when receiving a packet on the AX88772B. This shift was not included when the driver was updated to work with DriverModel. Testing on a Marvell DB-88F6820-ACM showed that the adapter was not functioning correctly (EHCI timeouts). This patch brings the two-byte shift to the DriverModel implementation of ops->recv (asix_eth_recv). Testing on the same board, we were able to TFTP a file over and confirm that the crc32 was correct. Signed-off-by: Joshua Scott <joshua.scott@alliedtelesis.co.nz> --- drivers/usb/eth/asix.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/usb/eth/asix.c b/drivers/usb/eth/asix.c index ad083cf..a610ae4 100644 --- a/drivers/usb/eth/asix.c +++ b/drivers/usb/eth/asix.c @@ -819,6 +819,11 @@ int asix_eth_recv(struct udevice *dev, int flags, uchar **packetp) } *packetp = ptr + sizeof(packet_len); + + if ((ueth->pusb_dev->descriptor.idVendor == ASIX_USB_VENDOR_ID) && + (ueth->pusb_dev->descriptor.idProduct == AX88772B_USB_PRODUCT_ID)) + *packetp += 2; + return packet_len; err: -- 2.9.0 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [U-Boot] [PATCH] net: asix: Fix AX88772B when used with DriverModel 2016-09-06 4:03 [U-Boot] [PATCH] net: asix: Fix AX88772B when used with DriverModel Joshua Scott @ 2016-09-06 19:15 ` Joe Hershberger 2016-09-09 18:57 ` [U-Boot] " Joe Hershberger 1 sibling, 0 replies; 16+ messages in thread From: Joe Hershberger @ 2016-09-06 19:15 UTC (permalink / raw) To: u-boot On Mon, Sep 5, 2016 at 11:03 PM, Joshua Scott <joshua.scott@alliedtelesis.co.nz> wrote: > A previous patch (net: asix: fix operation without eeprom) added a > two-byte shift to the packet buffer when receiving a packet on the > AX88772B. > > This shift was not included when the driver was updated to work with > DriverModel. Testing on a Marvell DB-88F6820-ACM showed that the adapter > was not functioning correctly (EHCI timeouts). > > This patch brings the two-byte shift to the DriverModel implementation > of ops->recv (asix_eth_recv). > > Testing on the same board, we were able to TFTP a file over and confirm > that the crc32 was correct. > > Signed-off-by: Joshua Scott <joshua.scott@alliedtelesis.co.nz> Acked-by: Joe Hershberger <joe.hershberger@ni.com> ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-06 4:03 [U-Boot] [PATCH] net: asix: Fix AX88772B when used with DriverModel Joshua Scott 2016-09-06 19:15 ` Joe Hershberger @ 2016-09-09 18:57 ` Joe Hershberger 2016-09-09 21:06 ` Marcel Ziswiler 1 sibling, 1 reply; 16+ messages in thread From: Joe Hershberger @ 2016-09-09 18:57 UTC (permalink / raw) To: u-boot Hi Joshua, https://patchwork.ozlabs.org/patch/666191/ was applied to u-boot-net.git. Thanks! -Joe ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-09 18:57 ` [U-Boot] " Joe Hershberger @ 2016-09-09 21:06 ` Marcel Ziswiler 2016-09-09 23:04 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Marcel Ziswiler @ 2016-09-09 21:06 UTC (permalink / raw) To: u-boot On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: > Hi Joshua, > > https://patchwork.ozlabs.org/patch/666191/ was applied to u-boot- > net.git. > > Thanks! > -Joe No, sorry, but this is really the wrong approach! As discussed before rather than Joshua's patch the one from Alban should long since have been applied: https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html I will send a revert ASAP and hope Alban's patch will finally make its way into master to fix this once and for all! ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-09 21:06 ` Marcel Ziswiler @ 2016-09-09 23:04 ` Marek Vasut 2016-09-09 23:13 ` Marcel Ziswiler 0 siblings, 1 reply; 16+ messages in thread From: Marek Vasut @ 2016-09-09 23:04 UTC (permalink / raw) To: u-boot On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: > On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >> Hi Joshua, >> >> https://patchwork.ozlabs.org/patch/666191/ was applied to u-boot- >> net.git. >> >> Thanks! >> -Joe > > No, sorry, but this is really the wrong approach! As discussed before > rather than Joshua's patch the one from Alban should long since have > been applied: > > https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html > > I will send a revert ASAP and hope Alban's patch will finally make its > way into master to fix this once and for all! > Can you, instead of sending a revert, just send a subsequent patch to fix this once and for all ? Thanks for taking care of this mess :) -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-09 23:04 ` Marek Vasut @ 2016-09-09 23:13 ` Marcel Ziswiler 2016-09-09 23:23 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Marcel Ziswiler @ 2016-09-09 23:13 UTC (permalink / raw) To: u-boot On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: > On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: > > > > On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: > > > > > > Hi Joshua, > > > > > > https://patchwork.ozlabs.org/patch/666191/ was applied to u-boot- > > > net.git. > > > > > > Thanks! > > > -Joe > > No, sorry, but this is really the wrong approach! As discussed > > before > > rather than Joshua's patch the one from Alban should long since > > have > > been applied: > > > > https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html > > > > I will send a revert ASAP and hope Alban's patch will finally make > > its > > way into master to fix this once and for all! > > > Can you, instead of sending a revert, just send a subsequent patch to > fix this once and for all ? Sure, I will just squash my revert and Alban's fix together and send that one along ASAP. > Thanks for taking care of this mess :) You are very welcome. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-09 23:13 ` Marcel Ziswiler @ 2016-09-09 23:23 ` Marek Vasut 2016-09-10 0:18 ` Marcel Ziswiler 0 siblings, 1 reply; 16+ messages in thread From: Marek Vasut @ 2016-09-09 23:23 UTC (permalink / raw) To: u-boot On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: > On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>> >>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>> >>>> Hi Joshua, >>>> >>>> https://patchwork.ozlabs.org/patch/666191/ was applied to u-boot- >>>> net.git. >>>> >>>> Thanks! >>>> -Joe >>> No, sorry, but this is really the wrong approach! As discussed >>> before >>> rather than Joshua's patch the one from Alban should long since >>> have >>> been applied: >>> >>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.html >>> >>> I will send a revert ASAP and hope Alban's patch will finally make >>> its >>> way into master to fix this once and for all! >>> >> Can you, instead of sending a revert, just send a subsequent patch to >> fix this once and for all ? > > Sure, I will just squash my revert and Alban's fix together and send > that one along ASAP. Thanks >> Thanks for taking care of this mess :) > > You are very welcome. > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-09 23:23 ` Marek Vasut @ 2016-09-10 0:18 ` Marcel Ziswiler 2016-09-10 1:34 ` Marcel Ziswiler 0 siblings, 1 reply; 16+ messages in thread From: Marcel Ziswiler @ 2016-09-10 0:18 UTC (permalink / raw) To: u-boot On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: > On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: > > > > On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: > > > > > > On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: > > > > > > > > > > > > On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: > > > > > > > > > > > > > > > Hi Joshua, > > > > > > > > > > https://patchwork.ozlabs.org/patch/666191/ was applied to u- > > > > > boot- > > > > > net.git. > > > > > > > > > > Thanks! > > > > > -Joe > > > > No, sorry, but this is really the wrong approach! As discussed > > > > before > > > > rather than Joshua's patch the one from Alban should long since > > > > have > > > > been applied: > > > > > > > > https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.htm > > > > l > > > > > > > > I will send a revert ASAP and hope Alban's patch will finally > > > > make > > > > its > > > > way into master to fix this once and for all! > > > > > > > Can you, instead of sending a revert, just send a subsequent > > > patch to > > > fix this once and for all ? > > Sure, I will just squash my revert and Alban's fix together and > > send > > that one along ASAP. > Thanks Don't thank me too early yet. While it works on Colibri T20 it currently fails on Colibri T30. More network and/or USB brokenness... Currently bisecting... > > > Thanks for taking care of this mess :) > > You are very welcome. How I do love U-Boot. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 0:18 ` Marcel Ziswiler @ 2016-09-10 1:34 ` Marcel Ziswiler 2016-09-10 10:01 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Marcel Ziswiler @ 2016-09-10 1:34 UTC (permalink / raw) To: u-boot On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: > On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: > > > > On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: > > > > > > > > > On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: > > > > > > > > > > > > On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: > > > > > > > > > > > > > > > > > > > > On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: > > > > > > > > > > > > > > > > > > > > > > > > Hi Joshua, > > > > > > > > > > > > https://patchwork.ozlabs.org/patch/666191/ was applied to > > > > > > u- > > > > > > boot- > > > > > > net.git. > > > > > > > > > > > > Thanks! > > > > > > -Joe > > > > > No, sorry, but this is really the wrong approach! As > > > > > discussed > > > > > before > > > > > rather than Joshua's patch the one from Alban should long > > > > > since > > > > > have > > > > > been applied: > > > > > > > > > > https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h > > > > > tm > > > > > l > > > > > > > > > > I will send a revert ASAP and hope Alban's patch will finally > > > > > make > > > > > its > > > > > way into master to fix this once and for all! > > > > > > > > > Can you, instead of sending a revert, just send a subsequent > > > > patch to > > > > fix this once and for all ? > > > Sure, I will just squash my revert and Alban's fix together and > > > send > > > that one along ASAP. > > Thanks > Don't thank me too early yet. While it works on Colibri T20 it > currently fails on Colibri T30. More network and/or USB brokenness... > Currently bisecting... > > > > > > > > > > > > > > Thanks for taking care of this mess :) > > > You are very welcome. > How I do love U-Boot. And the winner is: commit aa7a648747d8c704a9a81c9e493d386930724e9d Author: Joe Hershberger <joe.hershberger@ni.com> Date:???Mon Aug 15 14:42:15 2016 -0500 ????net: Stop including NFS overhead in defragment max ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 1:34 ` Marcel Ziswiler @ 2016-09-10 10:01 ` Marek Vasut 2016-09-10 16:28 ` Joe Hershberger 0 siblings, 1 reply; 16+ messages in thread From: Marek Vasut @ 2016-09-10 10:01 UTC (permalink / raw) To: u-boot On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: > On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>> >>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>> >>>> >>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>> >>>>> >>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>> >>>>>> >>>>>> >>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Hi Joshua, >>>>>>> >>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>> u- >>>>>>> boot- >>>>>>> net.git. >>>>>>> >>>>>>> Thanks! >>>>>>> -Joe >>>>>> No, sorry, but this is really the wrong approach! As >>>>>> discussed >>>>>> before >>>>>> rather than Joshua's patch the one from Alban should long >>>>>> since >>>>>> have >>>>>> been applied: >>>>>> >>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>> tm >>>>>> l >>>>>> >>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>> make >>>>>> its >>>>>> way into master to fix this once and for all! >>>>>> >>>>> Can you, instead of sending a revert, just send a subsequent >>>>> patch to >>>>> fix this once and for all ? >>>> Sure, I will just squash my revert and Alban's fix together and >>>> send >>>> that one along ASAP. >>> Thanks >> Don't thank me too early yet. While it works on Colibri T20 it >> currently fails on Colibri T30. More network and/or USB brokenness... >> Currently bisecting... >> >>> >>>> >>>>> >>>>> Thanks for taking care of this mess :) >>>> You are very welcome. >> How I do love U-Boot. > > And the winner is: > > commit aa7a648747d8c704a9a81c9e493d386930724e9d > Author: Joe Hershberger <joe.hershberger@ni.com> > Date: Mon Aug 15 14:42:15 2016 -0500 > > net: Stop including NFS overhead in defragment max > Uh oh, why is this aforementioned patch even correct ? And why don't we just revert it ? And why didn't anyone notice any problems with it ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 10:01 ` Marek Vasut @ 2016-09-10 16:28 ` Joe Hershberger 2016-09-10 16:51 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Joe Hershberger @ 2016-09-10 16:28 UTC (permalink / raw) To: u-boot Hi Marek, On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: > On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>> >>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>> >>>>> >>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>> >>>>>> >>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Hi Joshua, >>>>>>>> >>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>> u- >>>>>>>> boot- >>>>>>>> net.git. >>>>>>>> >>>>>>>> Thanks! >>>>>>>> -Joe >>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>> discussed >>>>>>> before >>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>> since >>>>>>> have >>>>>>> been applied: >>>>>>> >>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>> tm >>>>>>> l >>>>>>> >>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>> make >>>>>>> its >>>>>>> way into master to fix this once and for all! >>>>>>> >>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>> patch to >>>>>> fix this once and for all ? >>>>> Sure, I will just squash my revert and Alban's fix together and >>>>> send >>>>> that one along ASAP. >>>> Thanks >>> Don't thank me too early yet. While it works on Colibri T20 it >>> currently fails on Colibri T30. More network and/or USB brokenness... >>> Currently bisecting... >>> >>>> >>>>> >>>>>> >>>>>> Thanks for taking care of this mess :) >>>>> You are very welcome. >>> How I do love U-Boot. >> >> And the winner is: >> >> commit aa7a648747d8c704a9a81c9e493d386930724e9d >> Author: Joe Hershberger <joe.hershberger@ni.com> >> Date: Mon Aug 15 14:42:15 2016 -0500 >> >> net: Stop including NFS overhead in defragment max >> > > Uh oh, why is this aforementioned patch even correct ? And why don't we > just revert it ? And why didn't anyone notice any problems with it ? Before that patch, on at least some platforms, lots of memory was being wasted just because of trying to single-source the size of NFS overhead. That patch removed that. The comment from that patch: "If a board needs a specific different defragment size, that board can override this setting." That is the case here. Cheers, -Joe ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 16:28 ` Joe Hershberger @ 2016-09-10 16:51 ` Marek Vasut 2016-09-10 17:24 ` Joe Hershberger 0 siblings, 1 reply; 16+ messages in thread From: Marek Vasut @ 2016-09-10 16:51 UTC (permalink / raw) To: u-boot On 09/10/2016 06:28 PM, Joe Hershberger wrote: > Hi Marek, > > On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: >> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>>> >>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>>> >>>>>> >>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>>> >>>>>>> >>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> Hi Joshua, >>>>>>>>> >>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>>> u- >>>>>>>>> boot- >>>>>>>>> net.git. >>>>>>>>> >>>>>>>>> Thanks! >>>>>>>>> -Joe >>>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>>> discussed >>>>>>>> before >>>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>>> since >>>>>>>> have >>>>>>>> been applied: >>>>>>>> >>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>>> tm >>>>>>>> l >>>>>>>> >>>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>>> make >>>>>>>> its >>>>>>>> way into master to fix this once and for all! >>>>>>>> >>>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>>> patch to >>>>>>> fix this once and for all ? >>>>>> Sure, I will just squash my revert and Alban's fix together and >>>>>> send >>>>>> that one along ASAP. >>>>> Thanks >>>> Don't thank me too early yet. While it works on Colibri T20 it >>>> currently fails on Colibri T30. More network and/or USB brokenness... >>>> Currently bisecting... >>>> >>>>> >>>>>> >>>>>>> >>>>>>> Thanks for taking care of this mess :) >>>>>> You are very welcome. >>>> How I do love U-Boot. >>> >>> And the winner is: >>> >>> commit aa7a648747d8c704a9a81c9e493d386930724e9d >>> Author: Joe Hershberger <joe.hershberger@ni.com> >>> Date: Mon Aug 15 14:42:15 2016 -0500 >>> >>> net: Stop including NFS overhead in defragment max >>> >> >> Uh oh, why is this aforementioned patch even correct ? And why don't we >> just revert it ? And why didn't anyone notice any problems with it ? > > Before that patch, on at least some platforms, lots of memory was > being wasted just because of trying to single-source the size of NFS > overhead. That patch removed that. > > The comment from that patch: "If a board needs a specific different > defragment size, that board can override this setting." > > That is the case here. Can we be sure that this patch will not break other board(s) ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 16:51 ` Marek Vasut @ 2016-09-10 17:24 ` Joe Hershberger 2016-09-10 18:48 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Joe Hershberger @ 2016-09-10 17:24 UTC (permalink / raw) To: u-boot Hi Marek, On Sat, Sep 10, 2016 at 11:51 AM, Marek Vasut <marex@denx.de> wrote: > On 09/10/2016 06:28 PM, Joe Hershberger wrote: >> Hi Marek, >> >> On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: >>> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >>>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>>>> >>>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>>>> >>>>>>> >>>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>>>> >>>>>>>> >>>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hi Joshua, >>>>>>>>>> >>>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>>>> u- >>>>>>>>>> boot- >>>>>>>>>> net.git. >>>>>>>>>> >>>>>>>>>> Thanks! >>>>>>>>>> -Joe >>>>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>>>> discussed >>>>>>>>> before >>>>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>>>> since >>>>>>>>> have >>>>>>>>> been applied: >>>>>>>>> >>>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>>>> tm >>>>>>>>> l >>>>>>>>> >>>>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>>>> make >>>>>>>>> its >>>>>>>>> way into master to fix this once and for all! >>>>>>>>> >>>>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>>>> patch to >>>>>>>> fix this once and for all ? >>>>>>> Sure, I will just squash my revert and Alban's fix together and >>>>>>> send >>>>>>> that one along ASAP. >>>>>> Thanks >>>>> Don't thank me too early yet. While it works on Colibri T20 it >>>>> currently fails on Colibri T30. More network and/or USB brokenness... >>>>> Currently bisecting... >>>>> >>>>>> >>>>>>> >>>>>>>> >>>>>>>> Thanks for taking care of this mess :) >>>>>>> You are very welcome. >>>>> How I do love U-Boot. >>>> >>>> And the winner is: >>>> >>>> commit aa7a648747d8c704a9a81c9e493d386930724e9d >>>> Author: Joe Hershberger <joe.hershberger@ni.com> >>>> Date: Mon Aug 15 14:42:15 2016 -0500 >>>> >>>> net: Stop including NFS overhead in defragment max >>>> >>> >>> Uh oh, why is this aforementioned patch even correct ? And why don't we >>> just revert it ? And why didn't anyone notice any problems with it ? >> >> Before that patch, on at least some platforms, lots of memory was >> being wasted just because of trying to single-source the size of NFS >> overhead. That patch removed that. >> >> The comment from that patch: "If a board needs a specific different >> defragment size, that board can override this setting." >> >> That is the case here. > > Can we be sure that this patch will not break other board(s) ? It will likely affect 2 other boards in the same way... include/configs/apalis_t30.h: 56 #define CONFIG_TFTP_BLOCKSIZE 16384 include/configs/colibri_imx7.h: 49 #define CONFIG_TFTP_BLOCKSIZE 16384 include/configs/colibri_t30.h: 52 #define CONFIG_TFTP_BLOCKSIZE 16384 Cheers, -Joe ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 17:24 ` Joe Hershberger @ 2016-09-10 18:48 ` Marek Vasut 2016-09-10 19:29 ` Joe Hershberger 0 siblings, 1 reply; 16+ messages in thread From: Marek Vasut @ 2016-09-10 18:48 UTC (permalink / raw) To: u-boot On 09/10/2016 07:24 PM, Joe Hershberger wrote: > Hi Marek, > > On Sat, Sep 10, 2016 at 11:51 AM, Marek Vasut <marex@denx.de> wrote: >> On 09/10/2016 06:28 PM, Joe Hershberger wrote: >>> Hi Marek, >>> >>> On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: >>>> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >>>>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>>>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>>>>> >>>>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>>>>> >>>>>>>> >>>>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Hi Joshua, >>>>>>>>>>> >>>>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>>>>> u- >>>>>>>>>>> boot- >>>>>>>>>>> net.git. >>>>>>>>>>> >>>>>>>>>>> Thanks! >>>>>>>>>>> -Joe >>>>>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>>>>> discussed >>>>>>>>>> before >>>>>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>>>>> since >>>>>>>>>> have >>>>>>>>>> been applied: >>>>>>>>>> >>>>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>>>>> tm >>>>>>>>>> l >>>>>>>>>> >>>>>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>>>>> make >>>>>>>>>> its >>>>>>>>>> way into master to fix this once and for all! >>>>>>>>>> >>>>>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>>>>> patch to >>>>>>>>> fix this once and for all ? >>>>>>>> Sure, I will just squash my revert and Alban's fix together and >>>>>>>> send >>>>>>>> that one along ASAP. >>>>>>> Thanks >>>>>> Don't thank me too early yet. While it works on Colibri T20 it >>>>>> currently fails on Colibri T30. More network and/or USB brokenness... >>>>>> Currently bisecting... >>>>>> >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks for taking care of this mess :) >>>>>>>> You are very welcome. >>>>>> How I do love U-Boot. >>>>> >>>>> And the winner is: >>>>> >>>>> commit aa7a648747d8c704a9a81c9e493d386930724e9d >>>>> Author: Joe Hershberger <joe.hershberger@ni.com> >>>>> Date: Mon Aug 15 14:42:15 2016 -0500 >>>>> >>>>> net: Stop including NFS overhead in defragment max >>>>> >>>> >>>> Uh oh, why is this aforementioned patch even correct ? And why don't we >>>> just revert it ? And why didn't anyone notice any problems with it ? >>> >>> Before that patch, on at least some platforms, lots of memory was >>> being wasted just because of trying to single-source the size of NFS >>> overhead. That patch removed that. >>> >>> The comment from that patch: "If a board needs a specific different >>> defragment size, that board can override this setting." >>> >>> That is the case here. >> >> Can we be sure that this patch will not break other board(s) ? > > It will likely affect 2 other boards in the same way... > > include/configs/apalis_t30.h: 56 #define CONFIG_TFTP_BLOCKSIZE 16384 > include/configs/colibri_imx7.h: 49 #define CONFIG_TFTP_BLOCKSIZE 16384 > include/configs/colibri_t30.h: 52 #define CONFIG_TFTP_BLOCKSIZE 16384 I _think_ you're mixing IP_PKTSIZE and CONFIG_TFTP_BLOCKSIZE (I might be wrong, I'm no network stack expert). My biggest concern about the aa7a648747d8c704a9a81c9e493d386930724e9d patch is that it might cause silent memory corruption on a lot of systems. Are you positive this is not the case, ever ? -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 18:48 ` Marek Vasut @ 2016-09-10 19:29 ` Joe Hershberger 2016-09-11 21:30 ` Marek Vasut 0 siblings, 1 reply; 16+ messages in thread From: Joe Hershberger @ 2016-09-10 19:29 UTC (permalink / raw) To: u-boot On Sat, Sep 10, 2016 at 1:48 PM, Marek Vasut <marex@denx.de> wrote: > On 09/10/2016 07:24 PM, Joe Hershberger wrote: >> Hi Marek, >> >> On Sat, Sep 10, 2016 at 11:51 AM, Marek Vasut <marex@denx.de> wrote: >>> On 09/10/2016 06:28 PM, Joe Hershberger wrote: >>>> Hi Marek, >>>> >>>> On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: >>>>> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >>>>>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>>>>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>>>>>> >>>>>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Hi Joshua, >>>>>>>>>>>> >>>>>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>>>>>> u- >>>>>>>>>>>> boot- >>>>>>>>>>>> net.git. >>>>>>>>>>>> >>>>>>>>>>>> Thanks! >>>>>>>>>>>> -Joe >>>>>>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>>>>>> discussed >>>>>>>>>>> before >>>>>>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>>>>>> since >>>>>>>>>>> have >>>>>>>>>>> been applied: >>>>>>>>>>> >>>>>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>>>>>> tm >>>>>>>>>>> l >>>>>>>>>>> >>>>>>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>>>>>> make >>>>>>>>>>> its >>>>>>>>>>> way into master to fix this once and for all! >>>>>>>>>>> >>>>>>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>>>>>> patch to >>>>>>>>>> fix this once and for all ? >>>>>>>>> Sure, I will just squash my revert and Alban's fix together and >>>>>>>>> send >>>>>>>>> that one along ASAP. >>>>>>>> Thanks >>>>>>> Don't thank me too early yet. While it works on Colibri T20 it >>>>>>> currently fails on Colibri T30. More network and/or USB brokenness... >>>>>>> Currently bisecting... >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thanks for taking care of this mess :) >>>>>>>>> You are very welcome. >>>>>>> How I do love U-Boot. >>>>>> >>>>>> And the winner is: >>>>>> >>>>>> commit aa7a648747d8c704a9a81c9e493d386930724e9d >>>>>> Author: Joe Hershberger <joe.hershberger@ni.com> >>>>>> Date: Mon Aug 15 14:42:15 2016 -0500 >>>>>> >>>>>> net: Stop including NFS overhead in defragment max >>>>>> >>>>> >>>>> Uh oh, why is this aforementioned patch even correct ? And why don't we >>>>> just revert it ? And why didn't anyone notice any problems with it ? >>>> >>>> Before that patch, on at least some platforms, lots of memory was >>>> being wasted just because of trying to single-source the size of NFS >>>> overhead. That patch removed that. >>>> >>>> The comment from that patch: "If a board needs a specific different >>>> defragment size, that board can override this setting." >>>> >>>> That is the case here. >>> >>> Can we be sure that this patch will not break other board(s) ? >> >> It will likely affect 2 other boards in the same way... >> >> include/configs/apalis_t30.h: 56 #define CONFIG_TFTP_BLOCKSIZE 16384 >> include/configs/colibri_imx7.h: 49 #define CONFIG_TFTP_BLOCKSIZE 16384 >> include/configs/colibri_t30.h: 52 #define CONFIG_TFTP_BLOCKSIZE 16384 > > I _think_ you're mixing IP_PKTSIZE and CONFIG_TFTP_BLOCKSIZE (I might be > wrong, I'm no network stack expert). The difference between these two is 4 bytes. It's the fact that the eth + ip + udp + tftp headers + block size (data payload) for the TFTP packets is bigger than the max defrag (i.e. the buffer size for reconstructing the packet) as a result of this patch. There's nothing magic about this relationship, it's just implied and not explicit since it's possible to be some other network transfer besides TFTP and NFS, but those are the two I'm aware of that care about defrag. > My biggest concern about the > aa7a648747d8c704a9a81c9e493d386930724e9d patch is that it might cause > silent memory corruption on a lot of systems. Are you positive this > is not the case, ever ? Not on its own. The change is a reduction in default memory used for defrag. If a board was expecting it to be larger then it will have an issue (like we see on the t30). For TFTP, grep can tell us the 3 boards that are expecting more than they will get by default. The only board that overrides the default NFS read size is: include/configs/tqma6.h: 107 #define CONFIG_NFS_READ_SIZE 4096 That board is expecting only 4k, where as the default for IP_DEFRAG is 16k. That board will be fine. There are only a handful that even specify IP_DEFRAG: include/configs/apalis_t30.h: 54 #define CONFIG_IP_DEFRAG include/configs/bfin_adi_common.h: 241 # define CONFIG_IP_DEFRAG include/configs/colibri_imx7.h: 48 #define CONFIG_IP_DEFRAG include/configs/colibri_t20.h: 44 #define CONFIG_IP_DEFRAG include/configs/colibri_t30.h: 50 #define CONFIG_IP_DEFRAG include/configs/ma5d4evk.h: 71 #define CONFIG_IP_DEFRAG include/configs/sandbox.h: 128 #define CONFIG_IP_DEFRAG include/configs/tqma6.h: 105 #define CONFIG_IP_DEFRAG I think if we also adjust the apalis_t30 and the colibri_imx7 then we are good. Cheers, -Joe ^ permalink raw reply [flat|nested] 16+ messages in thread
* [U-Boot] net: asix: Fix AX88772B when used with DriverModel 2016-09-10 19:29 ` Joe Hershberger @ 2016-09-11 21:30 ` Marek Vasut 0 siblings, 0 replies; 16+ messages in thread From: Marek Vasut @ 2016-09-11 21:30 UTC (permalink / raw) To: u-boot On 09/10/2016 09:29 PM, Joe Hershberger wrote: > On Sat, Sep 10, 2016 at 1:48 PM, Marek Vasut <marex@denx.de> wrote: >> On 09/10/2016 07:24 PM, Joe Hershberger wrote: >>> Hi Marek, >>> >>> On Sat, Sep 10, 2016 at 11:51 AM, Marek Vasut <marex@denx.de> wrote: >>>> On 09/10/2016 06:28 PM, Joe Hershberger wrote: >>>>> Hi Marek, >>>>> >>>>> On Sat, Sep 10, 2016 at 5:01 AM, Marek Vasut <marex@denx.de> wrote: >>>>>> On 09/10/2016 03:34 AM, Marcel Ziswiler wrote: >>>>>>> On Sat, 2016-09-10 at 02:18 +0200, Marcel Ziswiler wrote: >>>>>>>> On Sat, 2016-09-10 at 01:23 +0200, Marek Vasut wrote: >>>>>>>>> >>>>>>>>> On 09/10/2016 01:13 AM, Marcel Ziswiler wrote: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> On Sat, 2016-09-10 at 01:04 +0200, Marek Vasut wrote: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> On 09/09/2016 11:06 PM, Marcel Ziswiler wrote: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> On Fri, 2016-09-09 at 13:57 -0500, Joe Hershberger wrote: >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Hi Joshua, >>>>>>>>>>>>> >>>>>>>>>>>>> https://patchwork.ozlabs.org/patch/666191/ was applied to >>>>>>>>>>>>> u- >>>>>>>>>>>>> boot- >>>>>>>>>>>>> net.git. >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks! >>>>>>>>>>>>> -Joe >>>>>>>>>>>> No, sorry, but this is really the wrong approach! As >>>>>>>>>>>> discussed >>>>>>>>>>>> before >>>>>>>>>>>> rather than Joshua's patch the one from Alban should long >>>>>>>>>>>> since >>>>>>>>>>>> have >>>>>>>>>>>> been applied: >>>>>>>>>>>> >>>>>>>>>>>> https://www.mail-archive.com/u-boot at lists.denx.de/msg221455.h >>>>>>>>>>>> tm >>>>>>>>>>>> l >>>>>>>>>>>> >>>>>>>>>>>> I will send a revert ASAP and hope Alban's patch will finally >>>>>>>>>>>> make >>>>>>>>>>>> its >>>>>>>>>>>> way into master to fix this once and for all! >>>>>>>>>>>> >>>>>>>>>>> Can you, instead of sending a revert, just send a subsequent >>>>>>>>>>> patch to >>>>>>>>>>> fix this once and for all ? >>>>>>>>>> Sure, I will just squash my revert and Alban's fix together and >>>>>>>>>> send >>>>>>>>>> that one along ASAP. >>>>>>>>> Thanks >>>>>>>> Don't thank me too early yet. While it works on Colibri T20 it >>>>>>>> currently fails on Colibri T30. More network and/or USB brokenness... >>>>>>>> Currently bisecting... >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Thanks for taking care of this mess :) >>>>>>>>>> You are very welcome. >>>>>>>> How I do love U-Boot. >>>>>>> >>>>>>> And the winner is: >>>>>>> >>>>>>> commit aa7a648747d8c704a9a81c9e493d386930724e9d >>>>>>> Author: Joe Hershberger <joe.hershberger@ni.com> >>>>>>> Date: Mon Aug 15 14:42:15 2016 -0500 >>>>>>> >>>>>>> net: Stop including NFS overhead in defragment max >>>>>>> >>>>>> >>>>>> Uh oh, why is this aforementioned patch even correct ? And why don't we >>>>>> just revert it ? And why didn't anyone notice any problems with it ? >>>>> >>>>> Before that patch, on at least some platforms, lots of memory was >>>>> being wasted just because of trying to single-source the size of NFS >>>>> overhead. That patch removed that. >>>>> >>>>> The comment from that patch: "If a board needs a specific different >>>>> defragment size, that board can override this setting." >>>>> >>>>> That is the case here. >>>> >>>> Can we be sure that this patch will not break other board(s) ? >>> >>> It will likely affect 2 other boards in the same way... >>> >>> include/configs/apalis_t30.h: 56 #define CONFIG_TFTP_BLOCKSIZE 16384 >>> include/configs/colibri_imx7.h: 49 #define CONFIG_TFTP_BLOCKSIZE 16384 >>> include/configs/colibri_t30.h: 52 #define CONFIG_TFTP_BLOCKSIZE 16384 >> >> I _think_ you're mixing IP_PKTSIZE and CONFIG_TFTP_BLOCKSIZE (I might be >> wrong, I'm no network stack expert). > > The difference between these two is 4 bytes. > > It's the fact that the eth + ip + udp + tftp headers + block size > (data payload) for the TFTP packets is bigger than the max defrag > (i.e. the buffer size for reconstructing the packet) as a result of > this patch. And that is OK ? > There's nothing magic about this relationship, it's just > implied and not explicit since it's possible to be some other network > transfer besides TFTP and NFS, but those are the two I'm aware of that > care about defrag. I don't understand this part, sorry. >> My biggest concern about the >> aa7a648747d8c704a9a81c9e493d386930724e9d patch is that it might cause >> silent memory corruption on a lot of systems. Are you positive this >> is not the case, ever ? > > Not on its own. The change is a reduction in default memory used for > defrag. If a board was expecting it to be larger then it will have an > issue (like we see on the t30). For TFTP, grep can tell us the 3 > boards that are expecting more than they will get by default. I see. Shouldn't we check for such condition and error out then ? > The only board that overrides the default NFS read size is: > > include/configs/tqma6.h: 107 #define CONFIG_NFS_READ_SIZE 4096 > > That board is expecting only 4k, where as the default for IP_DEFRAG is > 16k. That board will be fine. > > There are only a handful that even specify IP_DEFRAG: > > include/configs/apalis_t30.h: 54 #define CONFIG_IP_DEFRAG > include/configs/bfin_adi_common.h: 241 # define CONFIG_IP_DEFRAG > include/configs/colibri_imx7.h: 48 #define CONFIG_IP_DEFRAG > include/configs/colibri_t20.h: 44 #define CONFIG_IP_DEFRAG > include/configs/colibri_t30.h: 50 #define CONFIG_IP_DEFRAG > include/configs/ma5d4evk.h: 71 #define CONFIG_IP_DEFRAG > include/configs/sandbox.h: 128 #define CONFIG_IP_DEFRAG > include/configs/tqma6.h: 105 #define CONFIG_IP_DEFRAG > > I think if we also adjust the apalis_t30 and the colibri_imx7 then we are good. > > Cheers, > -Joe > -- Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2016-09-11 21:30 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-09-06 4:03 [U-Boot] [PATCH] net: asix: Fix AX88772B when used with DriverModel Joshua Scott 2016-09-06 19:15 ` Joe Hershberger 2016-09-09 18:57 ` [U-Boot] " Joe Hershberger 2016-09-09 21:06 ` Marcel Ziswiler 2016-09-09 23:04 ` Marek Vasut 2016-09-09 23:13 ` Marcel Ziswiler 2016-09-09 23:23 ` Marek Vasut 2016-09-10 0:18 ` Marcel Ziswiler 2016-09-10 1:34 ` Marcel Ziswiler 2016-09-10 10:01 ` Marek Vasut 2016-09-10 16:28 ` Joe Hershberger 2016-09-10 16:51 ` Marek Vasut 2016-09-10 17:24 ` Joe Hershberger 2016-09-10 18:48 ` Marek Vasut 2016-09-10 19:29 ` Joe Hershberger 2016-09-11 21:30 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox