Thread (35 messages) 35 messages, 3 authors, 2025-08-18

Re: [PATCH v4 3/5] net: rnpgbe: Add basic mbx ops support

From: Yibo Dong <dong100@mucse.com>
Date: 2025-08-15 03:16:22
Also in: linux-doc, lkml

On Fri, Aug 15, 2025 at 04:29:28AM +0200, Andrew Lunn wrote:
quoted
+#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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help