| Message ID | cover.1722421644.git.0x1207@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: stmmac: FPE via ethtool + tc | expand |
Hi Furong, On Wed, Jul 31, 2024 at 06:43:11PM +0800, Furong Xu wrote: > Move the Frame Preemption(FPE) over to the new standard API which uses > ethtool-mm/tc-mqprio/tc-taprio. Thanks for working on this! I will review it soon. On the DWMAC 5.10a that you've tested, were other patches also necessary? What is the status of the kselftest? Does it pass? Can you post its output as part of the cover letter?
On Thu, Aug 01, 2024 at 07:02:24PM +0300, Vladimir Oltean wrote: > Hi Furong, > > On Wed, Jul 31, 2024 at 06:43:11PM +0800, Furong Xu wrote: > > Move the Frame Preemption(FPE) over to the new standard API which uses > > ethtool-mm/tc-mqprio/tc-taprio. > > Thanks for working on this! I will review it soon. > > On the DWMAC 5.10a that you've tested, were other patches also necessary? > What is the status of the kselftest? Does it pass? Can you post its > output as part of the cover letter? Can you additionally test FPE across a suspend/resume cycle, in 2 cases: - FPE was enabled before suspend, make sure it runs again automatically after resume, and that there are no deadlock issues (to be confirmed with CONFIG_PROVE_LOCKING) - FPE was disabled before suspend, make sure it can be enabled successfully after resume
Hi Furong On Wed, Jul 31, 2024 at 06:43:11PM +0800, Furong Xu wrote: > Move the Frame Preemption(FPE) over to the new standard API which uses > ethtool-mm/tc-mqprio/tc-taprio. Thank you very much for the series. I am not that much aware of the FPE and ethtool MAC Merge guts. But I had a thoughtful glance to the FPE-handshaking algo and got to a realization that all the FPE-related data defined in the include/linux/stmmac.h weren't actually platform-data. All of that are the run-time settings utilized during the handshaking algo execution. So could you please move the fpe_cfg field to the stmmac_priv data and move the FPE-related declarations from the include/linux/stmmac.h header file to the drivers/net/ethernet/stmicro/stmmac/stmmac.h file? It's better to be done in a pre-requisite (preparation) patch of your series. Another useful cleanup would be moving the entire FPE-implementation from stmmac_main.c to a separate module. Thus the main driver code would be simplified a bit. I guess it could be moved to the stmmac_tc.c file since FPE is the TC-related feature. Right? Vladimir, what do you think about the suggestions above? -Serge(y) > > Furong Xu (5): > net: stmmac: configure FPE via ethtool-mm > net: stmmac: support fp parameter of tc-mqprio > net: stmmac: support fp parameter of tc-taprio > net: stmmac: drop unneeded FPE handshake code > net: stmmac: silence FPE kernel logs > > .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 6 + > drivers/net/ethernet/stmicro/stmmac/dwmac5.c | 37 +++++- > drivers/net/ethernet/stmicro/stmmac/dwmac5.h | 7 ++ > drivers/net/ethernet/stmicro/stmmac/hwif.h | 14 +++ > drivers/net/ethernet/stmicro/stmmac/stmmac.h | 3 + > .../ethernet/stmicro/stmmac/stmmac_ethtool.c | 111 ++++++++++++++++++ > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 25 ++-- > .../net/ethernet/stmicro/stmmac/stmmac_tc.c | 95 ++++++++++----- > include/linux/stmmac.h | 2 +- > 9 files changed, 248 insertions(+), 52 deletions(-) > > -- > 2.34.1 > >
Hi Serge On Mon, 5 Aug 2024 20:11:10 +0300, Serge Semin <fancer.lancer@gmail.com> wrote: > Hi Furong > > Thank you very much for the series. I am not that much aware of the > FPE and ethtool MAC Merge guts. But I had a thoughtful glance to the > FPE-handshaking algo and got to a realization that all the FPE-related > data defined in the include/linux/stmmac.h weren't actually > platform-data. All of that are the run-time settings utilized during > the handshaking algo execution. > > So could you please move the fpe_cfg field to the stmmac_priv data and > move the FPE-related declarations from the include/linux/stmmac.h > header file to the drivers/net/ethernet/stmicro/stmmac/stmmac.h file? > It's better to be done in a pre-requisite (preparation) patch of your > series. This will be included in V2 of this patchset. > > Another useful cleanup would be moving the entire FPE-implementation > from stmmac_main.c to a separate module. Thus the main > driver code would be simplified a bit. I guess it could be moved to > the stmmac_tc.c file since FPE is the TC-related feature. Right? Thanks for your advice. A few weeks ago, I sent a patchset to refactor FPE implementation: https://lore.kernel.org/all/cover.1720512888.git.0x1207@gmail.com/ Vladimir suggested me to move the FPE over to the new standard API, then this patchset comes. I am working on V2 of this patchset, once this patchset get merged, a new FPE implementation will be sent to review.
On Tue, Aug 06, 2024 at 12:55:24PM +0800, Furong Xu wrote: > Hi Serge > > On Mon, 5 Aug 2024 20:11:10 +0300, Serge Semin <fancer.lancer@gmail.com> wrote: > > Hi Furong > > > > Thank you very much for the series. I am not that much aware of the > > FPE and ethtool MAC Merge guts. But I had a thoughtful glance to the > > FPE-handshaking algo and got to a realization that all the FPE-related > > data defined in the include/linux/stmmac.h weren't actually > > platform-data. All of that are the run-time settings utilized during > > the handshaking algo execution. > > > > So could you please move the fpe_cfg field to the stmmac_priv data and > > move the FPE-related declarations from the include/linux/stmmac.h > > header file to the drivers/net/ethernet/stmicro/stmmac/stmmac.h file? > > It's better to be done in a pre-requisite (preparation) patch of your > > series. > This will be included in V2 of this patchset. > > > > > Another useful cleanup would be moving the entire FPE-implementation > > from stmmac_main.c to a separate module. Thus the main > > driver code would be simplified a bit. I guess it could be moved to > > the stmmac_tc.c file since FPE is the TC-related feature. Right? > > Thanks for your advice. > > A few weeks ago, I sent a patchset to refactor FPE implementation: > https://lore.kernel.org/all/cover.1720512888.git.0x1207@gmail.com/ > > Vladimir suggested me to move the FPE over to the new standard API, > then this patchset comes. > > I am working on V2 of this patchset, once this patchset get merged, > a new FPE implementation will be sent to review. If the new FPE-implementation includes the FPE-hanshaking stuff moved out from the stmmac_main.c it will be just wonderful. Thanks! -Serge(y)