* [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag
@ 2021-06-17 18:23 Bhaumik Bhatt
2021-06-17 18:52 ` Loic Poulain
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Bhaumik Bhatt @ 2021-06-17 18:23 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, carl.yin,
naveen.kumar, loic.poulain, Bhaumik Bhatt
Devices such as SDX24 do not have the provision for inband wake
doorbell in the form of channel 127. Newer devices such as SDX55
or SDX65 have it by default. Ensure the functionality is used
based on this such that device wake stays held when a client
driver uses mhi_device_get() API or the equivalent debugfs entry.
Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake operations")
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
---
drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++--------
1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
index d84b743..31360a2 100644
--- a/drivers/bus/mhi/pci_generic.c
+++ b/drivers/bus/mhi/pci_generic.c
@@ -32,6 +32,7 @@
* @edl: emergency download mode firmware path (if any)
* @bar_num: PCI base address register to use for MHI MMIO register space
* @dma_data_width: DMA transfer word size (32 or 64 bits)
+ * @no_inband_wake: Devices without inband wake support (such as sdx24)
*/
struct mhi_pci_dev_info {
const struct mhi_controller_config *config;
@@ -40,6 +41,7 @@ struct mhi_pci_dev_info {
const char *edl;
unsigned int bar_num;
unsigned int dma_data_width;
+ bool no_inband_wake;
};
#define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \
@@ -242,7 +244,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = {
.edl = "qcom/sdx65m/edl.mbn",
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
- .dma_data_width = 32
+ .dma_data_width = 32,
+ .no_inband_wake = false
};
static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
@@ -251,7 +254,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = {
.edl = "qcom/sdx55m/edl.mbn",
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
- .dma_data_width = 32
+ .dma_data_width = 32,
+ .no_inband_wake = false
};
static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = {
@@ -259,7 +263,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = {
.edl = "qcom/prog_firehose_sdx24.mbn",
.config = &modem_qcom_v1_mhiv_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
- .dma_data_width = 32
+ .dma_data_width = 32,
+ .no_inband_wake = true
};
static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = {
@@ -301,7 +306,8 @@ static const struct mhi_pci_dev_info mhi_quectel_em1xx_info = {
.edl = "qcom/prog_firehose_sdx24.mbn",
.config = &modem_quectel_em1xx_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
- .dma_data_width = 32
+ .dma_data_width = 32,
+ .no_inband_wake = true
};
static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = {
@@ -339,7 +345,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = {
.edl = "qcom/sdx55m/edl.mbn",
.config = &modem_foxconn_sdx55_config,
.bar_num = MHI_PCI_DEFAULT_BAR_NUM,
- .dma_data_width = 32
+ .dma_data_width = 32,
+ .no_inband_wake = false
};
static const struct pci_device_id mhi_pci_id_table[] = {
@@ -640,9 +647,12 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
mhi_cntrl->status_cb = mhi_pci_status_cb;
mhi_cntrl->runtime_get = mhi_pci_runtime_get;
mhi_cntrl->runtime_put = mhi_pci_runtime_put;
- mhi_cntrl->wake_get = mhi_pci_wake_get_nop;
- mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
- mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop;
+
+ if (info->no_inband_wake) {
+ mhi_cntrl->wake_get = mhi_pci_wake_get_nop;
+ mhi_cntrl->wake_put = mhi_pci_wake_put_nop;
+ mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop;
+ }
err = mhi_pci_claim(mhi_cntrl, info->bar_num, DMA_BIT_MASK(info->dma_data_width));
if (err)
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-17 18:23 [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag Bhaumik Bhatt @ 2021-06-17 18:52 ` Loic Poulain 2021-06-17 21:58 ` Hemant Kumar 2021-06-18 6:52 ` Manivannan Sadhasivam 2 siblings, 0 replies; 7+ messages in thread From: Loic Poulain @ 2021-06-17 18:52 UTC (permalink / raw) To: Bhaumik Bhatt Cc: Manivannan Sadhasivam, linux-arm-msm, Hemant Kumar, Jeffrey Hugo, open list, Carl Yin(殷张成), Naveen Kumar On Thu, 17 Jun 2021 at 20:24, Bhaumik Bhatt <bbhatt@codeaurora.org> wrote: > > Devices such as SDX24 do not have the provision for inband wake > doorbell in the form of channel 127. Newer devices such as SDX55 > or SDX65 have it by default. Ensure the functionality is used > based on this such that device wake stays held when a client > driver uses mhi_device_get() API or the equivalent debugfs entry. > > Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake operations") > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> Reviewed-by: Loic Poulain <loic.poulain@linaro.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-17 18:23 [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag Bhaumik Bhatt 2021-06-17 18:52 ` Loic Poulain @ 2021-06-17 21:58 ` Hemant Kumar 2021-06-18 6:52 ` Manivannan Sadhasivam 2 siblings, 0 replies; 7+ messages in thread From: Hemant Kumar @ 2021-06-17 21:58 UTC (permalink / raw) To: Bhaumik Bhatt, manivannan.sadhasivam Cc: linux-arm-msm, jhugo, linux-kernel, carl.yin, naveen.kumar, loic.poulain On Thu, 2021-06-17 at 11:23 -0700, Bhaumik Bhatt wrote: > Devices such as SDX24 do not have the provision for inband wake > doorbell in the form of channel 127. Newer devices such as SDX55 > or SDX65 have it by default. Ensure the functionality is used > based on this such that device wake stays held when a client > driver uses mhi_device_get() API or the equivalent debugfs entry. > > Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake > operations") > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-17 18:23 [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag Bhaumik Bhatt 2021-06-17 18:52 ` Loic Poulain 2021-06-17 21:58 ` Hemant Kumar @ 2021-06-18 6:52 ` Manivannan Sadhasivam 2021-06-18 17:06 ` Bhaumik Bhatt 2 siblings, 1 reply; 7+ messages in thread From: Manivannan Sadhasivam @ 2021-06-18 6:52 UTC (permalink / raw) To: Bhaumik Bhatt Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, carl.yin, naveen.kumar, loic.poulain On Thu, Jun 17, 2021 at 11:23:53AM -0700, Bhaumik Bhatt wrote: > Devices such as SDX24 do not have the provision for inband wake > doorbell in the form of channel 127. Newer devices such as SDX55 > or SDX65 have it by default. Ensure the functionality is used > based on this such that device wake stays held when a client > driver uses mhi_device_get() API or the equivalent debugfs entry. > > Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake operations") > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> > --- > drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++-------- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c > index d84b743..31360a2 100644 > --- a/drivers/bus/mhi/pci_generic.c > +++ b/drivers/bus/mhi/pci_generic.c > @@ -32,6 +32,7 @@ > * @edl: emergency download mode firmware path (if any) > * @bar_num: PCI base address register to use for MHI MMIO register space > * @dma_data_width: DMA transfer word size (32 or 64 bits) > + * @no_inband_wake: Devices without inband wake support (such as sdx24) I'd rather like this field to be "inband_wake" and set to false/true based on the capability of the devices. Rest looks good. Thanks, Mani > */ > struct mhi_pci_dev_info { > const struct mhi_controller_config *config; > @@ -40,6 +41,7 @@ struct mhi_pci_dev_info { > const char *edl; > unsigned int bar_num; > unsigned int dma_data_width; > + bool no_inband_wake; > }; > > #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \ > @@ -242,7 +244,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx65_info = { > .edl = "qcom/sdx65m/edl.mbn", > .config = &modem_qcom_v1_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > - .dma_data_width = 32 > + .dma_data_width = 32, > + .no_inband_wake = false > }; > > static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { > @@ -251,7 +254,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { > .edl = "qcom/sdx55m/edl.mbn", > .config = &modem_qcom_v1_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > - .dma_data_width = 32 > + .dma_data_width = 32, > + .no_inband_wake = false > }; > > static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = { > @@ -259,7 +263,8 @@ static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = { > .edl = "qcom/prog_firehose_sdx24.mbn", > .config = &modem_qcom_v1_mhiv_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > - .dma_data_width = 32 > + .dma_data_width = 32, > + .no_inband_wake = true > }; > > static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = { > @@ -301,7 +306,8 @@ static const struct mhi_pci_dev_info mhi_quectel_em1xx_info = { > .edl = "qcom/prog_firehose_sdx24.mbn", > .config = &modem_quectel_em1xx_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > - .dma_data_width = 32 > + .dma_data_width = 32, > + .no_inband_wake = true > }; > > static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = { > @@ -339,7 +345,8 @@ static const struct mhi_pci_dev_info mhi_foxconn_sdx55_info = { > .edl = "qcom/sdx55m/edl.mbn", > .config = &modem_foxconn_sdx55_config, > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > - .dma_data_width = 32 > + .dma_data_width = 32, > + .no_inband_wake = false > }; > > static const struct pci_device_id mhi_pci_id_table[] = { > @@ -640,9 +647,12 @@ static int mhi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) > mhi_cntrl->status_cb = mhi_pci_status_cb; > mhi_cntrl->runtime_get = mhi_pci_runtime_get; > mhi_cntrl->runtime_put = mhi_pci_runtime_put; > - mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > - mhi_cntrl->wake_put = mhi_pci_wake_put_nop; > - mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; > + > + if (info->no_inband_wake) { > + mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > + mhi_cntrl->wake_put = mhi_pci_wake_put_nop; > + mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; > + } > > err = mhi_pci_claim(mhi_cntrl, info->bar_num, DMA_BIT_MASK(info->dma_data_width)); > if (err) > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-18 6:52 ` Manivannan Sadhasivam @ 2021-06-18 17:06 ` Bhaumik Bhatt 2021-06-18 17:27 ` Manivannan Sadhasivam 0 siblings, 1 reply; 7+ messages in thread From: Bhaumik Bhatt @ 2021-06-18 17:06 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, carl.yin, naveen.kumar, loic.poulain Hi Mani, On 2021-06-17 11:52 PM, Manivannan Sadhasivam wrote: > On Thu, Jun 17, 2021 at 11:23:53AM -0700, Bhaumik Bhatt wrote: >> Devices such as SDX24 do not have the provision for inband wake >> doorbell in the form of channel 127. Newer devices such as SDX55 >> or SDX65 have it by default. Ensure the functionality is used >> based on this such that device wake stays held when a client >> driver uses mhi_device_get() API or the equivalent debugfs entry. >> >> Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake >> operations") >> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> >> --- >> drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++-------- >> 1 file changed, 18 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/bus/mhi/pci_generic.c >> b/drivers/bus/mhi/pci_generic.c >> index d84b743..31360a2 100644 >> --- a/drivers/bus/mhi/pci_generic.c >> +++ b/drivers/bus/mhi/pci_generic.c >> @@ -32,6 +32,7 @@ >> * @edl: emergency download mode firmware path (if any) >> * @bar_num: PCI base address register to use for MHI MMIO register >> space >> * @dma_data_width: DMA transfer word size (32 or 64 bits) >> + * @no_inband_wake: Devices without inband wake support (such as >> sdx24) > > I'd rather like this field to be "inband_wake" and set to false/true > based on the capability of the devices. Rest looks good. > > Thanks, > Mani > I should have known this was coming :) Can I use sideband_wake instead of no_inband_wake and leave the booleans as is? By default, inband_wake will have to be true for any and all devices moving forward. Please let me know your preference. >> */ >> struct mhi_pci_dev_info { >> const struct mhi_controller_config *config; >> @@ -40,6 +41,7 @@ struct mhi_pci_dev_info { >> const char *edl; >> unsigned int bar_num; >> unsigned int dma_data_width; >> + bool no_inband_wake; >> }; >> >> #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \ >> @@ -242,7 +244,8 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx65_info = { >> .edl = "qcom/sdx65m/edl.mbn", >> .config = &modem_qcom_v1_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> - .dma_data_width = 32 >> + .dma_data_width = 32, >> + .no_inband_wake = false >> }; >> >> static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { >> @@ -251,7 +254,8 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx55_info = { >> .edl = "qcom/sdx55m/edl.mbn", >> .config = &modem_qcom_v1_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> - .dma_data_width = 32 >> + .dma_data_width = 32, >> + .no_inband_wake = false >> }; >> >> static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = { >> @@ -259,7 +263,8 @@ static const struct mhi_pci_dev_info >> mhi_qcom_sdx24_info = { >> .edl = "qcom/prog_firehose_sdx24.mbn", >> .config = &modem_qcom_v1_mhiv_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> - .dma_data_width = 32 >> + .dma_data_width = 32, >> + .no_inband_wake = true >> }; >> >> static const struct mhi_channel_config mhi_quectel_em1xx_channels[] = >> { >> @@ -301,7 +306,8 @@ static const struct mhi_pci_dev_info >> mhi_quectel_em1xx_info = { >> .edl = "qcom/prog_firehose_sdx24.mbn", >> .config = &modem_quectel_em1xx_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> - .dma_data_width = 32 >> + .dma_data_width = 32, >> + .no_inband_wake = true >> }; >> >> static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] = >> { >> @@ -339,7 +345,8 @@ static const struct mhi_pci_dev_info >> mhi_foxconn_sdx55_info = { >> .edl = "qcom/sdx55m/edl.mbn", >> .config = &modem_foxconn_sdx55_config, >> .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> - .dma_data_width = 32 >> + .dma_data_width = 32, >> + .no_inband_wake = false >> }; >> >> static const struct pci_device_id mhi_pci_id_table[] = { >> @@ -640,9 +647,12 @@ static int mhi_pci_probe(struct pci_dev *pdev, >> const struct pci_device_id *id) >> mhi_cntrl->status_cb = mhi_pci_status_cb; >> mhi_cntrl->runtime_get = mhi_pci_runtime_get; >> mhi_cntrl->runtime_put = mhi_pci_runtime_put; >> - mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >> - mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >> - mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; >> + >> + if (info->no_inband_wake) { >> + mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >> + mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >> + mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; >> + } >> >> err = mhi_pci_claim(mhi_cntrl, info->bar_num, >> DMA_BIT_MASK(info->dma_data_width)); >> if (err) >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project >> Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-18 17:06 ` Bhaumik Bhatt @ 2021-06-18 17:27 ` Manivannan Sadhasivam 2021-06-18 17:41 ` Bhaumik Bhatt 0 siblings, 1 reply; 7+ messages in thread From: Manivannan Sadhasivam @ 2021-06-18 17:27 UTC (permalink / raw) To: Bhaumik Bhatt Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, carl.yin, naveen.kumar, loic.poulain On Fri, Jun 18, 2021 at 10:06:28AM -0700, Bhaumik Bhatt wrote: > Hi Mani, > On 2021-06-17 11:52 PM, Manivannan Sadhasivam wrote: > > On Thu, Jun 17, 2021 at 11:23:53AM -0700, Bhaumik Bhatt wrote: > > > Devices such as SDX24 do not have the provision for inband wake > > > doorbell in the form of channel 127. Newer devices such as SDX55 > > > or SDX65 have it by default. Ensure the functionality is used > > > based on this such that device wake stays held when a client > > > driver uses mhi_device_get() API or the equivalent debugfs entry. > > > > > > Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake > > > operations") > > > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> > > > --- > > > drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++-------- > > > 1 file changed, 18 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/bus/mhi/pci_generic.c > > > b/drivers/bus/mhi/pci_generic.c > > > index d84b743..31360a2 100644 > > > --- a/drivers/bus/mhi/pci_generic.c > > > +++ b/drivers/bus/mhi/pci_generic.c > > > @@ -32,6 +32,7 @@ > > > * @edl: emergency download mode firmware path (if any) > > > * @bar_num: PCI base address register to use for MHI MMIO register > > > space > > > * @dma_data_width: DMA transfer word size (32 or 64 bits) > > > + * @no_inband_wake: Devices without inband wake support (such as > > > sdx24) > > > > I'd rather like this field to be "inband_wake" and set to false/true > > based on the capability of the devices. Rest looks good. > > > > Thanks, > > Mani > > > I should have known this was coming :) > > Can I use sideband_wake instead of no_inband_wake and leave the booleans as > is? Do you mean, the older devices uses a dedicated sideband GPIO for controlling the wakeup? If so, what is it? > By default, inband_wake will have to be true for any and all devices moving > forward. > Right but still the functionality is not present in older devices. So I think it makes sense to stay with "inband_wake". Thanks, Mani > Please let me know your preference. > > > */ > > > struct mhi_pci_dev_info { > > > const struct mhi_controller_config *config; > > > @@ -40,6 +41,7 @@ struct mhi_pci_dev_info { > > > const char *edl; > > > unsigned int bar_num; > > > unsigned int dma_data_width; > > > + bool no_inband_wake; > > > }; > > > > > > #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \ > > > @@ -242,7 +244,8 @@ static const struct mhi_pci_dev_info > > > mhi_qcom_sdx65_info = { > > > .edl = "qcom/sdx65m/edl.mbn", > > > .config = &modem_qcom_v1_mhiv_config, > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > - .dma_data_width = 32 > > > + .dma_data_width = 32, > > > + .no_inband_wake = false > > > }; > > > > > > static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { > > > @@ -251,7 +254,8 @@ static const struct mhi_pci_dev_info > > > mhi_qcom_sdx55_info = { > > > .edl = "qcom/sdx55m/edl.mbn", > > > .config = &modem_qcom_v1_mhiv_config, > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > - .dma_data_width = 32 > > > + .dma_data_width = 32, > > > + .no_inband_wake = false > > > }; > > > > > > static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = { > > > @@ -259,7 +263,8 @@ static const struct mhi_pci_dev_info > > > mhi_qcom_sdx24_info = { > > > .edl = "qcom/prog_firehose_sdx24.mbn", > > > .config = &modem_qcom_v1_mhiv_config, > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > - .dma_data_width = 32 > > > + .dma_data_width = 32, > > > + .no_inband_wake = true > > > }; > > > > > > static const struct mhi_channel_config mhi_quectel_em1xx_channels[] > > > = { > > > @@ -301,7 +306,8 @@ static const struct mhi_pci_dev_info > > > mhi_quectel_em1xx_info = { > > > .edl = "qcom/prog_firehose_sdx24.mbn", > > > .config = &modem_quectel_em1xx_config, > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > - .dma_data_width = 32 > > > + .dma_data_width = 32, > > > + .no_inband_wake = true > > > }; > > > > > > static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] > > > = { > > > @@ -339,7 +345,8 @@ static const struct mhi_pci_dev_info > > > mhi_foxconn_sdx55_info = { > > > .edl = "qcom/sdx55m/edl.mbn", > > > .config = &modem_foxconn_sdx55_config, > > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, > > > - .dma_data_width = 32 > > > + .dma_data_width = 32, > > > + .no_inband_wake = false > > > }; > > > > > > static const struct pci_device_id mhi_pci_id_table[] = { > > > @@ -640,9 +647,12 @@ static int mhi_pci_probe(struct pci_dev *pdev, > > > const struct pci_device_id *id) > > > mhi_cntrl->status_cb = mhi_pci_status_cb; > > > mhi_cntrl->runtime_get = mhi_pci_runtime_get; > > > mhi_cntrl->runtime_put = mhi_pci_runtime_put; > > > - mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > > > - mhi_cntrl->wake_put = mhi_pci_wake_put_nop; > > > - mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; > > > + > > > + if (info->no_inband_wake) { > > > + mhi_cntrl->wake_get = mhi_pci_wake_get_nop; > > > + mhi_cntrl->wake_put = mhi_pci_wake_put_nop; > > > + mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; > > > + } > > > > > > err = mhi_pci_claim(mhi_cntrl, info->bar_num, > > > DMA_BIT_MASK(info->dma_data_width)); > > > if (err) > > > -- > > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > > > Forum, > > > a Linux Foundation Collaborative Project > > > > > Thanks, > Bhaumik > --- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag 2021-06-18 17:27 ` Manivannan Sadhasivam @ 2021-06-18 17:41 ` Bhaumik Bhatt 0 siblings, 0 replies; 7+ messages in thread From: Bhaumik Bhatt @ 2021-06-18 17:41 UTC (permalink / raw) To: Manivannan Sadhasivam Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, carl.yin, naveen.kumar, loic.poulain On 2021-06-18 10:27 AM, Manivannan Sadhasivam wrote: > On Fri, Jun 18, 2021 at 10:06:28AM -0700, Bhaumik Bhatt wrote: >> Hi Mani, >> On 2021-06-17 11:52 PM, Manivannan Sadhasivam wrote: >> > On Thu, Jun 17, 2021 at 11:23:53AM -0700, Bhaumik Bhatt wrote: >> > > Devices such as SDX24 do not have the provision for inband wake >> > > doorbell in the form of channel 127. Newer devices such as SDX55 >> > > or SDX65 have it by default. Ensure the functionality is used >> > > based on this such that device wake stays held when a client >> > > driver uses mhi_device_get() API or the equivalent debugfs entry. >> > > >> > > Fixes: e3e5e6508fc1 ("bus: mhi: pci_generic: No-Op for device_wake >> > > operations") >> > > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org> >> > > --- >> > > drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++-------- >> > > 1 file changed, 18 insertions(+), 8 deletions(-) >> > > >> > > diff --git a/drivers/bus/mhi/pci_generic.c >> > > b/drivers/bus/mhi/pci_generic.c >> > > index d84b743..31360a2 100644 >> > > --- a/drivers/bus/mhi/pci_generic.c >> > > +++ b/drivers/bus/mhi/pci_generic.c >> > > @@ -32,6 +32,7 @@ >> > > * @edl: emergency download mode firmware path (if any) >> > > * @bar_num: PCI base address register to use for MHI MMIO register >> > > space >> > > * @dma_data_width: DMA transfer word size (32 or 64 bits) >> > > + * @no_inband_wake: Devices without inband wake support (such as >> > > sdx24) >> > >> > I'd rather like this field to be "inband_wake" and set to false/true >> > based on the capability of the devices. Rest looks good. >> > >> > Thanks, >> > Mani >> > >> I should have known this was coming :) >> >> Can I use sideband_wake instead of no_inband_wake and leave the >> booleans as >> is? > > Do you mean, the older devices uses a dedicated sideband GPIO for > controlling the wakeup? If so, what is it? > From what I know, this used to be the DEVICE_WAKE GPIO for SDX24. We moved to use the channel 127 doorbell as WAKE doorbell for this purpose SDX55 onwards for ease of deployment. >> By default, inband_wake will have to be true for any and all devices >> moving >> forward. >> > > Right but still the functionality is not present in older devices. So I > think it makes sense to stay with "inband_wake". > > Thanks, > Mani > So, by default we would not have to add explicit "true" for inband_wake as newer devices will have the feature and controller drivers do not need to populate the entry for a static const struct. >> Please let me know your preference. >> > > */ >> > > struct mhi_pci_dev_info { >> > > const struct mhi_controller_config *config; >> > > @@ -40,6 +41,7 @@ struct mhi_pci_dev_info { >> > > const char *edl; >> > > unsigned int bar_num; >> > > unsigned int dma_data_width; >> > > + bool no_inband_wake; >> > > }; >> > > >> > > #define MHI_CHANNEL_CONFIG_UL(ch_num, ch_name, el_count, ev_ring) \ >> > > @@ -242,7 +244,8 @@ static const struct mhi_pci_dev_info >> > > mhi_qcom_sdx65_info = { >> > > .edl = "qcom/sdx65m/edl.mbn", >> > > .config = &modem_qcom_v1_mhiv_config, >> > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> > > - .dma_data_width = 32 >> > > + .dma_data_width = 32, >> > > + .no_inband_wake = false >> > > }; >> > > >> > > static const struct mhi_pci_dev_info mhi_qcom_sdx55_info = { >> > > @@ -251,7 +254,8 @@ static const struct mhi_pci_dev_info >> > > mhi_qcom_sdx55_info = { >> > > .edl = "qcom/sdx55m/edl.mbn", >> > > .config = &modem_qcom_v1_mhiv_config, >> > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> > > - .dma_data_width = 32 >> > > + .dma_data_width = 32, >> > > + .no_inband_wake = false >> > > }; >> > > >> > > static const struct mhi_pci_dev_info mhi_qcom_sdx24_info = { >> > > @@ -259,7 +263,8 @@ static const struct mhi_pci_dev_info >> > > mhi_qcom_sdx24_info = { >> > > .edl = "qcom/prog_firehose_sdx24.mbn", >> > > .config = &modem_qcom_v1_mhiv_config, >> > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> > > - .dma_data_width = 32 >> > > + .dma_data_width = 32, >> > > + .no_inband_wake = true >> > > }; >> > > >> > > static const struct mhi_channel_config mhi_quectel_em1xx_channels[] >> > > = { >> > > @@ -301,7 +306,8 @@ static const struct mhi_pci_dev_info >> > > mhi_quectel_em1xx_info = { >> > > .edl = "qcom/prog_firehose_sdx24.mbn", >> > > .config = &modem_quectel_em1xx_config, >> > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> > > - .dma_data_width = 32 >> > > + .dma_data_width = 32, >> > > + .no_inband_wake = true >> > > }; >> > > >> > > static const struct mhi_channel_config mhi_foxconn_sdx55_channels[] >> > > = { >> > > @@ -339,7 +345,8 @@ static const struct mhi_pci_dev_info >> > > mhi_foxconn_sdx55_info = { >> > > .edl = "qcom/sdx55m/edl.mbn", >> > > .config = &modem_foxconn_sdx55_config, >> > > .bar_num = MHI_PCI_DEFAULT_BAR_NUM, >> > > - .dma_data_width = 32 >> > > + .dma_data_width = 32, >> > > + .no_inband_wake = false >> > > }; >> > > >> > > static const struct pci_device_id mhi_pci_id_table[] = { >> > > @@ -640,9 +647,12 @@ static int mhi_pci_probe(struct pci_dev *pdev, >> > > const struct pci_device_id *id) >> > > mhi_cntrl->status_cb = mhi_pci_status_cb; >> > > mhi_cntrl->runtime_get = mhi_pci_runtime_get; >> > > mhi_cntrl->runtime_put = mhi_pci_runtime_put; >> > > - mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >> > > - mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >> > > - mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; >> > > + >> > > + if (info->no_inband_wake) { >> > > + mhi_cntrl->wake_get = mhi_pci_wake_get_nop; >> > > + mhi_cntrl->wake_put = mhi_pci_wake_put_nop; >> > > + mhi_cntrl->wake_toggle = mhi_pci_wake_toggle_nop; >> > > + } >> > > >> > > err = mhi_pci_claim(mhi_cntrl, info->bar_num, >> > > DMA_BIT_MASK(info->dma_data_width)); >> > > if (err) >> > > -- >> > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> > > Forum, >> > > a Linux Foundation Collaborative Project >> > > >> >> Thanks, >> Bhaumik >> --- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project Thanks, Bhaumik --- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-18 17:41 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-17 18:23 [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag Bhaumik Bhatt 2021-06-17 18:52 ` Loic Poulain 2021-06-17 21:58 ` Hemant Kumar 2021-06-18 6:52 ` Manivannan Sadhasivam 2021-06-18 17:06 ` Bhaumik Bhatt 2021-06-18 17:27 ` Manivannan Sadhasivam 2021-06-18 17:41 ` Bhaumik Bhatt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).