Re: [PATCH v3 2/2] mailbox: arm_mhuv3: Add driver

[Cristian Marussi]

On Mon, Apr 08, 2024 at 11:40:24AM +0100, Cristian Marussi wrote:
> On Sun, Apr 07, 2024 at 08:14:23PM -0500, Jassi Brar wrote:
> > On Thu, Apr 4, 2024 at 1:25 AM Cristian Marussi
> > <cristian.marussi@xxxxxxx> wrote:
> > >
> > > Add support for ARM MHUv3 mailbox controller.
> > >
> > > Support is limited to the MHUv3 Doorbell extension using only the PBX/MBX
> > > combined interrupts.
> > >
> 
> Hi Jassi,
> 
> thanks for having a look at this !
> 

[snip]

> > > Signed-off-by: Cristian Marussi <cristian.marussi@xxxxxxx>
> > > ---
> > > +struct aidr {
> > > +       u32 arch_minor_rev : 4;
> > > +       u32 arch_major_rev : 4;
> > > +       u32 pad : 24;
> > > +} __packed;
> > > +
> > I am not sure about using bitfields on register values. I know v2
> > driver also uses bitfields but this still is not very portable and is
> > dependent on compiler behaviour. We may actually save some loc by not
> > having unused fields if we use shifts and masks. Though I don't
> > strongly feel either way.
> > 
> 
> Yes, indeed seemed a bit odd way of handling regs when I saw it in mhuv2,
> BUT it seemed it had its advantages in terms of clarity of usage....did
> not know about possible drawbacks, though. I'll re-think about the pros
> and cons of this approach.
>

.replying to myself here...I am dropping bitfields too in favour of
bitmasks...I've read too many horror stories this afternoon about bitfields
and compilers interactions and their flaky usage while handling reg maps to
still feel comfortable using them...

Thanks,
Cristian