Re: [PATCH v6 22/40] arm_mpam: resctrl: Convert to/from MPAMs fixed-point formats

[James Morse]

Hi Gavin,

On 23/03/2026 22:49, Gavin Shan wrote:
> On 3/14/26 12:45 AM, Ben Horgan wrote:
>> From: Dave Martin <Dave.Martin@xxxxxxx>
>>
>> MPAM uses a fixed-point formats for some hardware controls.  Resctrl
>> provides the bandwidth controls as a percentage. Add helpers to convert
>> between these.
>>
>> Ensure bwa_wd is at most 16 to make it clear higher values have no meaning.

> One nitpick below, but this looks good to me in either way.
> 
> Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>

Thanks!


>> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
>> index 0e5e24ef60fe..0c97f7708722 100644
>> --- a/drivers/resctrl/mpam_devices.c
>> +++ b/drivers/resctrl/mpam_devices.c
>> @@ -713,6 +713,13 @@ static void mpam_ris_hw_probe(struct mpam_msc_ris *ris)
>>               mpam_set_feature(mpam_feat_mbw_part, props);
>>             props->bwa_wd = FIELD_GET(MPAMF_MBW_IDR_BWA_WD, mbw_features);
>> +
>> +        /*
>> +         * The BWA_WD field can represent 0-63, but the control fields it
>> +         * describes have a maximum of 16 bits.
>> +         */
>> +        props->bwa_wd = min(props->bwa_wd, 16);
>> +
> 
> 16 may deserve a definition for it since it's a constant value and referred
> for multiple times in this patch, if we need to give this series another
> respin :-)

Hmmm.,. I've left this, I'm not sure what you'd call it. U16_BITS? That sort of thing
might be needed for long/int etc. Here there is either a comment, or its
accepting/returning a u16. I think its fairly obvious where the number 16 is coming from.



Thanks,

James