Re: [PATCH v3 05/16] arm_mpam: propagate MSC read errors for msmon helpers

[Jonathan Cameron]

On Fri, 10 Jul 2026 16:45:09 +0200
Andre Przywara <andre.przywara@xxxxxxx> wrote:

> Allow the helper functions for msmon accesses to return an error, and
> propagate read errors from the lower level up.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
Hi Andre,

Trivial style suggestions.  I'm fussy - I read a lot of code ;)

Jonathan

> ---
>  drivers/resctrl/mpam_devices.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 3b6f9e552a9f..041fca018e68 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1201,25 +1201,31 @@ static void gen_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
>  	}
>  }
>  
> -static void read_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
> -				    u32 *flt_val)
> +static int read_msmon_ctl_flt_vals(struct mon_read *m, u32 *ctl_val,
> +				   u32 *flt_val)
>  {
>  	struct mpam_msc *msc = m->ris->vmsc->msc;
> +	int ret;
>  
>  	switch (m->type) {
>  	case mpam_feat_msmon_csu:
> -		mpam_read_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
> -		mpam_read_monsel_reg(msc, CFG_CSU_FLT, flt_val);
> +		ret = mpam_read_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
> +		if (!ret)
> +			ret = mpam_read_monsel_reg(msc, CFG_CSU_FLT, flt_val);
Burn a few lines to make this flow absolutely standard with errors as
the out of line sections.  Obviously it is a minor thing but the more
standard code is, the easier it is to review,

		ret = mpam_read_monsel_reg(msc, CFG_CSU_CTL, ctl_val);
		if (ret)
			return ret;

		return mpam_read_monsel_reg(msc, CFG_CS_FLT, flt_val);


>  		break;
>  	case mpam_feat_msmon_mbwu_31counter:
>  	case mpam_feat_msmon_mbwu_44counter:
>  	case mpam_feat_msmon_mbwu_63counter:
> -		mpam_read_monsel_reg(msc, CFG_MBWU_CTL, ctl_val);
> -		mpam_read_monsel_reg(msc, CFG_MBWU_FLT, flt_val);
> +		ret = mpam_read_monsel_reg(msc, CFG_MBWU_CTL, ctl_val);
> +		if (!ret)
> +			ret = mpam_read_monsel_reg(msc, CFG_MBWU_FLT, flt_val);

Same for this one.

>  		break;
>  	default:
>  		pr_warn("Unexpected monitor type %d\n", m->type);
> +		return -EINVAL;
>  	}
> +
> +	return ret;
With returns above, this can go.

>  }
>  
>  /* Remove values set by the hardware to prevent apparent mismatches. */