On Monday, April 16, 2018 04:41 PM, Kalle Valo wrote: > Daniel Mack <daniel@xxxxxxxxxx> writes: > >> On Monday, April 16, 2018 04:13 PM, Kalle Valo wrote: >>> Daniel Mack <daniel@xxxxxxxxxx> writes: >>>> On Monday, April 16, 2018 04:03 PM, Kalle Valo wrote: >>>>> Daniel Mack <daniel@xxxxxxxxxx> writes: >> >>>>>> them to the firmware message. The driver currently tells the core that >>>>>> it is capable of attaching up to WCN36XX_MAX_SCAN_IE_LEN octets, but >>>>>> doesn't actually pass them to the the hardware. >>>>>> >>>>>> Some defines were moved around to avoid cyclic include dependencies. >>>>> >>>>> Does this fix anything or change functionality somehow? You should >>>>> document that also in the commit log. >>>> >>>> I don't have a test case for this, no. But as the change was pretty much >>>> straight forward, I sent it anyway. >>> >>> Ok, but you should still explain that in the commit log. In other words, >>> you should always answer the question "Why?" in the commit log. >>> >>>> I can resend with some more information on this if you like. >>> >>> No need to resend because of this, I can edit the commit log. >> >> Hmm, given that I can't even be sure the firmware does the right thing >> when instructed this way, we should probably just drop this patch from >> the series. The others are more important anyway, as they address bugs >> that hit me on actual hardware. > > IMHO it's useful and if you don't see anything breaking I would prefer > to take it anyway. I can't immeadiately say in what use cases this helps > or fixes, though. But maybe you (or someone else) could verify that it > works correctly by comparing before and after probe requests with a > sniffer? It's certainly not a regression, no. I'm running extensive stress-tests with this driver. Then I guess adding the following to the commit log should suffice? "Note that this patch doesn't fix a bug that was observed. The change is merely done for the sake of completeness as the hardware supports appending IEs in scans. Tests show that network scans work fine with this patch applied." Thanks, Daniel
