bus: mhi: pci_generic: Apply no-op for wake using inband wake support flag

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 <[email protected]>

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 <[email protected]>

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 <[email protected]>

---
 drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

drivers/bus/mhi/pci_generic.c +0 -0

index d84b743..31360a2 100644

drivers/bus/mhi/pci_generic.c +17 -8

  */
 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)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Quoted 19 lines · 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 <[email protected]>

---
 drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

drivers/bus/mhi/pci_generic.c +0 -0

index d84b743..31360a2 100644
@@ -32,6 +32,7 @@
  * @edl: emergency download mode firmware path (if any)
  * @bar_num: PCI base address register to use for MHI MMIO register 

Quoted 4 lines

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

Quoted 83 lines

*/
struct mhi_pci_dev_info {

drivers/bus/mhi/pci_generic.c +3 -1

 	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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[] =
{

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[] =
{

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[] = {

drivers/bus/mhi/pci_generic.c +6 -3

@@ -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;
+	}
 

Quoted 7 lines

drivers/bus/mhi/pci_generic.c +0 -0

 	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

Hi Mani,

Quoted 30 lines · Manivannan Sadhasivam

Quoted 21 lines · 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 <[email protected]>

---
 drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

drivers/bus/mhi/pci_generic.c +0 -0

index d84b743..31360a2 100644
@@ -32,6 +32,7 @@
  * @edl: emergency download mode firmware path (if any)
  * @bar_num: PCI base address register to use for MHI MMIO register

Quoted 4 lines

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.

Quoted 3 lines

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.

Quoted 83 lines

Quoted 83 lines

*/
struct mhi_pci_dev_info {

drivers/bus/mhi/pci_generic.c +3 -1

 	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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[]
= {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[]
= {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[] = {

drivers/bus/mhi/pci_generic.c +6 -3

@@ -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;
+	}
 

Quoted 8 lines

Quoted 8 lines

drivers/bus/mhi/pci_generic.c +0 -0

 	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

Quoted 35 lines · Bhaumik Bhatt

Hi Mani,

Quoted 33 lines · Manivannan Sadhasivam

Quoted 21 lines · 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 <[email protected]>

---
 drivers/bus/mhi/pci_generic.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

drivers/bus/mhi/pci_generic.c +0 -0

index d84b743..31360a2 100644
@@ -32,6 +32,7 @@
  * @edl: emergency download mode firmware path (if any)
  * @bar_num: PCI base address register to use for MHI MMIO register

Quoted 4 lines

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

Quoted 4 lines

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?

Quoted 3 lines

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

Quoted 84 lines

Please let me know your preference.

Quoted 83 lines

Quoted 83 lines

*/
struct mhi_pci_dev_info {

drivers/bus/mhi/pci_generic.c +3 -1

 	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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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 = {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[]
= {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[]
= {

drivers/bus/mhi/pci_generic.c +2 -1

@@ -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[] = {

drivers/bus/mhi/pci_generic.c +6 -3

@@ -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;
+	}
 

Quoted 9 lines

Quoted 8 lines

Quoted 8 lines

drivers/bus/mhi/pci_generic.c +0 -0

 	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

Quoted 5 lines

Thanks,
Bhaumik
---
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project