| Message ID | 20201205191744.7847-1-rasmus.villemoes@prevas.dk (mailing list archive) |
|---|---|
| Headers | show |
| Series | ethernet: ucc_geth: assorted fixes and simplifications | expand |
On Sat, 5 Dec 2020 20:17:23 +0100 Rasmus Villemoes wrote: > While trying to figure out how to allow bumping the MTU with the > ucc_geth driver, I fell into a rabbit hole and stumbled on a whole > bunch of issues of varying importance - some are outright bug fixes, > while most are a matter of simplifying the code to make it more > accessible. > > At the end of digging around the code and data sheet to figure out how > it all works, I think the MTU issue might be fixed by a one-liner, but > I'm not sure it can be that simple. It does seem to work (ping -s X > works for larger values of X, and wireshark confirms that the packets > are not fragmented). > > Re patch 2, someone in NXP should check how the hardware actually > works and make an updated reference manual available. Looks like a nice clean up on a quick look. Please separate patches 1 and 11 (which are the two bug fixes I see) rebase (retest) and post them against the net tree: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ so they are available for backports. The reset should go into net-next: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/ Please indicate the tree in the tag like [PATCH net] or [PATCH net-next] so the test bots know which base to use for testing. Thanks!
On 05/12/2020 21.53, Jakub Kicinski wrote: > On Sat, 5 Dec 2020 20:17:23 +0100 Rasmus Villemoes wrote: >> While trying to figure out how to allow bumping the MTU with the >> ucc_geth driver, I fell into a rabbit hole and stumbled on a whole >> bunch of issues of varying importance - some are outright bug fixes, >> while most are a matter of simplifying the code to make it more >> accessible. >> >> At the end of digging around the code and data sheet to figure out how >> it all works, I think the MTU issue might be fixed by a one-liner, but >> I'm not sure it can be that simple. It does seem to work (ping -s X >> works for larger values of X, and wireshark confirms that the packets >> are not fragmented). >> >> Re patch 2, someone in NXP should check how the hardware actually >> works and make an updated reference manual available. > > Looks like a nice clean up on a quick look. > > Please separate patches 1 and 11 (which are the two bug fixes I see) I think patch 2 is a bug fix as well, but I'd like someone from NXP to comment. > rebase (retest) and post them against the net tree: > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ So I thought this would go through Li Yang's tree. That's where my previous QE related patches have gone through, and at least some need some input from NXP folks - and what MAINTAINERS suggests. So not marking the patches with net or net-next was deliberate. But I'm happy to rearrange and send to net/net-next as appropriate if that's what you and Li Yang can agree to. Thanks, Rasmus
On Sat, 5 Dec 2020 22:11:39 +0100 Rasmus Villemoes wrote: > > Looks like a nice clean up on a quick look. > > > > Please separate patches 1 and 11 (which are the two bug fixes I see) > > I think patch 2 is a bug fix as well, but I'd like someone from NXP to > comment. Sure, makes sense. > > rebase (retest) and post them against the net tree: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ > > So I thought this would go through Li Yang's tree. That's where my > previous QE related patches have gone through, and at least some need > some input from NXP folks - and what MAINTAINERS suggests. So not > marking the patches with net or net-next was deliberate. But I'm happy > to rearrange and send to net/net-next as appropriate if that's what you > and Li Yang can agree to. Ah, now I noticed you didn't CC all of the patches to netdev. Please don't do that, build bots won't be able to validate partially posted patches.
On 05/12/2020 22.27, Jakub Kicinski wrote: > On Sat, 5 Dec 2020 22:11:39 +0100 Rasmus Villemoes wrote: >>> rebase (retest) and post them against the net tree: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ >> >> So I thought this would go through Li Yang's tree. That's where my >> previous QE related patches have gone through, and at least some need >> some input from NXP folks - and what MAINTAINERS suggests. So not >> marking the patches with net or net-next was deliberate. But I'm happy >> to rearrange and send to net/net-next as appropriate if that's what you >> and Li Yang can agree to. > > Ah, now I noticed you didn't CC all of the patches to netdev. > Please don't do that, build bots won't be able to validate partially > posted patches. Hm, I mostly rely on scripts/get_maintainer.pl, invoked via a script I give to --to-cmd/--cc-cmd. They are all sent to LKML, so buildbots should be OK. But regardless of whether Li Yang will be handling it or not, I'll try to remember to cc the whole series to netdev when resending. Rasmus
On 06/12/2020 05:12, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > -----Original Message----- > From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> > Sent: 2020年12月6日 5:12 > To: Jakub Kicinski <kuba@kernel.org> > Cc: Leo Li <leoyang.li@nxp.com>; David S. Miller <davem@davemloft.net>; > Qiang Zhao <qiang.zhao@nxp.com>; netdev@vger.kernel.org; > linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; > linux-arm-kernel@lists.infradead.org; Vladimir Oltean > <vladimir.oltean@nxp.com> > Subject: Re: [PATCH 00/20] ethernet: ucc_geth: assorted fixes and > simplifications > > On 05/12/2020 21.53, Jakub Kicinski wrote: > > On Sat, 5 Dec 2020 20:17:23 +0100 Rasmus Villemoes wrote: > >> While trying to figure out how to allow bumping the MTU with the > >> ucc_geth driver, I fell into a rabbit hole and stumbled on a whole > >> bunch of issues of varying importance - some are outright bug fixes, > >> while most are a matter of simplifying the code to make it more > >> accessible. > >> > >> At the end of digging around the code and data sheet to figure out > >> how it all works, I think the MTU issue might be fixed by a > >> one-liner, but I'm not sure it can be that simple. It does seem to > >> work (ping -s X works for larger values of X, and wireshark confirms > >> that the packets are not fragmented). > >> > >> Re patch 2, someone in NXP should check how the hardware actually > >> works and make an updated reference manual available. > > > > Looks like a nice clean up on a quick look. > > > > Please separate patches 1 and 11 (which are the two bug fixes I see) > > I think patch 2 is a bug fix as well, but I'd like someone from NXP to comment. It 's ok for me. Best Regards, Qiang Zhao
On 08/12/2020 04.07, Qiang Zhao wrote: > On 06/12/2020 05:12, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote: > >> I think patch 2 is a bug fix as well, but I'd like someone from NXP to comment. > > It 's ok for me. I was hoping for something a bit more than that. Can you please go check with the people who made the hardware and those who wrote the manual (probably not the same ones) what is actually up and down, and then report on what they said. It's fairly obvious that allocating 192 bytes instead of 128 should never hurt (unless we run out of muram), but it would be nice with an official "Yes, table 8-111 is wrong, it should say 192", or alternatively, "No, table 8-53 is wrong, those MTU etc. fields don't really exist". Extra points for providing details such as "first revision of the IP had $foo, but that was never shipped in real products, then $bar was changed", etc. Thanks, Rasmus
On 05/12/2020 22.27, Jakub Kicinski wrote: > On Sat, 5 Dec 2020 22:11:39 +0100 Rasmus Villemoes wrote: >>> Looks like a nice clean up on a quick look. >>> >>> Please separate patches 1 and 11 (which are the two bug fixes I see) >> >> I think patch 2 is a bug fix as well, but I'd like someone from NXP to >> comment. > > Sure, makes sense. > >>> rebase (retest) and post them against the net tree: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/ >> >> So I thought this would go through Li Yang's tree. Li, any preference? Will you take this series, or are you ok with the three soc/fsl/qe patches going through the net tree along with the rest? Rasmus
While trying to figure out how to allow bumping the MTU with the ucc_geth driver, I fell into a rabbit hole and stumbled on a whole bunch of issues of varying importance - some are outright bug fixes, while most are a matter of simplifying the code to make it more accessible. At the end of digging around the code and data sheet to figure out how it all works, I think the MTU issue might be fixed by a one-liner, but I'm not sure it can be that simple. It does seem to work (ping -s X works for larger values of X, and wireshark confirms that the packets are not fragmented). Re patch 2, someone in NXP should check how the hardware actually works and make an updated reference manual available. Rasmus Villemoes (20): ethernet: ucc_geth: set dev->max_mtu to 1518 ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram ethernet: ucc_geth: remove unused read of temoder field soc: fsl: qe: make cpm_muram_offset take a const void* argument soc: fsl: qe: store muram_vbase as a void pointer instead of u8 soc: fsl: qe: add cpm_muram_free_addr() helper ethernet: ucc_geth: use qe_muram_free_addr() ethernet: ucc_geth: remove unnecessary memset_io() calls ethernet: ucc_geth: replace kmalloc+memset by kzalloc ethernet: ucc_geth: remove {rx,tx}_glbl_pram_offset from struct ucc_geth_private ethernet: ucc_geth: fix use-after-free in ucc_geth_remove() ethernet: ucc_geth: factor out parsing of {rx,tx}-clock{,-name} properties ethernet: ucc_geth: constify ugeth_primary_info ethernet: ucc_geth: don't statically allocate eight ucc_geth_info ethernet: ucc_geth: use UCC_GETH_{RX,TX}_BD_RING_ALIGNMENT macros directly ethernet: ucc_geth: remove bd_mem_part and all associated code ethernet: ucc_geth: replace kmalloc_array()+for loop by kcalloc() ethernet: ucc_geth: add helper to replace repeated switch statements ethernet: ucc_geth: inform the compiler that numQueues is always 1 ethernet: ucc_geth: simplify rx/tx allocations drivers/net/ethernet/freescale/ucc_geth.c | 553 ++++++++-------------- drivers/net/ethernet/freescale/ucc_geth.h | 15 +- drivers/soc/fsl/qe/qe_common.c | 20 +- include/soc/fsl/qe/qe.h | 15 +- include/soc/fsl/qe/ucc_fast.h | 1 - 5 files changed, 219 insertions(+), 385 deletions(-)