On Mon, Mar 13, 2023 at 05:39:04PM -0700, Jakub Kicinski wrote: > On Mon, 13 Mar 2023 11:56:17 -0500 Andrew Halaney wrote: > > EMAC3 is a Qualcomm variant of dwmac4 that functions the same, but has a > > different address space layout for MTL and DMA registers. This makes the > > patch a bit more complicated than we would like so let's explain why the > > current approach was used. > > Please drop all the static inlines in C sources, you're wrapping > a single function call, the compiler will do the right thing. > > Please no more than 6 function arguments. > Thanks for the feedback! With respect to <= 6 function arguments, if I counted right the only violation is this: static void do_config_cbs(struct mac_device_info *hw, u32 send_slope, u32 idle_slope, u32 high_credit, u32 low_credit, u32 queue, u32 etsx_ctrl_base_addr, u32 send_slp_credx_base_addr, u32 high_credx_base_addr, u32 low_credx_base_addr, void (*set_mtl_tx_queue_weight)(struct mac_device_info *hw, u32 weight, u32 queue)) (...) static void emac3_config_cbs(struct mac_device_info *hw, u32 send_slope, u32 idle_slope, u32 high_credit, u32 low_credit, u32 queue) I agree, that's quite gnarly to read. the emac3_config_cbs is the callback, so it's already at 6 arguments, so there's nothing I can trim there. I could create some struct for readability, populate that, then call the do_config_cbs() func with it from emac3_config_cbs. Is that the sort of thing you want to see? Thanks, Andrew
