Re: [PATCH v7 07/11] arm_mpam: propagate MSC access errors for mpam_reprogram_ris_partid()

[Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>]

On Fri, 31 Jul 2026 19:03:20 +0200
Andre Przywara <andre.przywara@arm.com> wrote:

> Allow the mpam_reprogram_ris_partid() function check for and return

function to check

> errors, and propagate MSC read and write errors from the lower level up.
> This also covers the callers of this function: mpam_reset_ris() and
> apply_config().
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
One thing below to perhaps modify if you are respinning

Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>

> ---
>  drivers/resctrl/mpam_devices.c | 136 ++++++++++++++++++++++-----------
>  1 file changed, 93 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 38450c55e45e..32088ad1d67e 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -1601,7 +1601,7 @@ void mpam_msmon_reset_mbwu(struct mpam_component *comp, struct mon_cfg *ctx)
>  	}
>  }
>  
> -static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
> +static int mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
>  {
>  	u32 num_words, msb;
>  	u32 bm = ~0;
> @@ -1610,15 +1610,19 @@ static void mpam_reset_msc_bitmap(struct mpam_msc *msc, u16 reg, u16 wd)
>  	lockdep_assert_held(&msc->part_sel_lock);
>  
>  	if (wd == 0)
> -		return;
> +		return 0;
>  
>  	/*
>  	 * Write all ~0 to all but the last 32bit-word, which may
>  	 * have fewer bits...
>  	 */
>  	num_words = DIV_ROUND_UP(wd, 32);
> -	for (i = 0; i < num_words - 1; i++, reg += sizeof(bm))
> -		__mpam_write_reg(msc, reg, bm);
> +	for (i = 0; i < num_words - 1; i++, reg += sizeof(bm)) {

Not really related to what you are doing here, but if you like you could
reduce the scope of i via

	for (int i = 0;...

as it isn't used outside of this loop.

> +		int ret = __mpam_write_reg(msc, reg, bm);
> +
> +		if (ret)
> +			return ret;
Personally I don't like this style but feel free to ignore as this is
very much a personal taste thing. I'd do
		int ret;

		ret = __mpam_write_reg(msc, reg, bm);
		if (ret)
			return ret;
	}
just to keep the source of error and check for error next to each other.

> +	}