(Was "Re: [PATCH v3 00/19] rtlwifi: halmac: Add new module halmac", changing the title to reflect what we are discussing) Pkshih <pkshih@xxxxxxxxxxx> writes: > On Thu, 2018-05-24 at 11:27 +0300, Kalle Valo wrote: >> >> You are missing my point: I don't even have time to review huge rtlwifi >> patches when they are not even ready for upstream. I cannot start >> working on cleaning up rtlwifi code and doing multiple iterations of >> reviews on these kind of huge patchsets. Either you need to >> significantly scale down the size of patchsets (especially LOC) or you >> need to get review help from someone else. But the current way of >> working is not doable for me. >> > > Is there a proper way to look for "someone else" you mentioned? I don't know, I think there might a project somewhere which helps with patch review for new people but not sure about that. Adding Dan in case he has some ideas. > We plan to rewrite a new driver excluding agnostic OS layer to support > new generation 11AC chips, because they're very different from the chips > existed in rtlwifi and rtl8xxxu. > > If we have a "someone" to review our driver, where is the proper place to > put developing driver repository? Staging or public git repository > (e.g. GitHub)? It depends on the driver really. If it's a good (code) quality driver following upstream rules, then taking it directly to drivers/net/wireless is the best approach. That's what we did with qtnfmac for example. But if the driver is more like a usual vendor driver with horrible code, and not following upstream rules, then other options are better. I don't look at staging at all so I can't comment staging vs github.com, others more knowledgeable can comment about that. > Finally, the driver is done. Are there explicit criteria to accept the > driver as a mainline driver? You mean like written rules? I don't think that I have seen anything. But here are some things I usually check when reviewing patches for upstream: * good quality, simple, self-documenting and readable code (no magic values, ugly hacks) * in general follows Linux coding style (not every whitespace needs to be correct but most of the rules need to be followed, no CamelCode etc) * clean and simple design (no unnecessary layers and such) * respects cfg80211 and mac80211 designs (doesn't reinvent the wheel, functionality which should be in mac80211 or cfg80211 is not in the driver) * user space interfaces follow the rules * commit log answers to "Why?" and doesn't leave questions unanswered This is not a comprehensive list but hopefully still helps you to give an idea what kind of things we are looking for. I'm sure there are more tips elsewhere. And as the last tip I want to give that try to submit the driver as simple as possible (=small), once it's accepted you can start adding more features on top. In other words: "Submit early, submit often" https://en.wikipedia.org/wiki/Release_early,_release_often -- Kalle Valo
