On Mon, Jul 07, 2025 at 02:00:16PM +0200, Andrew Lunn wrote: > 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: > > > > #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 > Ok, you are right. It should be the right name at the beginning. I will try to avoid this in the future.
