Daniel Mack <daniel@xxxxxxxxxx> writes: > 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." Looks good, thanks. I see that you submitted v2 already with this added. -- Kalle Valo
