Re: [PATCH v7 06/11] arm_mpam: propagate MSC access errors for state saving function
Jonathan Cameron <[email protected]>
| Newsgroups | gmane.linux.acpi.devel,gmane.linux.ports.arm.kernel,gmane.linux.kernel |
|---|---|
| Organization | Qualcomm |
| Message-ID | <[email protected]> |
On Fri, 31 Jul 2026 19:03:19 +0200 Andre Przywara <[email protected]> wrote: > Allow the mpam_save_mbwu_state() function to return an error, and > propagate read and write errors from the lower level up. > > Signed-off-by: Andre Przywara <[email protected]> > Reviewed-by: Jonathan Cameron <[email protected]> > Reviewed-by: Ben Horgan <[email protected]> If you happen to be respinning i think the code grouping vs white space in here could be slightly improved. > --- > drivers/resctrl/mpam_devices.c | 29 ++++++++++++++++++++++------- > 1 file changed, 22 insertions(+), 7 deletions(-) > > diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c > index fa8ed20a6740..38450c55e45e 100644 > --- a/drivers/resctrl/mpam_devices.c > +++ b/drivers/resctrl/mpam_devices.c > @@ -1838,22 +1838,37 @@ static int mpam_save_mbwu_state(void *arg) > > mon_sel = FIELD_PREP(MSMON_CFG_MON_SEL_MON_SEL, i) | > FIELD_PREP(MSMON_CFG_MON_SEL_RIS, ris->ris_idx); > - mpam_write_monsel_reg(msc, CFG_MON_SEL, mon_sel); > - mpam_read_monsel_reg(msc, CFG_MBWU_FLT, &cur_flt); > - mpam_read_monsel_reg(msc, CFG_MBWU_CTL, &cur_ctl); > - mpam_write_monsel_reg(msc, CFG_MBWU_CTL, 0); > + ret = mpam_write_monsel_reg(msc, CFG_MON_SEL, mon_sel); > + if (ret) > + return ret; > + ret = mpam_read_monsel_reg(msc, CFG_MBWU_FLT, &cur_flt); > + if (ret) > + return ret; > + ret = mpam_read_monsel_reg(msc, CFG_MBWU_CTL, &cur_ctl); > + if (ret) > + return ret; > + ret = mpam_write_monsel_reg(msc, CFG_MBWU_CTL, 0); > + if (ret) > + return ret; > > if (mpam_ris_has_mbwu_long_counter(ris)) { > ret = mpam_msc_read_mbwu_l(msc, &val); > if (ret) > return ret; > - mpam_msc_zero_mbwu_l(msc); > + ret = mpam_msc_zero_mbwu_l(msc); > + if (ret) > + return ret; > } else { > u32 val32; > > - mpam_read_monsel_reg(msc, MBWU, &val32); > + ret = mpam_read_monsel_reg(msc, MBWU, &val32); > + if (ret) > + return ret; > + > val = val32; > - mpam_write_monsel_reg(msc, MBWU, 0); > + ret = mpam_write_monsel_reg(msc, MBWU, 0); > + if (ret) > + return ret; The grouping via white space usage here seems less than ideal. You have: ret = mpam_read_monsel_reg(msc, MBWU, &val32); if (ret) return ret; val = val32; // this looks to be part of block below not above. ret = mpam_write_monsel_reg(msc, MBWU, 0); if (ret) return ret; Probably: ret = mpam_read_monsel_reg(msc, MBWU, &val32); if (ret) return ret; val = val32; // Now it is in groups with related code. ret = mpam_write_monsel_reg(msc, MBWU, 0); if (ret) return ret; Or don't have a blank line in there at all. > } > > cfg->mon = i;