Hi Viktor, > The RFC is divided into separate patches on a per-file basis to simplify > the review process. Wow, umm, seems big! LOC maybe not, but # of files ... not that easy to review. I've only looked a little while now, but for making this easier to review it would be nice to split it into a "minimum viable product" first, i.e. have a much smaller driver, and this is probably not an exhaustive list: * without command line interfaces with string parsing built into the kernel (OK, that's probably something that will never go upstream anyway) * Kconfig vs. code inconsistencies cleaned up, you have a TON of CONFIG_* ifdefs that don't even exist and can never be set, so remove the code, at least for now? * remove wrappers like cl_snprintf(), though that seems part of all those command line interfaces built into the driver * consider joining some of the many header files into bigger chunks, header files that are 50% boilerplate aren't really all that useful * namespace your things better - e.g. "is_ipv4_packet" and friends? (why do you even care?) * remove all vendor commands for now, and read the vendor commands upstreaming documentation before re-introducing them https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api * remove utils/string.c, obviously, use kernel stuff directly or improve where needed I guess - but likely all part of the weird CLI- in-kernel stuff? * remove abstraction layers like cl_timer * what's this VNS stuff? doesn't look like it belongs into a driver (a mac80211 one at that!) * bitfields in endian-safe code are generally frowned upon, so you shouldn't need cl_are_host_bits_le(). And also cl_are_host_bits_be()? What? * Use ERR_PTR() and friends, instead of out pointer parameters. You'll probably also notice a lot of issues yourself if you take a step back and actually read your patches, rather than just the code they were generated from, e.g. the Kconfig confusion I mentioned below. Thanks, johannes
