Re: [PATCH net-next v2 0/3] net: stmmac: EST implementation

[Serge Semin]

Hi Rohan

On Fri, Dec 01, 2023 at 01:52:49PM +0800, Rohan G Thomas wrote:
> Hi,
> This patchset extends EST interrupt handling support to DWXGMAC IP
> followed by refactoring of EST implementation. Added a separate
> module for EST and moved all EST related functions to the new module.
> 
> Also added support for EST cycle-time-extension.
> 
> changelog v2:
> * Refactor EST implementation as suggested by Serge and Jakub
> * Added support for EST cycle-time-extension

Thanks for the update and especially for keeping your promise in
refactoring the EST code. The series has already been merged in, but
anyway here is my tag:

Reviewed-by: Serge Semin <fancer.lancer@xxxxxxxxx>

* which at least for Patch 1 you must have forgotten to add since v1
* seeing the patch hasn't changed.

Regarding the multiplier and the MTL_EST_Status.BTRL field width info
you submitted earlier:

On Fri, Dec 01, 2023 at 14:49:09PM +0800, Rohan G Thomas wrote:
> Managed to get DWC_ether_qos_relnotes.pdf for v5.20a and v5.30a. But I couldn't
> find anything related to this. So for refactoring, I'm keeping the logic as in
> the upstream code to avoid any regression.
>
>>
>> Also please double check that your DW QoS Eth v5.30a for sure states that
>> MTL_EST_CONTROL.PTOV contains value multiplied by _6_. So we wouldn't
>> be wasting time trying to workaround a more complex problem than we
>> already have.
>
> Yes, I checked this again. For DW QoS Eth v5.30a the multiplier for
> MTL_EST_CONTROL.PTOV is _9_ as per the databook.
>
> Also noticed a similar difference for MTL_EST_Status.BTRL field length. As per
> the upstream code and DW QoS Eth v5.10a databook this field covers bit 8 to bit
> 11. But for the xgmac IP and DW QoS Eth v5.30a databook this field covers bit 8
> to bit 15. Again nothing mentioned in the release notes. Here also I'm keeping
> the logic as in the upstream code to avoid any regression.

Thank you for digging into the problem. It's strange that Synopsys
hasn't mentioned about so many EST CSRs changes in the Release Notes.
Anyway if nothing in there my next step would have been to reach
somebody from Synopsys to clarify the situation and find out the
reason of the change. But seeing it's an additional burden and vendors
reply not that swiftly as we would wish I guess the best choice would
be indeed keeping the semantics as is, at least until somebody finds
that problem critical.

-Serge(y)

> 
> Rohan G Thomas (3):
>   net: stmmac: xgmac: EST interrupts handling
>   net: stmmac: Refactor EST implementation
>   net: stmmac: Add support for EST cycle-time-extension
> 
>  drivers/net/ethernet/stmicro/stmmac/Makefile  |   2 +-
>  drivers/net/ethernet/stmicro/stmmac/common.h  |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac4_core.c |   4 -
>  drivers/net/ethernet/stmicro/stmmac/dwmac5.c  | 137 ---------------
>  drivers/net/ethernet/stmicro/stmmac/dwmac5.h  |  51 ------
>  .../net/ethernet/stmicro/stmmac/dwxgmac2.h    |  16 --
>  .../ethernet/stmicro/stmmac/dwxgmac2_core.c   |  53 ------
>  drivers/net/ethernet/stmicro/stmmac/hwif.c    |  21 +++
>  drivers/net/ethernet/stmicro/stmmac/hwif.h    |  22 ++-
>  drivers/net/ethernet/stmicro/stmmac/stmmac.h  |   1 +
>  .../net/ethernet/stmicro/stmmac/stmmac_est.c  | 165 ++++++++++++++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_est.h  |  64 +++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |   2 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_ptp.c  |   4 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_tc.c   |   8 +-
>  15 files changed, 276 insertions(+), 275 deletions(-)
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_est.c
>  create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_est.h
> 
> -- 
> 2.26.2
>