From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> To: Li Yang <leoyang.li@nxp.com> Cc: "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Zhao Qiang <qiang.zhao@nxp.com>, Vladimir Oltean <vladimir.oltean@nxp.com>, Netdev <netdev@vger.kernel.org>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, lkml <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram Date: Tue, 8 Dec 2020 21:12:55 +0100 [thread overview] Message-ID: <22fb47eb-9f51-90ca-907d-bf8252424fce@prevas.dk> (raw) In-Reply-To: <CADRPPNTgqwd37VSqiUcv2otGVr4mnQbuv6r887w_yCp=ha1dvA@mail.gmail.com> On 08/12/2020 20.14, Li Yang wrote: > On Sat, Dec 5, 2020 at 1:21 PM Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >> Table 8-53 in the QUICC Engine Reference manual shows definitions of >> fields up to a size of 192 bytes, not just 128. But in table 8-111, >> one does find the text >> >> Base Address of the Global Transmitter Parameter RAM Page. [...] >> The user needs to allocate 128 bytes for this page. The address must >> be aligned to the page size. >> >> I've checked both rev. 7 (11/2015) and rev. 9 (05/2018) of the manual; >> they both have this inconsistency (and the table numbers are the >> same). > > This does seem to be an inconsistency. I will try to see if I can > find someone who is familiar with this as this is really an old IP. > > Figure 8-61 does mention that size = 128 byte + 64 byte if .... But > this part is not clear also. Hm, indeed, that sentence is simply cut short, it literally says "Additional 64 bytes are needed if". The next line contains "Hierarchical Scheduler, or IP" in a smaller font, but that seems to be a label for the arrow. > > The overlapping does seem to be a problem. Maybe these global > parameters are not sampled at runtime or the parameter RAM is really > only using 128byte depending on the operation mode. Yes, I'm thinking something like that is likely to be the case, since this hasn't seemed to ever cause any problems. But who knows, maybe a few frames just get fragmented very occasionally becauces the MTU0 field spuriously has some random small value. > > Are you getting useful information by reading from the additional 64 > bytes, AFAICT, after the additional allocation, the extra 64 bytes stay at 0, but that's to be expected; they are supposed to be written by the CPU and read by the engine AFAIU. or getting changed behavior for setting these bytes after your > changes? No, as I said: >> I haven't observed any failure that could be attributed to this, I haven't played around with explicitly writing to those 64 bytes after initialization. This whole series started because I searched for the string "MTU" in the manual, but at the end, it didn't seem that I actually needed to modify those MTU fields. Rasmus
WARNING: multiple messages have this Message-ID (diff)
From: Rasmus Villemoes <rasmus.villemoes@prevas.dk> To: Li Yang <leoyang.li@nxp.com> Cc: Vladimir Oltean <vladimir.oltean@nxp.com>, lkml <linux-kernel@vger.kernel.org>, Netdev <netdev@vger.kernel.org>, Jakub Kicinski <kuba@kernel.org>, linuxppc-dev <linuxppc-dev@lists.ozlabs.org>, "David S. Miller" <davem@davemloft.net>, Zhao Qiang <qiang.zhao@nxp.com> Subject: Re: [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram Date: Tue, 8 Dec 2020 21:12:55 +0100 [thread overview] Message-ID: <22fb47eb-9f51-90ca-907d-bf8252424fce@prevas.dk> (raw) In-Reply-To: <CADRPPNTgqwd37VSqiUcv2otGVr4mnQbuv6r887w_yCp=ha1dvA@mail.gmail.com> On 08/12/2020 20.14, Li Yang wrote: > On Sat, Dec 5, 2020 at 1:21 PM Rasmus Villemoes > <rasmus.villemoes@prevas.dk> wrote: >> >> Table 8-53 in the QUICC Engine Reference manual shows definitions of >> fields up to a size of 192 bytes, not just 128. But in table 8-111, >> one does find the text >> >> Base Address of the Global Transmitter Parameter RAM Page. [...] >> The user needs to allocate 128 bytes for this page. The address must >> be aligned to the page size. >> >> I've checked both rev. 7 (11/2015) and rev. 9 (05/2018) of the manual; >> they both have this inconsistency (and the table numbers are the >> same). > > This does seem to be an inconsistency. I will try to see if I can > find someone who is familiar with this as this is really an old IP. > > Figure 8-61 does mention that size = 128 byte + 64 byte if .... But > this part is not clear also. Hm, indeed, that sentence is simply cut short, it literally says "Additional 64 bytes are needed if". The next line contains "Hierarchical Scheduler, or IP" in a smaller font, but that seems to be a label for the arrow. > > The overlapping does seem to be a problem. Maybe these global > parameters are not sampled at runtime or the parameter RAM is really > only using 128byte depending on the operation mode. Yes, I'm thinking something like that is likely to be the case, since this hasn't seemed to ever cause any problems. But who knows, maybe a few frames just get fragmented very occasionally becauces the MTU0 field spuriously has some random small value. > > Are you getting useful information by reading from the additional 64 > bytes, AFAICT, after the additional allocation, the extra 64 bytes stay at 0, but that's to be expected; they are supposed to be written by the CPU and read by the engine AFAIU. or getting changed behavior for setting these bytes after your > changes? No, as I said: >> I haven't observed any failure that could be attributed to this, I haven't played around with explicitly writing to those 64 bytes after initialization. This whole series started because I searched for the string "MTU" in the manual, but at the end, it didn't seem that I actually needed to modify those MTU fields. Rasmus
next prev parent reply other threads:[~2020-12-08 20:26 UTC|newest] Thread overview: 102+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-05 19:17 [PATCH 00/20] ethernet: ucc_geth: assorted fixes and simplifications Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 01/20] ethernet: ucc_geth: set dev->max_mtu to 1518 Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-10 1:25 ` Andrew Lunn 2020-12-10 1:25 ` Andrew Lunn 2021-01-05 14:17 ` Joakim Tjernlund 2021-01-05 14:17 ` Joakim Tjernlund 2021-01-05 14:33 ` Andrew Lunn 2021-01-05 14:33 ` Andrew Lunn 2021-01-05 14:44 ` Joakim Tjernlund 2021-01-05 14:44 ` Joakim Tjernlund 2021-01-05 14:54 ` Andrew Lunn 2021-01-05 14:54 ` Andrew Lunn 2020-12-05 19:17 ` [PATCH 02/20] ethernet: ucc_geth: fix definition and size of ucc_geth_tx_global_pram Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-08 19:14 ` Li Yang 2020-12-08 19:14 ` Li Yang 2020-12-08 20:12 ` Rasmus Villemoes [this message] 2020-12-08 20:12 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 03/20] ethernet: ucc_geth: remove unused read of temoder field Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 04/20] soc: fsl: qe: make cpm_muram_offset take a const void* argument Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 05/20] soc: fsl: qe: store muram_vbase as a void pointer instead of u8 Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 06/20] soc: fsl: qe: add cpm_muram_free_addr() helper Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 07/20] ethernet: ucc_geth: use qe_muram_free_addr() Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-10 1:36 ` Andrew Lunn 2020-12-10 1:36 ` Andrew Lunn 2020-12-05 19:17 ` [PATCH 08/20] ethernet: ucc_geth: remove unnecessary memset_io() calls Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 09/20] ethernet: ucc_geth: replace kmalloc+memset by kzalloc Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 10/20] ethernet: ucc_geth: remove {rx,tx}_glbl_pram_offset from struct ucc_geth_private Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 10/20] ethernet: ucc_geth: remove {rx, tx}_glbl_pram_offset " Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 11/20] ethernet: ucc_geth: fix use-after-free in ucc_geth_remove() Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 20:48 ` Jakub Kicinski 2020-12-05 20:48 ` Jakub Kicinski 2020-12-05 21:04 ` Rasmus Villemoes 2020-12-05 21:04 ` Rasmus Villemoes 2020-12-05 21:19 ` Jakub Kicinski 2020-12-05 21:19 ` Jakub Kicinski 2020-12-05 21:35 ` Rasmus Villemoes 2020-12-05 21:35 ` Rasmus Villemoes 2020-12-05 21:50 ` Rasmus Villemoes 2020-12-05 21:50 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 12/20] ethernet: ucc_geth: factor out parsing of {rx,tx}-clock{,-name} properties Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 12/20] ethernet: ucc_geth: factor out parsing of {rx, tx}-clock{, -name} properties Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 13/20] ethernet: ucc_geth: constify ugeth_primary_info Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 14/20] ethernet: ucc_geth: don't statically allocate eight ucc_geth_info Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-08 15:13 ` Christophe Leroy 2020-12-08 15:13 ` Christophe Leroy 2020-12-08 21:17 ` Rasmus Villemoes 2020-12-08 21:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 15/20] ethernet: ucc_geth: use UCC_GETH_{RX,TX}_BD_RING_ALIGNMENT macros directly Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 15/20] ethernet: ucc_geth: use UCC_GETH_{RX, TX}_BD_RING_ALIGNMENT " Rasmus Villemoes 2020-12-08 15:14 ` Christophe Leroy 2020-12-08 15:14 ` Christophe Leroy 2020-12-05 19:17 ` [PATCH 16/20] ethernet: ucc_geth: remove bd_mem_part and all associated code Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 17/20] ethernet: ucc_geth: replace kmalloc_array()+for loop by kcalloc() Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 18/20] ethernet: ucc_geth: add helper to replace repeated switch statements Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-08 15:21 ` Christophe Leroy 2020-12-08 15:21 ` Christophe Leroy 2020-12-08 20:55 ` Rasmus Villemoes 2020-12-08 20:55 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 19/20] ethernet: ucc_geth: inform the compiler that numQueues is always 1 Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 19:17 ` [PATCH 20/20] ethernet: ucc_geth: simplify rx/tx allocations Rasmus Villemoes 2020-12-05 19:17 ` Rasmus Villemoes 2020-12-05 20:53 ` [PATCH 00/20] ethernet: ucc_geth: assorted fixes and simplifications Jakub Kicinski 2020-12-05 20:53 ` Jakub Kicinski 2020-12-05 20:53 ` Jakub Kicinski 2020-12-05 21:11 ` Rasmus Villemoes 2020-12-05 21:11 ` Rasmus Villemoes 2020-12-05 21:11 ` Rasmus Villemoes 2020-12-05 21:27 ` Jakub Kicinski 2020-12-05 21:27 ` Jakub Kicinski 2020-12-05 21:27 ` Jakub Kicinski 2020-12-05 21:36 ` Rasmus Villemoes 2020-12-05 21:36 ` Rasmus Villemoes 2020-12-05 21:36 ` Rasmus Villemoes 2020-12-10 7:52 ` Rasmus Villemoes 2020-12-10 7:52 ` Rasmus Villemoes 2020-12-10 7:52 ` Rasmus Villemoes 2020-12-08 3:07 ` Qiang Zhao 2020-12-08 3:07 ` Qiang Zhao 2020-12-08 3:07 ` Qiang Zhao 2020-12-08 8:13 ` Rasmus Villemoes 2020-12-08 8:13 ` Rasmus Villemoes 2020-12-08 8:13 ` Rasmus Villemoes
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=22fb47eb-9f51-90ca-907d-bf8252424fce@prevas.dk \ --to=rasmus.villemoes@prevas.dk \ --cc=davem@davemloft.net \ --cc=kuba@kernel.org \ --cc=leoyang.li@nxp.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=netdev@vger.kernel.org \ --cc=qiang.zhao@nxp.com \ --cc=vladimir.oltean@nxp.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.