Re: [net v3 PATCH 3/5] octeontx2-af: Fix mcs stats register address

Simon Horman
2023-12-03T17:01:04Z

On Thu, Nov 30, 2023 at 01:28:16PM +0530, Geetha sowjanya wrote:
> This patch adds the miss mcs stats register
> for mcs supported platforms.
>
> Fixes: 9312150af8da ("octeontx2-af: cn10k: mcs: Support for stats collection")
> Signed-off-by: Geetha sowjanya <gakula@marvell.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@intel.com>
> ---
> .../net/ethernet/marvell/octeontx2/af/mcs.c | 4 +--
> .../ethernet/marvell/octeontx2/af/mcs_reg.h | 31 ++++++++++++++++---
> 2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mcs.c b/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> index d6effbe46208..d4a4e4c837ec 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs.c
> @@ -117,7 +117,7 @@ void mcs_get_rx_secy_stats(struct mcs *mcs, struct mcs_secy_stats *stats, int id
> reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYTAGGEDCTLX(id);
> stats->pkt_tagged_ctl_cnt = mcs_reg_read(mcs, reg);
>
> - reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYUNTAGGEDORNOTAGX(id);
> + reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYUNTAGGEDX(id);
> stats->pkt_untaged_cnt = mcs_reg_read(mcs, reg);
>
> reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYCTLX(id);
> @@ -215,7 +215,7 @@ void mcs_get_sc_stats(struct mcs *mcs, struct mcs_sc_stats *stats,
> reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSCNOTVALIDX(id);
> stats->pkt_notvalid_cnt = mcs_reg_read(mcs, reg);
>
> - reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSCUNCHECKEDOROKX(id);
> + reg = MCSX_CSE_RX_MEM_SLAVE_INPKTSSCUNCHECKEDX(id);
> stats->pkt_unchecked_cnt = mcs_reg_read(mcs, reg);
>
> if (mcs->hw->mcs_blks > 1) {
> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mcs_reg.h b/drivers/net/ethernet/marvell/octeontx2/af/mcs_reg.h
> index f3ab01fc363c..f4c6de89002c 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mcs_reg.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mcs_reg.h
> @@ -810,14 +810,37 @@
> offset = 0x9d8ull; \
> offset; })
>
> +#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSCUNCHECKEDX(a) ({ \
> + u64 offset; \
> + \
> + offset = 0xee80ull; \
> + if (mcs->hw->mcs_blks > 1) \
> + offset = 0xe818ull; \
> + offset += (a) * 0x8ull; \
> + offset; })

Hi Geetha,

I see this is consistent with existing code in this file,
but I do wonder if there would be value in moving to a more
compact mechanism at some point. F.e. (completely untested!):

#define MCSX_REG(base, a) ((base) + (a) * 0x8ull)
#define MCSX_MB_REG(base_mb, base, a) \
MCSX_REG((mcs->hw->mcs_blks > 1 ? (base_mb) : (base)), (a))
...
#define MCSX_MCS_TOP_SLAVE_PORT_RESET(a) MCSX_MB_REG(0xa28ull, 0x408ull, (a))
...
#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYCTLX(a) MCSX_REG(0xb680ull, (a))
...

In any case, such a change isn't for this patch, which looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

> +
> +#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYUNTAGGEDX(a) ({ \
> + u64 offset; \
> + \
> + offset = 0xa680ull; \
> + if (mcs->hw->mcs_blks > 1) \
> + offset = 0xd018ull; \
> + offset += (a) * 0x8ull; \
> + offset; })
> +
> +#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSCLATEORDELAYEDX(a) ({ \
> + u64 offset; \
> + \
> + offset = 0xf680ull; \
> + if (mcs->hw->mcs_blks > 1) \
> + offset = 0xe018ull; \
> + offset += (a) * 0x8ull; \
> + offset; })
> +
> #define MCSX_CSE_RX_MEM_SLAVE_INOCTETSSCDECRYPTEDX(a) (0xe680ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INOCTETSSCVALIDATEX(a) (0xde80ull + (a) * 0x8ull)
> -#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYUNTAGGEDORNOTAGX(a) (0xa680ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYNOTAGX(a) (0xd218 + (a) * 0x8ull)
> -#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYUNTAGGEDX(a) (0xd018ull + (a) * 0x8ull)
> -#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSCUNCHECKEDOROKX(a) (0xee80ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INPKTSSECYCTLX(a) (0xb680ull + (a) * 0x8ull)
> -#define MCSX_CSE_RX_MEM_SLAVE_INPKTSSCLATEORDELAYEDX(a) (0xf680ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INPKTSSAINVALIDX(a) (0x12680ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INPKTSSANOTUSINGSAERRORX(a) (0x15680ull + (a) * 0x8ull)
> #define MCSX_CSE_RX_MEM_SLAVE_INPKTSSANOTVALIDX(a) (0x13680ull + (a) * 0x8ull)
> --
> 2.25.1
>