Re: [PATCH 03/15] net: rnpgbe: Add basic mbx ops support

On Mon, Jul 07, 2025 at 02:39:55PM +0800, Yibo Dong wrote:

On Fri, Jul 04, 2025 at 08:13:19PM +0200, Andrew Lunn wrote:

quoted

quoted

 #define MBX_FEATURE_WRITE_DELAY BIT(1)
 	u32 mbx_feature;
 	/* cm3 <-> pf mbx */
-	u32 cpu_pf_shm_base;
-	u32 pf2cpu_mbox_ctrl;
-	u32 pf2cpu_mbox_mask;
-	u32 cpu_pf_mbox_mask;
-	u32 cpu2pf_mbox_vec;
+	u32 fw_pf_shm_base;
+	u32 pf2fw_mbox_ctrl;
+	u32 pf2fw_mbox_mask;
+	u32 fw_pf_mbox_mask;
+	u32 fw2pf_mbox_vec;

Why is a patch adding a new feature deleting code?

Not delete code, 'cpu' here means controller in the chip, not host.
So, I just rename 'cpu' to 'fw' to avoid confusion.

So, so let me rephrase my point. Why was it not called fw_foo right
from the beginning? You are making the code harder to review by doing
stuff like this. And your code is going to need a lot of review and
revisions because its quality if low if you ask me.

	Andrew