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

Message ID 20231130075818.18401-4-gakula@marvell.com
State New
Headers
Series octeontx2-af: miscellaneous fixes |

Commit Message

Geetha sowjanya Nov. 30, 2023, 7:58 a.m. UTC
  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(-)
  

Comments

Simon Horman Dec. 3, 2023, 5 p.m. UTC | #1
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
>
  
Geetha sowjanya Dec. 4, 2023, 5:22 a.m. UTC | #2
> -----Original Message-----
> From: Simon Horman <horms@kernel.org>
> Sent: Sunday, December 3, 2023 10:31 PM
> To: Geethasowjanya Akula <gakula@marvell.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; kuba@kernel.org;
> davem@davemloft.net; pabeni@redhat.com; edumazet@google.com; Sunil
> Kovvuri Goutham <sgoutham@marvell.com>; Linu Cherian
> <lcherian@marvell.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>;
> Subbaraya Sundeep Bhatta <sbhatta@marvell.com>; Hariprasad Kelam
> <hkelam@marvell.com>
> Subject: [EXT] Re: [net v3 PATCH 3/5] octeontx2-af: Fix mcs stats register
> address
> 
> External Email
> 
> ----------------------------------------------------------------------
> 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)) ...
> 
Hi Simon,
Thanks for your suggestion. Will take this in different patch set.

> 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
> >
  

Patch

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; })
+
+#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)