Messages in this thread | | | Date | Fri, 18 Jun 2021 10:06:28 -0700 | From | Bhaumik Bhatt <> | Subject | Re: [PATCH] bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag |
| |
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
| |