> On Tue, Feb 25, 2025 at 04:53:30PM +0200, Gur Stavi wrote: > > From: Fan Gong <gongfan1@xxxxxxxxxx> > > > > This is [1/3] part of hinic3 Ethernet driver initial submission. > > With this patch hinic3 is a valid kernel module but non-functional > > driver. > > IMHO, there's a huge amount of code so it makes reviewing pretty > difficult. > > Is there no way to split this into multiple smaller patches? I am > sure his was asked and answered in a previous thread that I missed. > > I took a quick pass over the code, but probably missed many things > due to the large amount of code in a single patch. > > Doesn't this function need to re-enable hw IRQs? Maybe it does > somewhere in one of the helpers and I missed it? > > Even so, it should probably be checking napi_complete_done before > re-enabling IRQs and I don't see a call to that anywhere, but maybe > I missed it? > > I also don't see any calls to netif_napi_add, so I'm not sure if > this code needs to be included in this patch ? > Hi Joe, thanks for the review. There was a discussion during our initial submission where we were required to limit submission size to 4k LoC. https://lore.kernel.org/netdev/cover.1730290527.git.gur.stavi@xxxxxxxxxx After removing almost anything optional from the driver we reached ~12K LoC and broke it into 3 submissions of 4K LoC that compile. This is the 1st submission. The rational for patch ordering was to start with the code that have lots of interaction with Linux interfaces and therefore is of more interest to the community and keep the code that deals with proprietary HW interfaces for later. We also try to make the patches incremental and avoid introducing ad-hoc code that is being deleted or modified in later submission. The actual calls to rx/tx poll and napi require IRQs that require hw management interfaces. I can try to pull these functions from later submission (without breaking the 4K limit) into this patch. They are currently static functions so I will have to un-static them to avoid compilation errors and then re-static them at a later submission. As for breaking the 4K submission into smaller patches, I will try to find some finer granularity that makes sense. When I studied the mailing list, there were conflicting opinions about the right way for initial driver submission. For example: https://lore.kernel.org/netdev/20170817.220343.905568389038615738.davem@xxxxxxxxxxxxx
