Re: [PATCH v3 01/16] arm_mpam: let low level MSC read accessors return an error

[Andre Przywara]

Hi Jonathan, Ben,

On 7/15/26 18:01, Ben Horgan wrote:
> Hi Andre,
>
> On 7/10/26 22:40, Andre Przywara wrote:
> > Hi Jonathan,
> >
> > many thanks for the time you spent on this - though it wasn't particularly pretty, I guess.
> >
> > This is just a quick reply, unfortunately (well...) I will be on holidays next week, so cannot reply
> > in detail now.
> >
> > On 7/10/26 20:11, Jonathan Cameron wrote:
> > > On Fri, 10 Jul 2026 16:45:05 +0200
> > > Andre Przywara <andre.przywara@xxxxxxx> wrote:
> > >
> > > > The upcoming MPAM-Fb support does not use MMIO primitives to access an
> > > > MSC, but employs a shared-memory/doorbell based firmware protocol.
> > > > Its complexity means that is must be able to handle errors, whereas we
> > > > always assume an MSC access succeeds today.
> > > >
> > > > Change the __mpam_read_reg() low level accessor function to return the
> > > > requested data through a pointer, and return an error code instead.
> > > > At the moment this is always 0, but this will change with alternative
> > > > MSC access methods.
> > > > Change all users of those MSC read wrappers to comply with the new
> > > > prototype, though at the moment without propagating any errors.
> > > >
> > > > Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> > >
> > > Why do it in this order? It seems like it is a path for some code churn
> > > from patch to path.
> >
> > Well, this is pain however you do this - as you figured. And I knew this before, that's why I
> > avoided this in v1, but then found we have to bite the bullet at some point anyway, so we could as
> > well just do it now.
> >
> > So I did the change in one go, and the diff was completely unreadable and even more so unreviewable.
> > Then I figured to start with the actual root cause: the low level accessors, to show the motivation,
> > then split the rest up in reasonably small chunks. And with the unavoidable dependencies, this led
> > to the structure you see, with the added churn of requested changes and refactors between v2 and v3.
>
>  From my point view, I think the order of the patches is ok and I'd consider it a bonus if you do
> find a clean way to avoid changing the same line twice in this patch set. What I'm more interested
> in is that the code itself is clean and consistent but with correctness trumping everything. I
> hadn't discovered the ACQUIRE() and ACQUIRE_ERR() before but I do expect that, as Jonathan points
> out, they could be used judiciously to make the code clearer.

So I now squashed most of the write patches into the ones introducing 
the read return handling, more grouping them by function rather than by 
read or write. This already removes most of the "touches a line again" 
issues. The cost is a larger first patch.

And as mentioned in the previous replies, I also used scoped_guard() and 
friends to make the error handling look more common and more schematic.

Cheers,
Andre