From: Lorenzo Bianconi <lorenzo@kernel.org> To: sean.wang@mediatek.com Cc: lorenzo.bianconi@redhat.com, nbd@nbd.name, Soul.Huang@mediatek.com, YN.Chen@mediatek.com, Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com, Mark-YW.Chen@mediatek.com, Deren.Wu@mediatek.com, km.lin@mediatek.com, robin.chiu@mediatek.com, Eddie.Chen@mediatek.com, ch.yeh@mediatek.com, posh.sun@mediatek.com, ted.huang@mediatek.com, Eric.Liang@mediatek.com, Stella.Chang@mediatek.com, Tom.Chou@mediatek.com, steve.lee@mediatek.com, jsiuda@google.com, frankgor@google.com, jemele@google.com, abhishekpandit@google.com, shawnku@google.com, linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH 2/2] mt76: mt7921s: fix the device cannot sleep deeply in suspend Date: Mon, 22 Nov 2021 19:10:33 +0100 [thread overview] Message-ID: <YZvdGdUYZIIp/LNU@lore-desk> (raw) In-Reply-To: <1637601035-16521-1-git-send-email-sean.wang@mediatek.com> [-- Attachment #1: Type: text/plain, Size: 2610 bytes --] > From: Sean Wang <sean.wang@mediatek.com> > > >> From: Sean Wang <sean.wang@mediatek.com> > >> > >> > >> <snip> > >> > >> >> >> > >> >> >> - if (test_bit(MT76_MCU_RESET, &dev->phy.state)) { > >> >> >> - if (!mt76s_txqs_empty(dev)) > >> >> >> - continue; > >> >> >> - else > >> >> >> - wake_up(&sdio->wait); > >> >> >> - } > >> >> >> } while (nframes > 0); > >> >> >> > >> >> >> + if (test_bit(MT76_MCU_RESET, &dev->phy.state) && > >> >> >> + mt76s_txqs_empty(dev)) > >> >> >> + wake_up(&sdio->wait); > >> >> >> + > >> >> > >> >> If doing so, mt76s_txqs_empty may not always be true because > >> >> enqueuing packets to q_tx or MCU command to q_mcu simultanenously > >> >> from the other contexts in different cpu is possible. > >> >> > >> >> It seemed to me we should check it for each iteration to guarantee > >> >> that we can wake up the one that is waiting for the all the queues are empty at some time. > >> > > >> >IIUC what we are interested here is there are no queued frames into the hw queues during suspend or reset, right? > >> > >> That is not completely true. Take the suspend procedure on mt7921s as an example. > >> > >> That should be "There are no queued frames into the hw queues right after mt76_connac_mcu_set_hif_suspend." > >> > >> The MCU data and WiFi are all handled in mt76s_txrx_worker so we > >> should synchronize all of the Tx queues are all empty and then handle > >> mt76_connac_mcu_set_hif_suspend to guarantee mt76_connac_mcu_set_hif_suspend is the last one to access the SDIO bus and there is no frame that accesses SDIO bus afterhand. > > > >ack, correct, "there are no queued frames into the hw queues right after mt76_connac_mcu_set_hif_suspend." > >What I mean is we are not really checking there are no frames in the hw queue here, but mt76 sdio has processed all the frames, got my point? maybe it is what we are looking for.. > > It seemed to me there is no a way to check if no frames in the hw queue for mt7921s > > I think we can discuss the topic "it is ok move the check out of the loop" with another patch in the future > > because it seemed not related to the patch. sure, fine to me. Regards, Lorenzo > > > > >Regards, > >Lorenzo > > > >> > >> > > >> >> > >> >> >> /* enable interrupt */ > >> >> >> sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL); > >> >> >> sdio_release_host(sdio->func); > >> >> >> > >> >> >> Regards, > >> >> >> Lorenzo > >> >> >> > >> >> >> > -- > >> >> >> > 2.25.1 > >> >> >> > > >> >> > > >> >> > > >> >> > > >> >> > >> > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: Lorenzo Bianconi <lorenzo@kernel.org> To: sean.wang@mediatek.com Cc: lorenzo.bianconi@redhat.com, nbd@nbd.name, Soul.Huang@mediatek.com, YN.Chen@mediatek.com, Leon.Yen@mediatek.com, Eric-SY.Chang@mediatek.com, Mark-YW.Chen@mediatek.com, Deren.Wu@mediatek.com, km.lin@mediatek.com, robin.chiu@mediatek.com, Eddie.Chen@mediatek.com, ch.yeh@mediatek.com, posh.sun@mediatek.com, ted.huang@mediatek.com, Eric.Liang@mediatek.com, Stella.Chang@mediatek.com, Tom.Chou@mediatek.com, steve.lee@mediatek.com, jsiuda@google.com, frankgor@google.com, jemele@google.com, abhishekpandit@google.com, shawnku@google.com, linux-wireless@vger.kernel.org, linux-mediatek@lists.infradead.org Subject: Re: [PATCH 2/2] mt76: mt7921s: fix the device cannot sleep deeply in suspend Date: Mon, 22 Nov 2021 19:10:33 +0100 [thread overview] Message-ID: <YZvdGdUYZIIp/LNU@lore-desk> (raw) In-Reply-To: <1637601035-16521-1-git-send-email-sean.wang@mediatek.com> [-- Attachment #1.1: Type: text/plain, Size: 2610 bytes --] > From: Sean Wang <sean.wang@mediatek.com> > > >> From: Sean Wang <sean.wang@mediatek.com> > >> > >> > >> <snip> > >> > >> >> >> > >> >> >> - if (test_bit(MT76_MCU_RESET, &dev->phy.state)) { > >> >> >> - if (!mt76s_txqs_empty(dev)) > >> >> >> - continue; > >> >> >> - else > >> >> >> - wake_up(&sdio->wait); > >> >> >> - } > >> >> >> } while (nframes > 0); > >> >> >> > >> >> >> + if (test_bit(MT76_MCU_RESET, &dev->phy.state) && > >> >> >> + mt76s_txqs_empty(dev)) > >> >> >> + wake_up(&sdio->wait); > >> >> >> + > >> >> > >> >> If doing so, mt76s_txqs_empty may not always be true because > >> >> enqueuing packets to q_tx or MCU command to q_mcu simultanenously > >> >> from the other contexts in different cpu is possible. > >> >> > >> >> It seemed to me we should check it for each iteration to guarantee > >> >> that we can wake up the one that is waiting for the all the queues are empty at some time. > >> > > >> >IIUC what we are interested here is there are no queued frames into the hw queues during suspend or reset, right? > >> > >> That is not completely true. Take the suspend procedure on mt7921s as an example. > >> > >> That should be "There are no queued frames into the hw queues right after mt76_connac_mcu_set_hif_suspend." > >> > >> The MCU data and WiFi are all handled in mt76s_txrx_worker so we > >> should synchronize all of the Tx queues are all empty and then handle > >> mt76_connac_mcu_set_hif_suspend to guarantee mt76_connac_mcu_set_hif_suspend is the last one to access the SDIO bus and there is no frame that accesses SDIO bus afterhand. > > > >ack, correct, "there are no queued frames into the hw queues right after mt76_connac_mcu_set_hif_suspend." > >What I mean is we are not really checking there are no frames in the hw queue here, but mt76 sdio has processed all the frames, got my point? maybe it is what we are looking for.. > > It seemed to me there is no a way to check if no frames in the hw queue for mt7921s > > I think we can discuss the topic "it is ok move the check out of the loop" with another patch in the future > > because it seemed not related to the patch. sure, fine to me. Regards, Lorenzo > > > > >Regards, > >Lorenzo > > > >> > >> > > >> >> > >> >> >> /* enable interrupt */ > >> >> >> sdio_writel(sdio->func, WHLPCR_INT_EN_SET, MCR_WHLPCR, NULL); > >> >> >> sdio_release_host(sdio->func); > >> >> >> > >> >> >> Regards, > >> >> >> Lorenzo > >> >> >> > >> >> >> > -- > >> >> >> > 2.25.1 > >> >> >> > > >> >> > > >> >> > > >> >> > > >> >> > >> > [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 170 bytes --] _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek
next prev parent reply other threads:[~2021-11-22 18:10 UTC|newest] Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <YWcuGcFPGCtaPh+2@lore-desk--annotate> 2021-10-13 21:22 ` [PATCH v4 11/16] mt76: sdio: extend sdio module to support CONNAC2 sean.wang 2021-10-13 21:22 ` sean.wang 2021-11-22 17:10 ` [PATCH 2/2] mt76: mt7921s: fix the device cannot sleep deeply in suspend sean.wang 2021-11-22 17:10 ` sean.wang 2021-11-22 18:10 ` Lorenzo Bianconi [this message] 2021-11-22 18:10 ` Lorenzo Bianconi [not found] <YZq9cBbzwtvzEKyN@lore-desk--annotate> 2021-11-22 3:46 ` sean.wang 2021-11-22 3:46 ` sean.wang 2021-11-22 10:49 ` Lorenzo Bianconi 2021-11-22 10:49 ` Lorenzo Bianconi [not found] <YZkDmDIqnePjIF+O@lore-desk--annotate> 2021-11-20 20:29 ` sean.wang 2021-11-20 20:29 ` sean.wang 2021-11-21 21:43 ` Lorenzo Bianconi 2021-11-21 21:43 ` Lorenzo Bianconi 2021-11-19 23:22 [PATCH 1/2] mt76: mt7921: move mt76_connac_mcu_set_hif_suspend to bus-related files sean.wang 2021-11-19 23:22 ` [PATCH 2/2] mt76: mt7921s: fix the device cannot sleep deeply in suspend sean.wang 2021-11-19 23:22 ` sean.wang 2021-11-20 13:00 ` Lorenzo Bianconi 2021-11-20 13:00 ` Lorenzo Bianconi 2021-11-20 14:18 ` Lorenzo Bianconi 2021-11-20 14:18 ` Lorenzo Bianconi
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=YZvdGdUYZIIp/LNU@lore-desk \ --to=lorenzo@kernel.org \ --cc=Deren.Wu@mediatek.com \ --cc=Eddie.Chen@mediatek.com \ --cc=Eric-SY.Chang@mediatek.com \ --cc=Eric.Liang@mediatek.com \ --cc=Leon.Yen@mediatek.com \ --cc=Mark-YW.Chen@mediatek.com \ --cc=Soul.Huang@mediatek.com \ --cc=Stella.Chang@mediatek.com \ --cc=Tom.Chou@mediatek.com \ --cc=YN.Chen@mediatek.com \ --cc=abhishekpandit@google.com \ --cc=ch.yeh@mediatek.com \ --cc=frankgor@google.com \ --cc=jemele@google.com \ --cc=jsiuda@google.com \ --cc=km.lin@mediatek.com \ --cc=linux-mediatek@lists.infradead.org \ --cc=linux-wireless@vger.kernel.org \ --cc=lorenzo.bianconi@redhat.com \ --cc=nbd@nbd.name \ --cc=posh.sun@mediatek.com \ --cc=robin.chiu@mediatek.com \ --cc=sean.wang@mediatek.com \ --cc=shawnku@google.com \ --cc=steve.lee@mediatek.com \ --cc=ted.huang@mediatek.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.