SGkgS2FsbGUsDQo+DQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gR2FuYXBhdGhpIEJoYXQgPGdiaGF0QG1h
cnZlbGwuY29tPiB3cm90ZToNCj4NCj4gPiBGcm9tOiBTaHJlbmlrIFNoaWtoYXJlIDxzaHJlbmlr
QG1hcnZlbGwuY29tPg0KPiA+DQo+ID4gVGhlcmUgaXMgYSByYWNlIGNvbmRpdGlvbiBmb3IgYWNx
dWlyaW5nIHJ4X3Byb2NfbG9jayBiZXR3ZWVuIHJ4IHdvcmtlcg0KPiA+IHRocmVhZCBhbmQgVVNC
IFJYIGRhdGEgaW50ZXJydXB0DQo+ID4gKG13aWZpZXhfdXNiX3J4X2NvbXBsZXRlKToNCj4gPg0K
PiA+IDEuIFVTQiByZWNlaXZlcyBhbiBSWCBkYXRhIGludGVycnVwdCwgcXVldWVzIHJ4X3dvcmsg
Mi4gcnhfd29yaw0KPiA+IGVtcHRpZXMgcnhfZGF0YV9xLCB0cmllcyB0byBhY3F1aXJlIHJ4X3By
b2NfbG9jayAodG8gY2xlYXINCj4gPiByeF9wcm9jZXNzaW5nIGZsYWcpIDMuIFdoaWxlICMyIGlz
IHlldCB0byBhY3F1aXJlIHJ4X3Byb2NfbG9jaywgZHJpdmVyDQo+ID4gcmVjZWl2ZXMgY29udGlu
dW91cyBSWCBkYXRhIGludGVydXB0cyhtd2lmaWV4X3VzYl9yeF9jb21wbGV0ZSkNCj4gPiAzLiBG
b3IgZWFjaCBpbnRlcnJ1cHQgYXQgIzMsIGRyaXZlciBhY3F1aXJlcyByeF9wcm9jX2xvY2soaXQg
Z2V0cyB0aGUNCj4gPiBsb2NrIHNpbmNlIGl0IGlzIGluIGludGVycnVwdCBjb250ZXh0KSwgdHJp
ZXMgdG8gcXVldWUgcnhfd29yaywgYnV0DQo+ID4gZmFpbHMgdG8gZG8gc28gc2luY2UgcnhfcHJv
Y2Vzc2luZyBpcyBzdGlsbCBzZXQoIzIpIDQuIFdoZW4gcnhfcGVuZGluZw0KPiA+IGV4Y2VlZHMg
SElHSF9SWF9QRU5ESU5HLCBkcml2ZXIgc3RvcHMgc3VibWl0dGluZyBVUkJzIGJhY2sgdG8gVVNC
DQo+ID4gc3Vic3lzdGVtIGFuZCB0aHVzIGZpcm13YXJlIHN0b3BzIHVwbG9hZGluZyBSWCBkYXRh
IHRvIGRyaXZlciA1LiBOb3cNCj4gPiBmaW5hbGx5ICMyIHdpbGwgYWNxdWlyZSByeF9wcm9jX2xv
Y2ssIGJ1dCBiZWNhdXNlIG9mICM0LCB0aGVyZSBhcmUgbm8NCj4gPiBmdXJ0aGVyIHRyaWdnZXJz
IHRvIHNjaGVkdWxlIHJ4X3dvcmsgYWdhaW4NCj4gPg0KPiA+IFRoZSBhYm92ZSBzY2VuYXJpbyBv
Y2N1cnMgaW4gc29tZSBwbGF0Zm9ybXMgd2hlcmUgdGhlIFJYIHByb2Nlc3NpbmcgaXMNCj4gPiBj
b21wYXJpdGl2ZWx5IHNsb3dlci4gVGhpcyByZXN1bHRzIGluIFJYIHN0YWxsIGluIGRyaXZlciwg
Y29tbWFuZC9UWA0KPiA+IHRpbWVvdXRzIGluIGZpcm13YXJlLiBUaGUgYWJvdmUgc2NlbmFyaW8g
aXMgaW50cm9kdWNlZCBhZnRlciBjb21taXQNCj4gPiBjN2RiZGNiMmE0ZTENCj4gPiAoIm13aWZp
ZXg6IHNjaGVkdWxlIHJ4X3dvcmsgb24gUlggaW50ZXJydXB0IGZvciBVU0IiKQ0KPiA+DQo+ID4g
VG8gZml4IHRoaXMgc2V0IGEgbmV3IG1vcmVfcnhfdGFza19mbGFnIHdoZW5ldmVyIFJYIGRhdGEg
Y2FsbGJhY2sgaXMNCj4gPiB0cnlpbmcgdG8gc2NoZWR1bGUgcnhfd29yayBidXQgcnhfcHJvY2Vz
c2luZyBpcyBub3QgeWV0IGNsZWFyZWQuIFRoaXMNCj4gPiB3aWxsIGxldCB0aGUgY3VycmVudCBy
eF93b3JrKHdoaWNoIHdhcyB3YWl0aW5nIGZvcg0KPiA+IHJ4X3Byb2NfbG9jaykgdG8gbG9vcGJh
Y2sgYW5kIHByb2Nlc3MgbmV3bHkgYXJyaXZlZCBSWCBwYWNrZXRzLg0KPiA+DQo+ID4gU2lnbmVk
LW9mZi1ieTogQ2F0aHkgTHVvIDxjbHVvQG1hcnZlbGwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6
IEdhbmFwYXRoaSBCaGF0IDxnYmhhdEBtYXJ2ZWxsLmNvbT4NCj4NCj4gSSBjYW4ndCBmaW5kIGFu
eSBjb21taXQgd2l0aCBpZCBjN2RiZGNiMmE0ZTEsIGlzIGl0IGNvcnJlY3Q/DQpDb3JyZWN0LiBB
Y3R1YWxseSB0aGUgY29tbWl0IGlkIGM3ZGJkY2IyYTRlMSBpcyB0aGUgUEFUQ0ggWzEvMl0gc2Vu
dCBpbiB0aGlzIHNlcmllcy4NCj4NCj4gLS0NCj4gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9y
Zy9wYXRjaC8xMDE3ODcyOS8NCj4NCj4gaHR0cHM6Ly93aXJlbGVzcy53aWtpLmtlcm5lbC5vcmcv
ZW4vZGV2ZWxvcGVycy9kb2N1bWVudGF0aW9uL3N1Ym1pdHRpbmdwDQo+IGF0Y2hlcw0KUmVnYXJk
cywNCkdhbmFwYXRoaQ0K
On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
> > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this series.
What? Why would you introduce a bug and only fix it in the next patch?
Does that mean you should just combine the two? Or reverse the order, if
patch 2 doesn't cause problems on its own?
Brian
Ganapathi Bhat <[email protected]> writes:
>> > The above scenario occurs in some platforms where the RX processing is
>> > comparitively slower. This results in RX stall in driver, command/TX
>> > timeouts in firmware. The above scenario is introduced after commit
>> > c7dbdcb2a4e1
>> > ("mwifiex: schedule rx_work on RX interrupt for USB")
>> >
>> > To fix this set a new more_rx_task_flag whenever RX data callback is
>> > trying to schedule rx_work but rx_processing is not yet cleared. This
>> > will let the current rx_work(which was waiting for
>> > rx_proc_lock) to loopback and process newly arrived RX packets.
>> >
>> > Signed-off-by: Cathy Luo <[email protected]>
>> > Signed-off-by: Ganapathi Bhat <[email protected]>
>>
>> I can't find any commit with id c7dbdcb2a4e1, is it correct?
>
> Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this series.
Actually the commit id will be different, I just tested it to be sure:
$ git reset --hard master
HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o errors
$ git am -s -3 1.patch
Applying: mwifiex: schedule rx_work on RX interrupt for USB
$ git log --oneline -1 | cat
676bc4833907 mwifiex: schedule rx_work on RX interrupt for USB
$ git reset --hard master
HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o errors
$ git am -s -3 1.patch
Applying: mwifiex: schedule rx_work on RX interrupt for USB
$ git log --oneline -1 | cat
74c5fc1d45b4 mwifiex: schedule rx_work on RX interrupt for USB
$
So the date, and most likely also the commiter, is included when
calculating the hash. So you can't really refer to uncommited patches
using a commit id as the id is determined only once the maintainer
applies the patch.
--
Kalle Valo
Hi Kalle,
> -----Original Message-----
> From: Kalle Valo [mailto:[email protected]]
> Sent: Thursday, January 25, 2018 6:29 PM
> To: Ganapathi Bhat
> Cc: [email protected]; Brian Norris; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> Ganapathi Bhat <[email protected]> writes:
>
> >> > The above scenario occurs in some platforms where the RX processing
> >> > is comparitively slower. This results in RX stall in driver,
> >> > command/TX timeouts in firmware. The above scenario is introduced
> >> > after commit
> >> > c7dbdcb2a4e1
> >> > ("mwifiex: schedule rx_work on RX interrupt for USB")
> >> >
> >> > To fix this set a new more_rx_task_flag whenever RX data callback
> >> > is trying to schedule rx_work but rx_processing is not yet cleared.
> >> > This will let the current rx_work(which was waiting for
> >> > rx_proc_lock) to loopback and process newly arrived RX packets.
> >> >
> >> > Signed-off-by: Cathy Luo <[email protected]>
> >> > Signed-off-by: Ganapathi Bhat <[email protected]>
> >>
> >> I can't find any commit with id c7dbdcb2a4e1, is it correct?
> >
> > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this
> series.
>
> Actually the commit id will be different, I just tested it to be sure:
>
> $ git reset --hard master
> HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o
> errors $ git am -s -3 1.patch
> Applying: mwifiex: schedule rx_work on RX interrupt for USB $ git log --
> oneline -1 | cat
> 676bc4833907 mwifiex: schedule rx_work on RX interrupt for USB $ git reset -
> -hard master HEAD is now at b69c1df47281 brcmfmac: separate firmware
> errors from i/o errors $ git am -s -3 1.patch
> Applying: mwifiex: schedule rx_work on RX interrupt for USB $ git log --
> oneline -1 | cat
> 74c5fc1d45b4 mwifiex: schedule rx_work on RX interrupt for USB $
>
> So the date, and most likely also the commiter, is included when calculating
> the hash. So you can't really refer to uncommited patches using a commit id
> as the id is determined only once the maintainer applies the patch.
Ok. So, what can be done in this case. Should we wait for 1st patch to be committed and then send v3 of second patch?
>
> --
> Kalle Valo
Regards,
Ganapathi
Hi Brian,
> -----Original Message-----
> From: Brian Norris [mailto:[email protected]]
> Sent: Friday, January 26, 2018 12:04 AM
> To: Ganapathi Bhat
> Cc: Kalle Valo; [email protected]; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
> > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this
> series.
>
> What? Why would you introduce a bug and only fix it in the next patch?
With the first patch the original issue took considerably longer time to recreate. Also it followed a different path to get recreated(shared in commit message).
> Does that mean you should just combine the two?
Let us know if that is fine to merge them. We can modify the commit message accordingly.
> Or reverse the order, if patch 2 doesn't cause problems on its own?
Patch 2 has a dependency on patch 1.
>
> Brian
Regards,
Ganapathi