On Fri, Aug 15, 2025 at 04:29:28AM +0200, Andrew Lunn wrote:
> > +#define MUCSE_MAILBOX_WORDS 14
> > +#define MUCSE_FW_MAILBOX_WORDS MUCSE_MAILBOX_WORDS
> > +#define FW_PF_SHM(mbx) ((mbx)->fw_pf_shm_base)
> > +#define FW2PF_COUNTER(mbx) (FW_PF_SHM(mbx) + 0)
> > +#define PF2FW_COUNTER(mbx) (FW_PF_SHM(mbx) + 4)
> > +#define FW_PF_SHM_DATA(mbx) (FW_PF_SHM(mbx) + 8)
>
> There seems to be quite a bit of obfuscation here. Why is both
> MUCSE_MAILBOX_WORDS and MUCSE_FW_MAILBOX_WORDS needed?
>
I will remove MUCSE_MAILBOX_WORDS.
> Why not
>
> #define FW2PF_COUNTER(mbx) (mbx->fw_pf_shm_base + 0)
>
> Or even better
>
> #define MBX_FW2PF_COUNTER 0
> #define MBX_W2PF_COUNTER 4
> #define MBX_FW_PF_SHM_DATA 8
>
> static u32 mbx_rd32(struct mbx *mbx, int reg) {
>
> return readl(mbx->hw->hw_addr + reg);
> }
>
> u32 val = mbx_rd32(mbx, MBX_FW2PF_COUNTER);
>
> Look at what other drivers do. They are much more likely to define a
> set of offset from the base address, and let the read/write helper do
> the addition to the base.
>
> Andrew
>
Ok, I will use 'static u32 mbx_rd32(struct mbx *mbx, int reg)' way.
Thanks for your feedback.
