Quoting Claudio Imbrenda (2023-05-16 19:17:24)
[...]
> > diff --git a/lib/s390x/interrupt.c b/lib/s390x/interrupt.c
> > index 3f993a363ae2..1180ec44d72f 100644
> > --- a/lib/s390x/interrupt.c
> > +++ b/lib/s390x/interrupt.c
[...]
> > +void irq_set_dat_mode(bool dat, uint64_t as)
> > +{
[...]
> > + for (struct psw *irq_psw = irq_psws[0]; irq_psw != NULL; irq_psw++) {
>
> just call it psw, or cur_psw, it's a little confusing otherwise
will do.
[...]
> alternatively, you can redefine psw with a bitfield (as you mentioned
> offline):
>
> cur_psw->mask.dat = dat;
> if (dat)
> cur_psw->mask.as = as;
Yep, I'll go with that.
>
> > + else
> > + irq_psw->mask |= PSW_MASK_DAT | as << (63 - 16);
>
> otherwise here you're ORing stuff to other stuff, if you had 3 and you
> OR 0 you get 3, but you actually want 0
And that's the advantage of the bitfield. :)
>
> > + }
> > +
> > + mb();
>
> what's the purpose of this?
Make sure that the lowcore really has been written, but I think it's quite
useless, since a function is a sequence point, right?
