diff mbox series

[net-next] eth: fbnic: fix csr boundary for RPM RAM section

Message ID 20241218232614.439329-1-mohsin.bashr@gmail.com (mailing list archive)
State New
Delegated to: Netdev Maintainers
Headers show
Series [net-next] eth: fbnic: fix csr boundary for RPM RAM section | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 1 this patch: 1
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: 3d12862b216d ("eth: fbnic: Add support to dump registers")'
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-12-19--12-00 (tests: 881)

Commit Message

Mohsin Bashir Dec. 18, 2024, 11:25 p.m. UTC
The CSR dump support leverages the FBNIC_BOUNDS macro, which pads the end
condition for each section by adding an offset of 1. However, the RPC RAM
section, which is dumped differently from other sections, does not rely
on this macro and instead directly uses end boundary address. Hence,
subtracting 1 from the end address results in skipping a register.

Fixes 3d12862b216d (“eth: fbnic: Add support to dump registers”)
Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
---
 drivers/net/ethernet/meta/fbnic/fbnic_csr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michal Swiatkowski Dec. 19, 2024, 7:31 a.m. UTC | #1
On Wed, Dec 18, 2024 at 03:25:58PM -0800, Mohsin Bashir wrote:
> The CSR dump support leverages the FBNIC_BOUNDS macro, which pads the end
> condition for each section by adding an offset of 1. However, the RPC RAM
> section, which is dumped differently from other sections, does not rely
> on this macro and instead directly uses end boundary address. Hence,
> subtracting 1 from the end address results in skipping a register.
> 
> Fixes 3d12862b216d (âeth: fbnic: Add support to dump registersâ)
> Signed-off-by: Mohsin Bashir <mohsin.bashr@gmail.com>
> ---
>  drivers/net/ethernet/meta/fbnic/fbnic_csr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
> index 2118901b25e9..aeb9f333f4c7 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
> @@ -64,7 +64,7 @@ static void fbnic_csr_get_regs_rpc_ram(struct fbnic_dev *fbd, u32 **data_p)
>  	u32 i, j;
>  
>  	*(data++) = start;
> -	*(data++) = end - 1;
> +	*(data++) = end;
>  
>  	/* FBNIC_RPC_TCAM_ACT */
>  	for (i = 0; i < FBNIC_RPC_TCAM_ACT_NUM_ENTRIES; i++) {
> -- 

Thanks,
Reviewed-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>

> 2.43.5
Andrew Lunn Dec. 19, 2024, 8:25 a.m. UTC | #2
On Wed, Dec 18, 2024 at 03:25:58PM -0800, Mohsin Bashir wrote:
> The CSR dump support leverages the FBNIC_BOUNDS macro, which pads the end
> condition for each section by adding an offset of 1. However, the RPC RAM
> section, which is dumped differently from other sections, does not rely
> on this macro and instead directly uses end boundary address. Hence,
> subtracting 1 from the end address results in skipping a register.

Maybe it would be better to actually use FBNIC_BOUNDS macro, to make
it the same as all the others, and so avoid errors like this because
it is special?

	Andrew
Jakub Kicinski Dec. 20, 2024, 9:03 p.m. UTC | #3
On Thu, 19 Dec 2024 09:25:41 +0100 Andrew Lunn wrote:
> On Wed, Dec 18, 2024 at 03:25:58PM -0800, Mohsin Bashir wrote:
> > The CSR dump support leverages the FBNIC_BOUNDS macro, which pads the end
> > condition for each section by adding an offset of 1. However, the RPC RAM
> > section, which is dumped differently from other sections, does not rely
> > on this macro and instead directly uses end boundary address. Hence,
> > subtracting 1 from the end address results in skipping a register.  
> 
> Maybe it would be better to actually use FBNIC_BOUNDS macro, to make
> it the same as all the others, and so avoid errors like this because
> it is special?

That may require changing the format / contents of the dump a bit
to include unimplemented registers (which would read as ff).
Note that the subject is incorrect, the patch is for net, 
so the one-liner fix is more appealing at this stage.
But I may be biased...
diff mbox series

Patch

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
index 2118901b25e9..aeb9f333f4c7 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
@@ -64,7 +64,7 @@  static void fbnic_csr_get_regs_rpc_ram(struct fbnic_dev *fbd, u32 **data_p)
 	u32 i, j;
 
 	*(data++) = start;
-	*(data++) = end - 1;
+	*(data++) = end;
 
 	/* FBNIC_RPC_TCAM_ACT */
 	for (i = 0; i < FBNIC_RPC_TCAM_ACT_NUM_ENTRIES; i++) {