Re: [PATCH 04/17] bus/dpaa: add port buffer manager stats

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 8/1/2024 11:53 AM, Hemant Agrawal wrote:
> Add BMI statistics and improving the existing extended
> statistics
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> ---
>  drivers/bus/dpaa/base/fman/fman_hw.c | 65 +++++++++++++++++++++++++++-
>  drivers/bus/dpaa/include/fman.h      |  4 +-
>  drivers/bus/dpaa/include/fsl_fman.h  | 12 +++++
>  drivers/bus/dpaa/version.map         |  4 ++
>  drivers/net/dpaa/dpaa_ethdev.c       | 46 +++++++++++++++++---
>  drivers/net/dpaa/dpaa_ethdev.h       | 12 +++++
>  6 files changed, 134 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/bus/dpaa/base/fman/fman_hw.c b/drivers/bus/dpaa/base/fman/fman_hw.c
> index 24a99f7235..27b39a4975 100644
> --- a/drivers/bus/dpaa/base/fman/fman_hw.c
> +++ b/drivers/bus/dpaa/base/fman/fman_hw.c
> @@ -244,8 +244,8 @@ fman_if_stats_get_all(struct fman_if *p, uint64_t *value, int n)
>  	uint64_t base_offset = offsetof(struct memac_regs, reoct_l);
>  
>  	for (i = 0; i < n; i++)
> -		value[i] = (((u64)in_be32((char *)regs + base_offset + 8 * i) |
> -				(u64)in_be32((char *)regs + base_offset +
> +		value[i] = ((u64)in_be32((char *)regs + base_offset + 8 * i) |
> +				((u64)in_be32((char *)regs + base_offset +
>  				8 * i + 4)) << 32);
>

Above change looks like a bug fix, it is converting from
"(a | b) << 32"  to  "a | (b << 32)"

Syntax wise a small change that is easy to miss, but impacts the result.

Why not simplify it something like:
uint64_t a = in_be32((char *)regs + base_offset + 8 * i)
uint64_t b = in_be32((char *)regs + base_offset + 8 * i + 4)
value[i] = a | b << 32

Anyway, my point is, should it go to its own patch, with fixes tag and
stable tag, so it can be backported to stable releases.

<...>