linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes
@ 2021-05-06 19:51 Bhaumik Bhatt
  2021-05-06 19:51 ` [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation Bhaumik Bhatt
                   ` (6 more replies)
  0 siblings, 7 replies; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

This patch series improves the power up behavior by allowing MHI host driver to
set BHI and/or BHIe offsets early on in the preparation phase and fail pre-power
up if offsets are not found or not within a limited MMIO region. This also
allows MHI host to clean up the offsets in the unprepare after power down phase.

Going forward, controllers will be required to specify a reg_len field which
will be used to check whether the BHI/BHIe offsets are in range or not.

This series has been tested on X86_64 architecture with the PCI generic driver
as controller and an SDX55 device.

v4:
-Added reviewed-by tags
-Updated range check patch to include BHI/e offsets in the error message

v3:
-Added reviewed-by tags
-Updated order of reg_len in mhi_controller structure documentation

v2:
-Added reviewed-by tags
-Moved reg_len entry in mhi_controller structure to allow for a packed struct

Bhaumik Bhatt (6):
  bus: mhi: core: Set BHI/BHIe offsets on power up preparation
  bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
  bus: mhi: Add MMIO region length to controller structure
  ath11k: set register access length for MHI driver
  bus: mhi: pci_generic: Set register access length for MHI driver
  bus: mhi: core: Add range checks for BHI and BHIe

 drivers/bus/mhi/core/init.c           | 61 ++++++++++++++++++++++++-----------
 drivers/bus/mhi/core/pm.c             | 28 +++-------------
 drivers/bus/mhi/pci_generic.c         |  1 +
 drivers/net/wireless/ath/ath11k/mhi.c |  1 +
 include/linux/mhi.h                   |  2 ++
 5 files changed, 50 insertions(+), 43 deletions(-)

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


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-21 13:36   ` Manivannan Sadhasivam
  2021-05-06 19:51 ` [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up Bhaumik Bhatt
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

Set the BHI and/or BHIe offsets in mhi_prepare_for_power_up(),
rearrange the function, and remove the equivalent from
mhi_async_power_up(). This helps consolidate multiple checks
in different parts of the driver and can help MHI fail early on
before power up begins if the offsets are not read correctly.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
---
 drivers/bus/mhi/core/init.c | 42 +++++++++++++++++++++++-------------------
 drivers/bus/mhi/core/pm.c   | 28 ++++------------------------
 2 files changed, 27 insertions(+), 43 deletions(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index c81b377..11c7a3d 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1063,7 +1063,7 @@ EXPORT_SYMBOL_GPL(mhi_free_controller);
 int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 {
 	struct device *dev = &mhi_cntrl->mhi_dev->dev;
-	u32 bhie_off;
+	u32 bhi_off, bhie_off;
 	int ret;
 
 	mutex_lock(&mhi_cntrl->pm_mutex);
@@ -1072,29 +1072,36 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 	if (ret)
 		goto error_dev_ctxt;
 
-	/*
-	 * Allocate RDDM table if specified, this table is for debugging purpose
-	 */
-	if (mhi_cntrl->rddm_size) {
-		mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->rddm_image,
-				     mhi_cntrl->rddm_size);
+	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIOFF, &bhi_off);
+	if (ret) {
+		dev_err(dev, "Error getting BHI offset\n");
+		goto error_reg_offset;
+	}
+	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
 
-		/*
-		 * This controller supports RDDM, so we need to manually clear
-		 * BHIE RX registers since POR values are undefined.
-		 */
+	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
 		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIEOFF,
 				   &bhie_off);
 		if (ret) {
 			dev_err(dev, "Error getting BHIE offset\n");
-			goto bhie_error;
+			goto error_reg_offset;
 		}
-
 		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
+	}
+
+	if (mhi_cntrl->rddm_size) {
+		/*
+		 * This controller supports RDDM, so we need to manually clear
+		 * BHIE RX registers since POR values are undefined.
+		 */
 		memset_io(mhi_cntrl->bhie + BHIE_RXVECADDR_LOW_OFFS,
 			  0, BHIE_RXVECSTATUS_OFFS - BHIE_RXVECADDR_LOW_OFFS +
 			  4);
-
+		/*
+		 * Allocate RDDM table for debugging purpose if specified
+		 */
+		mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->rddm_image,
+				     mhi_cntrl->rddm_size);
 		if (mhi_cntrl->rddm_image)
 			mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
 	}
@@ -1103,11 +1110,8 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 
 	return 0;
 
-bhie_error:
-	if (mhi_cntrl->rddm_image) {
-		mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->rddm_image);
-		mhi_cntrl->rddm_image = NULL;
-	}
+error_reg_offset:
+	mhi_deinit_dev_ctxt(mhi_cntrl);
 
 error_dev_ctxt:
 	mutex_unlock(&mhi_cntrl->pm_mutex);
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index e2e59a3..adf426c 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -1066,28 +1066,8 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 	if (ret)
 		goto error_setup_irq;
 
-	/* Setup BHI offset & INTVEC */
+	/* Setup BHI INTVEC */
 	write_lock_irq(&mhi_cntrl->pm_lock);
-	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIOFF, &val);
-	if (ret) {
-		write_unlock_irq(&mhi_cntrl->pm_lock);
-		goto error_bhi_offset;
-	}
-
-	mhi_cntrl->bhi = mhi_cntrl->regs + val;
-
-	/* Setup BHIE offset */
-	if (mhi_cntrl->fbc_download) {
-		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIEOFF, &val);
-		if (ret) {
-			write_unlock_irq(&mhi_cntrl->pm_lock);
-			dev_err(dev, "Error reading BHIE offset\n");
-			goto error_bhi_offset;
-		}
-
-		mhi_cntrl->bhie = mhi_cntrl->regs + val;
-	}
-
 	mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
 	mhi_cntrl->pm_state = MHI_PM_POR;
 	mhi_cntrl->ee = MHI_EE_MAX;
@@ -1098,7 +1078,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 	if (!MHI_IN_PBL(current_ee) && current_ee != MHI_EE_AMSS) {
 		dev_err(dev, "Not a valid EE for power on\n");
 		ret = -EIO;
-		goto error_bhi_offset;
+		goto error_async_power_up;
 	}
 
 	state = mhi_get_mhi_state(mhi_cntrl);
@@ -1117,7 +1097,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 		if (!ret) {
 			ret = -EIO;
 			dev_info(dev, "Failed to reset MHI due to syserr state\n");
-			goto error_bhi_offset;
+			goto error_async_power_up;
 		}
 
 		/*
@@ -1139,7 +1119,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
 
 	return 0;
 
-error_bhi_offset:
+error_async_power_up:
 	mhi_deinit_free_irq(mhi_cntrl);
 
 error_setup_irq:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
  2021-05-06 19:51 ` [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-21 13:37   ` Manivannan Sadhasivam
  2021-05-06 19:51 ` [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure Bhaumik Bhatt
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

Set the BHI and BHIe pointers to NULL as part of clean-up. This
makes sure that stale pointers are not accessed after powering
MHI down.

Suggested-by: Hemant Kumar <hemantk@codeaurora.org>
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
---
 drivers/bus/mhi/core/init.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 11c7a3d..1cc2f22 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1132,6 +1132,9 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
 		mhi_cntrl->rddm_image = NULL;
 	}
 
+	mhi_cntrl->bhi = NULL;
+	mhi_cntrl->bhie = NULL;
+
 	mhi_deinit_dev_ctxt(mhi_cntrl);
 }
 EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
  2021-05-06 19:51 ` [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation Bhaumik Bhatt
  2021-05-06 19:51 ` [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-21 13:38   ` Manivannan Sadhasivam
  2021-05-06 19:51 ` [PATCH v4 4/6] ath11k: set register access length for MHI driver Bhaumik Bhatt
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

Make controller driver specify the MMIO register region length
for range checking of BHI or BHIe space. This can help validate
that offsets are in acceptable memory region or not and avoid any
boot-up issues due to BHI or BHIe memory accesses.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
---
 include/linux/mhi.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 944aa3a..9c347f5 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -303,6 +303,7 @@ struct mhi_controller_config {
  * @rddm_size: RAM dump size that host should allocate for debugging purpose
  * @sbl_size: SBL image size downloaded through BHIe (optional)
  * @seg_len: BHIe vector size (optional)
+ * @reg_len: Length of the MHI MMIO region (required)
  * @fbc_image: Points to firmware image buffer
  * @rddm_image: Points to RAM dump buffer
  * @mhi_chan: Points to the channel configuration table
@@ -386,6 +387,7 @@ struct mhi_controller {
 	size_t rddm_size;
 	size_t sbl_size;
 	size_t seg_len;
+	size_t reg_len;
 	struct image_info *fbc_image;
 	struct image_info *rddm_image;
 	struct mhi_chan *mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
                   ` (2 preceding siblings ...)
  2021-05-06 19:51 ` [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-21 13:51   ` Manivannan Sadhasivam
  2021-05-06 19:51 ` [PATCH v4 5/6] bus: mhi: pci_generic: Set " Bhaumik Bhatt
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

MHI driver requires register space length to add range checks and
prevent memory region accesses outside of that for MMIO space.
Set it before registering the MHI controller.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
---
 drivers/net/wireless/ath/ath11k/mhi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 09858e5..c0f013c 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -235,6 +235,7 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
 	mhi_ctrl->cntrl_dev = ab->dev;
 	mhi_ctrl->fw_image = ab_pci->amss_path;
 	mhi_ctrl->regs = ab->mem;
+	mhi_ctrl->reg_len = ab->mem_len;
 
 	ret = ath11k_mhi_get_msi(ab_pci);
 	if (ret) {
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v4 5/6] bus: mhi: pci_generic: Set register access length for MHI driver
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
                   ` (3 preceding siblings ...)
  2021-05-06 19:51 ` [PATCH v4 4/6] ath11k: set register access length for MHI driver Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-21 13:52   ` Manivannan Sadhasivam
  2021-05-06 19:51 ` [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe Bhaumik Bhatt
  2021-06-25  5:28 ` [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Manivannan Sadhasivam
  6 siblings, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

MHI driver requires register space length to add range checks and
prevent memory region accesses outside of that for MMIO space.
Set it from the PCI generic controller driver before registering
the MHI controller.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/bus/mhi/pci_generic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
index 7c810f0..fb7889f 100644
--- a/drivers/bus/mhi/pci_generic.c
+++ b/drivers/bus/mhi/pci_generic.c
@@ -463,6 +463,7 @@ static int mhi_pci_claim(struct mhi_controller *mhi_cntrl,
 		return err;
 	}
 	mhi_cntrl->regs = pcim_iomap_table(pdev)[bar_num];
+	mhi_cntrl->reg_len = pci_resource_len(pdev, bar_num);
 
 	err = pci_set_dma_mask(pdev, dma_mask);
 	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] 24+ messages in thread

* [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
                   ` (4 preceding siblings ...)
  2021-05-06 19:51 ` [PATCH v4 5/6] bus: mhi: pci_generic: Set " Bhaumik Bhatt
@ 2021-05-06 19:51 ` Bhaumik Bhatt
  2021-05-07  2:33   ` Hemant Kumar
  2021-05-21 13:54   ` Manivannan Sadhasivam
  2021-06-25  5:28 ` [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Manivannan Sadhasivam
  6 siblings, 2 replies; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:51 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

When obtaining the BHI or BHIe offsets during the power up
preparation phase, range checks are missing. These can help
controller drivers avoid accessing any address outside of the
MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI
registration as it is a required field and range checks will
fail without it.

Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
 drivers/bus/mhi/core/init.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 1cc2f22..aeb1e3c 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -885,7 +885,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
 	if (!mhi_cntrl || !mhi_cntrl->cntrl_dev || !mhi_cntrl->regs ||
 	    !mhi_cntrl->runtime_get || !mhi_cntrl->runtime_put ||
 	    !mhi_cntrl->status_cb || !mhi_cntrl->read_reg ||
-	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs || !mhi_cntrl->irq)
+	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs ||
+	    !mhi_cntrl->irq || !mhi_cntrl->reg_len)
 		return -EINVAL;
 
 	ret = parse_config(mhi_cntrl, config);
@@ -1077,6 +1078,13 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 		dev_err(dev, "Error getting BHI offset\n");
 		goto error_reg_offset;
 	}
+
+	if (bhi_off >= mhi_cntrl->reg_len) {
+		dev_err(dev, "BHI offset: 0x%x is out of range: 0x%zx\n",
+			bhi_off, mhi_cntrl->reg_len);
+		ret = -EINVAL;
+		goto error_reg_offset;
+	}
 	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
 
 	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
@@ -1086,6 +1094,14 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
 			dev_err(dev, "Error getting BHIE offset\n");
 			goto error_reg_offset;
 		}
+
+		if (bhie_off >= mhi_cntrl->reg_len) {
+			dev_err(dev,
+				"BHIe offset: 0x%x is out of range: 0x%zx\n",
+				bhie_off, mhi_cntrl->reg_len);
+			ret = -EINVAL;
+			goto error_reg_offset;
+		}
 		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
 	}
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe
  2021-05-06 19:51 ` [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe Bhaumik Bhatt
@ 2021-05-07  2:33   ` Hemant Kumar
  2021-05-21 13:54   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 24+ messages in thread
From: Hemant Kumar @ 2021-05-07  2:33 UTC (permalink / raw)
  To: Bhaumik Bhatt, manivannan.sadhasivam
  Cc: linux-arm-msm, jhugo, linux-kernel, loic.poulain, linux-wireless,
	kvalo, ath11k



On 5/6/21 12:51 PM, Bhaumik Bhatt wrote:
> When obtaining the BHI or BHIe offsets during the power up
> preparation phase, range checks are missing. These can help
> controller drivers avoid accessing any address outside of the
> MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI
> registration as it is a required field and range checks will
> fail without it.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation
  2021-05-06 19:51 ` [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation Bhaumik Bhatt
@ 2021-05-21 13:36   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:36 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:40PM -0700, Bhaumik Bhatt wrote:
> Set the BHI and/or BHIe offsets in mhi_prepare_for_power_up(),
> rearrange the function, and remove the equivalent from
> mhi_async_power_up(). This helps consolidate multiple checks
> in different parts of the driver and can help MHI fail early on
> before power up begins if the offsets are not read correctly.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/init.c | 42 +++++++++++++++++++++++-------------------
>  drivers/bus/mhi/core/pm.c   | 28 ++++------------------------
>  2 files changed, 27 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index c81b377..11c7a3d 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1063,7 +1063,7 @@ EXPORT_SYMBOL_GPL(mhi_free_controller);
>  int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  {
>  	struct device *dev = &mhi_cntrl->mhi_dev->dev;
> -	u32 bhie_off;
> +	u32 bhi_off, bhie_off;
>  	int ret;
>  
>  	mutex_lock(&mhi_cntrl->pm_mutex);
> @@ -1072,29 +1072,36 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  	if (ret)
>  		goto error_dev_ctxt;
>  
> -	/*
> -	 * Allocate RDDM table if specified, this table is for debugging purpose
> -	 */
> -	if (mhi_cntrl->rddm_size) {
> -		mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->rddm_image,
> -				     mhi_cntrl->rddm_size);
> +	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIOFF, &bhi_off);
> +	if (ret) {
> +		dev_err(dev, "Error getting BHI offset\n");
> +		goto error_reg_offset;
> +	}
> +	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
>  
> -		/*
> -		 * This controller supports RDDM, so we need to manually clear
> -		 * BHIE RX registers since POR values are undefined.
> -		 */
> +	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
>  		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIEOFF,
>  				   &bhie_off);
>  		if (ret) {
>  			dev_err(dev, "Error getting BHIE offset\n");
> -			goto bhie_error;
> +			goto error_reg_offset;
>  		}
> -
>  		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
> +	}
> +
> +	if (mhi_cntrl->rddm_size) {
> +		/*
> +		 * This controller supports RDDM, so we need to manually clear
> +		 * BHIE RX registers since POR values are undefined.
> +		 */
>  		memset_io(mhi_cntrl->bhie + BHIE_RXVECADDR_LOW_OFFS,
>  			  0, BHIE_RXVECSTATUS_OFFS - BHIE_RXVECADDR_LOW_OFFS +
>  			  4);
> -
> +		/*
> +		 * Allocate RDDM table for debugging purpose if specified
> +		 */
> +		mhi_alloc_bhie_table(mhi_cntrl, &mhi_cntrl->rddm_image,
> +				     mhi_cntrl->rddm_size);
>  		if (mhi_cntrl->rddm_image)
>  			mhi_rddm_prepare(mhi_cntrl, mhi_cntrl->rddm_image);
>  	}
> @@ -1103,11 +1110,8 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  
>  	return 0;
>  
> -bhie_error:
> -	if (mhi_cntrl->rddm_image) {
> -		mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->rddm_image);
> -		mhi_cntrl->rddm_image = NULL;
> -	}
> +error_reg_offset:
> +	mhi_deinit_dev_ctxt(mhi_cntrl);
>  
>  error_dev_ctxt:
>  	mutex_unlock(&mhi_cntrl->pm_mutex);
> diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
> index e2e59a3..adf426c 100644
> --- a/drivers/bus/mhi/core/pm.c
> +++ b/drivers/bus/mhi/core/pm.c
> @@ -1066,28 +1066,8 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	if (ret)
>  		goto error_setup_irq;
>  
> -	/* Setup BHI offset & INTVEC */
> +	/* Setup BHI INTVEC */
>  	write_lock_irq(&mhi_cntrl->pm_lock);
> -	ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIOFF, &val);
> -	if (ret) {
> -		write_unlock_irq(&mhi_cntrl->pm_lock);
> -		goto error_bhi_offset;
> -	}
> -
> -	mhi_cntrl->bhi = mhi_cntrl->regs + val;
> -
> -	/* Setup BHIE offset */
> -	if (mhi_cntrl->fbc_download) {
> -		ret = mhi_read_reg(mhi_cntrl, mhi_cntrl->regs, BHIEOFF, &val);
> -		if (ret) {
> -			write_unlock_irq(&mhi_cntrl->pm_lock);
> -			dev_err(dev, "Error reading BHIE offset\n");
> -			goto error_bhi_offset;
> -		}
> -
> -		mhi_cntrl->bhie = mhi_cntrl->regs + val;
> -	}
> -
>  	mhi_write_reg(mhi_cntrl, mhi_cntrl->bhi, BHI_INTVEC, 0);
>  	mhi_cntrl->pm_state = MHI_PM_POR;
>  	mhi_cntrl->ee = MHI_EE_MAX;
> @@ -1098,7 +1078,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  	if (!MHI_IN_PBL(current_ee) && current_ee != MHI_EE_AMSS) {
>  		dev_err(dev, "Not a valid EE for power on\n");
>  		ret = -EIO;
> -		goto error_bhi_offset;
> +		goto error_async_power_up;
>  	}
>  
>  	state = mhi_get_mhi_state(mhi_cntrl);
> @@ -1117,7 +1097,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  		if (!ret) {
>  			ret = -EIO;
>  			dev_info(dev, "Failed to reset MHI due to syserr state\n");
> -			goto error_bhi_offset;
> +			goto error_async_power_up;
>  		}
>  
>  		/*
> @@ -1139,7 +1119,7 @@ int mhi_async_power_up(struct mhi_controller *mhi_cntrl)
>  
>  	return 0;
>  
> -error_bhi_offset:
> +error_async_power_up:
>  	mhi_deinit_free_irq(mhi_cntrl);
>  
>  error_setup_irq:
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
  2021-05-06 19:51 ` [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up Bhaumik Bhatt
@ 2021-05-21 13:37   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:37 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:41PM -0700, Bhaumik Bhatt wrote:
> Set the BHI and BHIe pointers to NULL as part of clean-up. This
> makes sure that stale pointers are not accessed after powering
> MHI down.
> 
> Suggested-by: Hemant Kumar <hemantk@codeaurora.org>
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/init.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 11c7a3d..1cc2f22 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -1132,6 +1132,9 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
>  		mhi_cntrl->rddm_image = NULL;
>  	}
>  
> +	mhi_cntrl->bhi = NULL;
> +	mhi_cntrl->bhie = NULL;
> +
>  	mhi_deinit_dev_ctxt(mhi_cntrl);
>  }
>  EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure
  2021-05-06 19:51 ` [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure Bhaumik Bhatt
@ 2021-05-21 13:38   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:38 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:42PM -0700, Bhaumik Bhatt wrote:
> Make controller driver specify the MMIO register region length
> for range checking of BHI or BHIe space. This can help validate
> that offsets are in acceptable memory region or not and avoid any
> boot-up issues due to BHI or BHIe memory accesses.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  include/linux/mhi.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/mhi.h b/include/linux/mhi.h
> index 944aa3a..9c347f5 100644
> --- a/include/linux/mhi.h
> +++ b/include/linux/mhi.h
> @@ -303,6 +303,7 @@ struct mhi_controller_config {
>   * @rddm_size: RAM dump size that host should allocate for debugging purpose
>   * @sbl_size: SBL image size downloaded through BHIe (optional)
>   * @seg_len: BHIe vector size (optional)
> + * @reg_len: Length of the MHI MMIO region (required)
>   * @fbc_image: Points to firmware image buffer
>   * @rddm_image: Points to RAM dump buffer
>   * @mhi_chan: Points to the channel configuration table
> @@ -386,6 +387,7 @@ struct mhi_controller {
>  	size_t rddm_size;
>  	size_t sbl_size;
>  	size_t seg_len;
> +	size_t reg_len;
>  	struct image_info *fbc_image;
>  	struct image_info *rddm_image;
>  	struct mhi_chan *mhi_chan;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-05-06 19:51 ` [PATCH v4 4/6] ath11k: set register access length for MHI driver Bhaumik Bhatt
@ 2021-05-21 13:51   ` Manivannan Sadhasivam
  2021-06-14 16:02     ` Kalle Valo
  0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:51 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
> MHI driver requires register space length to add range checks and
> prevent memory region accesses outside of that for MMIO space.
> Set it before registering the MHI controller.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Kalle, should we do immutable branch for this patch or I can pick it up via MHI
tree (if there are no other patches expected from ath11k for this controller)?

Thanks,
Mani

> ---
>  drivers/net/wireless/ath/ath11k/mhi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index 09858e5..c0f013c 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -235,6 +235,7 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
>  	mhi_ctrl->cntrl_dev = ab->dev;
>  	mhi_ctrl->fw_image = ab_pci->amss_path;
>  	mhi_ctrl->regs = ab->mem;
> +	mhi_ctrl->reg_len = ab->mem_len;
>  
>  	ret = ath11k_mhi_get_msi(ab_pci);
>  	if (ret) {
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 5/6] bus: mhi: pci_generic: Set register access length for MHI driver
  2021-05-06 19:51 ` [PATCH v4 5/6] bus: mhi: pci_generic: Set " Bhaumik Bhatt
@ 2021-05-21 13:52   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:52 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:44PM -0700, Bhaumik Bhatt wrote:
> MHI driver requires register space length to add range checks and
> prevent memory region accesses outside of that for MMIO space.
> Set it from the PCI generic controller driver before registering
> the MHI controller.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/pci_generic.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/bus/mhi/pci_generic.c b/drivers/bus/mhi/pci_generic.c
> index 7c810f0..fb7889f 100644
> --- a/drivers/bus/mhi/pci_generic.c
> +++ b/drivers/bus/mhi/pci_generic.c
> @@ -463,6 +463,7 @@ static int mhi_pci_claim(struct mhi_controller *mhi_cntrl,
>  		return err;
>  	}
>  	mhi_cntrl->regs = pcim_iomap_table(pdev)[bar_num];
> +	mhi_cntrl->reg_len = pci_resource_len(pdev, bar_num);
>  
>  	err = pci_set_dma_mask(pdev, dma_mask);
>  	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] 24+ messages in thread

* Re: [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe
  2021-05-06 19:51 ` [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe Bhaumik Bhatt
  2021-05-07  2:33   ` Hemant Kumar
@ 2021-05-21 13:54   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-05-21 13:54 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:45PM -0700, Bhaumik Bhatt wrote:
> When obtaining the BHI or BHIe offsets during the power up
> preparation phase, range checks are missing. These can help
> controller drivers avoid accessing any address outside of the
> MMIO region. Ensure that mhi_cntrl->reg_len is set before MHI
> registration as it is a required field and range checks will
> fail without it.
> 
> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> ---
>  drivers/bus/mhi/core/init.c | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
> index 1cc2f22..aeb1e3c 100644
> --- a/drivers/bus/mhi/core/init.c
> +++ b/drivers/bus/mhi/core/init.c
> @@ -885,7 +885,8 @@ int mhi_register_controller(struct mhi_controller *mhi_cntrl,
>  	if (!mhi_cntrl || !mhi_cntrl->cntrl_dev || !mhi_cntrl->regs ||
>  	    !mhi_cntrl->runtime_get || !mhi_cntrl->runtime_put ||
>  	    !mhi_cntrl->status_cb || !mhi_cntrl->read_reg ||
> -	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs || !mhi_cntrl->irq)
> +	    !mhi_cntrl->write_reg || !mhi_cntrl->nr_irqs ||
> +	    !mhi_cntrl->irq || !mhi_cntrl->reg_len)
>  		return -EINVAL;
>  
>  	ret = parse_config(mhi_cntrl, config);
> @@ -1077,6 +1078,13 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  		dev_err(dev, "Error getting BHI offset\n");
>  		goto error_reg_offset;
>  	}
> +
> +	if (bhi_off >= mhi_cntrl->reg_len) {
> +		dev_err(dev, "BHI offset: 0x%x is out of range: 0x%zx\n",
> +			bhi_off, mhi_cntrl->reg_len);
> +		ret = -EINVAL;
> +		goto error_reg_offset;
> +	}
>  	mhi_cntrl->bhi = mhi_cntrl->regs + bhi_off;
>  
>  	if (mhi_cntrl->fbc_download || mhi_cntrl->rddm_size) {
> @@ -1086,6 +1094,14 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
>  			dev_err(dev, "Error getting BHIE offset\n");
>  			goto error_reg_offset;
>  		}
> +
> +		if (bhie_off >= mhi_cntrl->reg_len) {
> +			dev_err(dev,
> +				"BHIe offset: 0x%x is out of range: 0x%zx\n",
> +				bhie_off, mhi_cntrl->reg_len);
> +			ret = -EINVAL;
> +			goto error_reg_offset;
> +		}
>  		mhi_cntrl->bhie = mhi_cntrl->regs + bhie_off;
>  	}
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-05-21 13:51   ` Manivannan Sadhasivam
@ 2021-06-14 16:02     ` Kalle Valo
  2021-06-14 17:49       ` Bhaumik Bhatt
  0 siblings, 1 reply; 24+ messages in thread
From: Kalle Valo @ 2021-06-14 16:02 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Bhaumik Bhatt, linux-arm-msm, hemantk, jhugo, linux-kernel,
	loic.poulain, linux-wireless, ath11k

Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:

> On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
>> MHI driver requires register space length to add range checks and
>> prevent memory region accesses outside of that for MMIO space.
>> Set it before registering the MHI controller.
>> 
>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>
> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>
> Kalle, should we do immutable branch for this patch or I can pick it up via MHI
> tree (if there are no other patches expected from ath11k for this controller)?

I'm not expecting any conflicts with this, and if there are, they should
be easy for Stephen or Linus to fix. So it's easiest to route this via
your tree. But I'm not giving my ack yet, see below.

I'm worried that this patchset breaks bisect. Every patch in the
patchset should not break existing functionality, what if only patches
1-3 are included in the tree but not patch 4? Wouldn't ath11k be broken
then? I didn't review the whole patchset, but I suspect the fix is to
include the ath11k change in the actual mhi patch which changes the
functionality. So that way we would not have a separate ath11k patch at
all.

Also I'm not able to test this patchset at the moment. Can someone else
help and do a quick test with QCA6390 to verify these doesn't break
ath11k?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-14 16:02     ` Kalle Valo
@ 2021-06-14 17:49       ` Bhaumik Bhatt
  2021-06-16 17:38         ` Bhaumik Bhatt
  2021-06-23 17:34         ` Kalle Valo
  0 siblings, 2 replies; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-06-14 17:49 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Manivannan Sadhasivam, linux-arm-msm, hemantk, jhugo,
	linux-kernel, loic.poulain, linux-wireless, ath11k

Hi Kalle,

On 2021-06-14 09:02 AM, Kalle Valo wrote:
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> 
>> On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
>>> MHI driver requires register space length to add range checks and
>>> prevent memory region accesses outside of that for MMIO space.
>>> Set it before registering the MHI controller.
>>> 
>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>> 
>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>> 
>> Kalle, should we do immutable branch for this patch or I can pick it 
>> up via MHI
>> tree (if there are no other patches expected from ath11k for this 
>> controller)?
> 
> I'm not expecting any conflicts with this, and if there are, they 
> should
> be easy for Stephen or Linus to fix. So it's easiest to route this via
> your tree. But I'm not giving my ack yet, see below.
> 
> I'm worried that this patchset breaks bisect. Every patch in the
> patchset should not break existing functionality, what if only patches
> 1-3 are included in the tree but not patch 4? Wouldn't ath11k be broken
> then? I didn't review the whole patchset, but I suspect the fix is to
> include the ath11k change in the actual mhi patch which changes the
> functionality. So that way we would not have a separate ath11k patch at
> all.
> 
> Also I'm not able to test this patchset at the moment. Can someone else
> help and do a quick test with QCA6390 to verify these doesn't break
> ath11k?

I have requested someone to try and test this patch series with QCA6390.

I or the testers will get back to you with the test results when they 
are
available.

As far as your concerns go, you can choose to pick patches 1-3 and that 
would
be just fine.

Things will break if patchset 4 is _not_ in place with patchset 6 being 
part of
the tree.

It would, however, be nice to pick the whole series instead and ensure 
that
the functionality MHI introduces for boot-up sanity is in place for any
controllers such as ath11k.

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] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-14 17:49       ` Bhaumik Bhatt
@ 2021-06-16 17:38         ` Bhaumik Bhatt
  2021-06-18  6:45           ` Manivannan Sadhasivam
  2021-06-23 17:34         ` Kalle Valo
  1 sibling, 1 reply; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-06-16 17:38 UTC (permalink / raw)
  To: Kalle Valo, Manivannan Sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, ath11k, bbhatt=codeaurora.org, lilic, kangxu

Hi Kalle/Mani,

On 2021-06-14 10:49 AM, Bhaumik Bhatt wrote:
> Hi Kalle,
> 
> On 2021-06-14 09:02 AM, Kalle Valo wrote:
>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
>> 
>>> On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
>>>> MHI driver requires register space length to add range checks and
>>>> prevent memory region accesses outside of that for MMIO space.
>>>> Set it before registering the MHI controller.
>>>> 
>>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>>> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>>> 
>>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>> 
>>> Kalle, should we do immutable branch for this patch or I can pick it 
>>> up via MHI
>>> tree (if there are no other patches expected from ath11k for this 
>>> controller)?
>> 
>> I'm not expecting any conflicts with this, and if there are, they 
>> should
>> be easy for Stephen or Linus to fix. So it's easiest to route this via
>> your tree. But I'm not giving my ack yet, see below.
>> 
>> I'm worried that this patchset breaks bisect. Every patch in the
>> patchset should not break existing functionality, what if only patches
>> 1-3 are included in the tree but not patch 4? Wouldn't ath11k be 
>> broken
>> then? I didn't review the whole patchset, but I suspect the fix is to
>> include the ath11k change in the actual mhi patch which changes the
>> functionality. So that way we would not have a separate ath11k patch 
>> at
>> all.
>> 
>> Also I'm not able to test this patchset at the moment. Can someone 
>> else
>> help and do a quick test with QCA6390 to verify these doesn't break
>> ath11k?
> 
> I have requested someone to try and test this patch series with 
> QCA6390.
> 
> I or the testers will get back to you with the test results when they 
> are
> available.
> 
> As far as your concerns go, you can choose to pick patches 1-3 and that 
> would
> be just fine.
> 
> Things will break if patchset 4 is _not_ in place with patchset 6 being 
> part of
> the tree.
> 
> It would, however, be nice to pick the whole series instead and ensure 
> that
> the functionality MHI introduces for boot-up sanity is in place for any
> controllers such as ath11k.
> 
> Thanks,
> Bhaumik
> ---
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
> Forum,
> a Linux Foundation Collaborative Project

Just got confirmation that the whole patch series was tested for 
functional sanity on
Dell E7590 + QCA6390 with Ubuntu18.04 and patch 4/6 is also good to go.

Can you please ACK and pick up this series?

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] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-16 17:38         ` Bhaumik Bhatt
@ 2021-06-18  6:45           ` Manivannan Sadhasivam
  2021-06-23 17:29             ` Kalle Valo
  0 siblings, 1 reply; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-06-18  6:45 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: Kalle Valo, linux-arm-msm, hemantk, jhugo, linux-kernel,
	loic.poulain, linux-wireless, ath11k, bbhatt=codeaurora.org,
	lilic, kangxu

On Wed, Jun 16, 2021 at 10:38:01AM -0700, Bhaumik Bhatt wrote:
> Hi Kalle/Mani,
> 
> On 2021-06-14 10:49 AM, Bhaumik Bhatt wrote:
> > Hi Kalle,
> > 
> > On 2021-06-14 09:02 AM, Kalle Valo wrote:
> > > Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> > > 
> > > > On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
> > > > > MHI driver requires register space length to add range checks and
> > > > > prevent memory region accesses outside of that for MMIO space.
> > > > > Set it before registering the MHI controller.
> > > > > 
> > > > > Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
> > > > > Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
> > > > 
> > > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > > > 
> > > > Kalle, should we do immutable branch for this patch or I can
> > > > pick it up via MHI
> > > > tree (if there are no other patches expected from ath11k for
> > > > this controller)?
> > > 
> > > I'm not expecting any conflicts with this, and if there are, they
> > > should
> > > be easy for Stephen or Linus to fix. So it's easiest to route this via
> > > your tree. But I'm not giving my ack yet, see below.
> > > 
> > > I'm worried that this patchset breaks bisect. Every patch in the
> > > patchset should not break existing functionality, what if only patches
> > > 1-3 are included in the tree but not patch 4? Wouldn't ath11k be
> > > broken
> > > then? I didn't review the whole patchset, but I suspect the fix is to
> > > include the ath11k change in the actual mhi patch which changes the
> > > functionality. So that way we would not have a separate ath11k patch
> > > at
> > > all.
> > > 
> > > Also I'm not able to test this patchset at the moment. Can someone
> > > else
> > > help and do a quick test with QCA6390 to verify these doesn't break
> > > ath11k?
> > 
> > I have requested someone to try and test this patch series with QCA6390.
> > 
> > I or the testers will get back to you with the test results when they
> > are
> > available.
> > 
> > As far as your concerns go, you can choose to pick patches 1-3 and that
> > would
> > be just fine.
> > 
> > Things will break if patchset 4 is _not_ in place with patchset 6 being
> > part of
> > the tree.
> > 
> > It would, however, be nice to pick the whole series instead and ensure
> > that
> > the functionality MHI introduces for boot-up sanity is in place for any
> > controllers such as ath11k.
> > 
> > Thanks,
> > Bhaumik
> > ---
> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> > Forum,
> > a Linux Foundation Collaborative Project
> 
> Just got confirmation that the whole patch series was tested for functional
> sanity on
> Dell E7590 + QCA6390 with Ubuntu18.04 and patch 4/6 is also good to go.
> 
> Can you please ACK and pick up this series?
> 

I can pick the series but I need an Ack from Kalle since it contains
ath11k changes. Kalle, can you please Ack this patch?

I'm planning to send the PR by this weekend.

Thanks,
Mani

> 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] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-18  6:45           ` Manivannan Sadhasivam
@ 2021-06-23 17:29             ` Kalle Valo
  2021-06-24  6:09               ` Manivannan Sadhasivam
  0 siblings, 1 reply; 24+ messages in thread
From: Kalle Valo @ 2021-06-23 17:29 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Bhaumik Bhatt, linux-arm-msm, hemantk, jhugo, linux-kernel,
	loic.poulain, linux-wireless, ath11k, bbhatt=codeaurora.org,
	lilic, kangxu

Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:

> On Wed, Jun 16, 2021 at 10:38:01AM -0700, Bhaumik Bhatt wrote:
>> Hi Kalle/Mani,
>> 
>> On 2021-06-14 10:49 AM, Bhaumik Bhatt wrote:
>> Just got confirmation that the whole patch series was tested for functional
>> sanity on
>> Dell E7590 + QCA6390 with Ubuntu18.04 and patch 4/6 is also good to go.
>> 
>> Can you please ACK and pick up this series?
>> 
>
> I can pick the series but I need an Ack from Kalle since it contains
> ath11k changes. Kalle, can you please Ack this patch?
>
> I'm planning to send the PR by this weekend.

Sorry for the late reply. Yes, as this now tested with ath11k driver
please take this ath11k patch via the mhi tree:

Acked-by: Kalle Valo <kvalo@codeaurora.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-14 17:49       ` Bhaumik Bhatt
  2021-06-16 17:38         ` Bhaumik Bhatt
@ 2021-06-23 17:34         ` Kalle Valo
  2021-06-23 21:33           ` Bhaumik Bhatt
  1 sibling, 1 reply; 24+ messages in thread
From: Kalle Valo @ 2021-06-23 17:34 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: Manivannan Sadhasivam, linux-arm-msm, hemantk, jhugo,
	linux-kernel, loic.poulain, linux-wireless, ath11k

Bhaumik Bhatt <bbhatt@codeaurora.org> writes:

> Hi Kalle,
>
> On 2021-06-14 09:02 AM, Kalle Valo wrote:
>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
>>
>>> On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
>>>> MHI driver requires register space length to add range checks and
>>>> prevent memory region accesses outside of that for MMIO space.
>>>> Set it before registering the MHI controller.
>>>>
>>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>>> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>>>
>>> Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
>>>
>>> Kalle, should we do immutable branch for this patch or I can pick
>>> it up via MHI
>>> tree (if there are no other patches expected from ath11k for this
>>> controller)?
>>
>> I'm not expecting any conflicts with this, and if there are, they
>> should
>> be easy for Stephen or Linus to fix. So it's easiest to route this via
>> your tree. But I'm not giving my ack yet, see below.
>>
>> I'm worried that this patchset breaks bisect. Every patch in the
>> patchset should not break existing functionality, what if only patches
>> 1-3 are included in the tree but not patch 4? Wouldn't ath11k be broken
>> then? I didn't review the whole patchset, but I suspect the fix is to
>> include the ath11k change in the actual mhi patch which changes the
>> functionality. So that way we would not have a separate ath11k patch at
>> all.
>>
>> Also I'm not able to test this patchset at the moment. Can someone else
>> help and do a quick test with QCA6390 to verify these doesn't break
>> ath11k?
>
> I have requested someone to try and test this patch series with QCA6390.
>
> I or the testers will get back to you with the test results when they
> are available.
>
> As far as your concerns go, you can choose to pick patches 1-3 and
> that would be just fine.
>
> Things will break if patchset 4 is _not_ in place with patchset 6
> being part of the tree.
>
> It would, however, be nice to pick the whole series instead and ensure
> that the functionality MHI introduces for boot-up sanity is in place
> for any controllers such as ath11k.

Just to be clear, this is not about me picking up any patches
separately. I was instead making sure git-bisect works correctly, as it
can randomly choose to test any commit in the tree. But based on your
description everything seems to be in order in this patchset and bisect
will work correctly.

git-bisect is an important tool for me when I'm searching the root cause
for ath11k regressions, that's why I'm so careful to make sure it works.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-23 17:34         ` Kalle Valo
@ 2021-06-23 21:33           ` Bhaumik Bhatt
  0 siblings, 0 replies; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-06-23 21:33 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Manivannan Sadhasivam, linux-arm-msm, hemantk, jhugo,
	linux-kernel, loic.poulain, linux-wireless, ath11k

On 2021-06-23 10:34 AM, Kalle Valo wrote:
> Bhaumik Bhatt <bbhatt@codeaurora.org> writes:
> 
>> Hi Kalle,
>> 
>> On 2021-06-14 09:02 AM, Kalle Valo wrote:
>>> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
>>> 
>>>> On Thu, May 06, 2021 at 12:51:43PM -0700, Bhaumik Bhatt wrote:
>>>>> MHI driver requires register space length to add range checks and
>>>>> prevent memory region accesses outside of that for MMIO space.
>>>>> Set it before registering the MHI controller.
>>>>> 
>>>>> Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
>>>>> Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
>>>> 
>>>> Reviewed-by: Manivannan Sadhasivam 
>>>> <manivannan.sadhasivam@linaro.org>
>>>> 
>>>> Kalle, should we do immutable branch for this patch or I can pick
>>>> it up via MHI
>>>> tree (if there are no other patches expected from ath11k for this
>>>> controller)?
>>> 
>>> I'm not expecting any conflicts with this, and if there are, they
>>> should
>>> be easy for Stephen or Linus to fix. So it's easiest to route this 
>>> via
>>> your tree. But I'm not giving my ack yet, see below.
>>> 
>>> I'm worried that this patchset breaks bisect. Every patch in the
>>> patchset should not break existing functionality, what if only 
>>> patches
>>> 1-3 are included in the tree but not patch 4? Wouldn't ath11k be 
>>> broken
>>> then? I didn't review the whole patchset, but I suspect the fix is to
>>> include the ath11k change in the actual mhi patch which changes the
>>> functionality. So that way we would not have a separate ath11k patch 
>>> at
>>> all.
>>> 
>>> Also I'm not able to test this patchset at the moment. Can someone 
>>> else
>>> help and do a quick test with QCA6390 to verify these doesn't break
>>> ath11k?
>> 
>> I have requested someone to try and test this patch series with 
>> QCA6390.
>> 
>> I or the testers will get back to you with the test results when they
>> are available.
>> 
>> As far as your concerns go, you can choose to pick patches 1-3 and
>> that would be just fine.
>> 
>> Things will break if patchset 4 is _not_ in place with patchset 6
>> being part of the tree.
>> 
>> It would, however, be nice to pick the whole series instead and ensure
>> that the functionality MHI introduces for boot-up sanity is in place
>> for any controllers such as ath11k.
> 
> Just to be clear, this is not about me picking up any patches
> separately. I was instead making sure git-bisect works correctly, as it
> can randomly choose to test any commit in the tree. But based on your
> description everything seems to be in order in this patchset and bisect
> will work correctly.
> 
> git-bisect is an important tool for me when I'm searching the root 
> cause
> for ath11k regressions, that's why I'm so careful to make sure it 
> works.
OK. Understood.

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] 24+ messages in thread

* Re: [PATCH v4 4/6] ath11k: set register access length for MHI driver
  2021-06-23 17:29             ` Kalle Valo
@ 2021-06-24  6:09               ` Manivannan Sadhasivam
  0 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-06-24  6:09 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Bhaumik Bhatt, linux-arm-msm, hemantk, jhugo, linux-kernel,
	loic.poulain, linux-wireless, ath11k, bbhatt=codeaurora.org,
	lilic, kangxu

On Wed, Jun 23, 2021 at 08:29:26PM +0300, Kalle Valo wrote:
> Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> writes:
> 
> > On Wed, Jun 16, 2021 at 10:38:01AM -0700, Bhaumik Bhatt wrote:
> >> Hi Kalle/Mani,
> >> 
> >> On 2021-06-14 10:49 AM, Bhaumik Bhatt wrote:
> >> Just got confirmation that the whole patch series was tested for functional
> >> sanity on
> >> Dell E7590 + QCA6390 with Ubuntu18.04 and patch 4/6 is also good to go.
> >> 
> >> Can you please ACK and pick up this series?
> >> 
> >
> > I can pick the series but I need an Ack from Kalle since it contains
> > ath11k changes. Kalle, can you please Ack this patch?
> >
> > I'm planning to send the PR by this weekend.
> 
> Sorry for the late reply. Yes, as this now tested with ath11k driver
> please take this ath11k patch via the mhi tree:
> 
> Acked-by: Kalle Valo <kvalo@codeaurora.org>

Thanks for the Ack, Kalle. But I've sent the pull to Greg for 5.14. So
once the merge window closes, I'll apply this series for 5.15.

Thanks,
Mani

> 
> -- 
> https://patchwork.kernel.org/project/linux-wireless/list/
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes
  2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
                   ` (5 preceding siblings ...)
  2021-05-06 19:51 ` [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe Bhaumik Bhatt
@ 2021-06-25  5:28 ` Manivannan Sadhasivam
  6 siblings, 0 replies; 24+ messages in thread
From: Manivannan Sadhasivam @ 2021-06-25  5:28 UTC (permalink / raw)
  To: Bhaumik Bhatt
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k

On Thu, May 06, 2021 at 12:51:39PM -0700, Bhaumik Bhatt wrote:
> This patch series improves the power up behavior by allowing MHI host driver to
> set BHI and/or BHIe offsets early on in the preparation phase and fail pre-power
> up if offsets are not found or not within a limited MMIO region. This also
> allows MHI host to clean up the offsets in the unprepare after power down phase.
> 
> Going forward, controllers will be required to specify a reg_len field which
> will be used to check whether the BHI/BHIe offsets are in range or not.
> 
> This series has been tested on X86_64 architecture with the PCI generic driver
> as controller and an SDX55 device.
> 

Since there is a chance to send this series for 5.14, I've applied it
now to mhi-next!

Thanks,
Mani

> v4:
> -Added reviewed-by tags
> -Updated range check patch to include BHI/e offsets in the error message
> 
> v3:
> -Added reviewed-by tags
> -Updated order of reg_len in mhi_controller structure documentation
> 
> v2:
> -Added reviewed-by tags
> -Moved reg_len entry in mhi_controller structure to allow for a packed struct
> 
> Bhaumik Bhatt (6):
>   bus: mhi: core: Set BHI/BHIe offsets on power up preparation
>   bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
>   bus: mhi: Add MMIO region length to controller structure
>   ath11k: set register access length for MHI driver
>   bus: mhi: pci_generic: Set register access length for MHI driver
>   bus: mhi: core: Add range checks for BHI and BHIe
> 
>  drivers/bus/mhi/core/init.c           | 61 ++++++++++++++++++++++++-----------
>  drivers/bus/mhi/core/pm.c             | 28 +++-------------
>  drivers/bus/mhi/pci_generic.c         |  1 +
>  drivers/net/wireless/ath/ath11k/mhi.c |  1 +
>  include/linux/mhi.h                   |  2 ++
>  5 files changed, 50 insertions(+), 43 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up
  2021-05-06 19:32 Bhaumik Bhatt
@ 2021-05-06 19:32 ` Bhaumik Bhatt
  0 siblings, 0 replies; 24+ messages in thread
From: Bhaumik Bhatt @ 2021-05-06 19:32 UTC (permalink / raw)
  To: manivannan.sadhasivam
  Cc: linux-arm-msm, hemantk, jhugo, linux-kernel, loic.poulain,
	linux-wireless, kvalo, ath11k, Bhaumik Bhatt

From: Bhaumik Bhatt <bbhatt@codeaurora.org>

Set the BHI and BHIe pointers to NULL as part of clean-up. This
makes sure that stale pointers are not accessed after powering
MHI down.

Suggested-by: Hemant Kumar <hemantk@codeaurora.org>
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
---
 drivers/bus/mhi/core/init.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 11c7a3d..1cc2f22 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1132,6 +1132,9 @@ void mhi_unprepare_after_power_down(struct mhi_controller *mhi_cntrl)
 		mhi_cntrl->rddm_image = NULL;
 	}
 
+	mhi_cntrl->bhi = NULL;
+	mhi_cntrl->bhie = NULL;
+
 	mhi_deinit_dev_ctxt(mhi_cntrl);
 }
 EXPORT_SYMBOL_GPL(mhi_unprepare_after_power_down);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


^ permalink raw reply related	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-06-25  5:28 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06 19:51 [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Bhaumik Bhatt
2021-05-06 19:51 ` [PATCH v4 1/6] bus: mhi: core: Set BHI/BHIe offsets on power up preparation Bhaumik Bhatt
2021-05-21 13:36   ` Manivannan Sadhasivam
2021-05-06 19:51 ` [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up Bhaumik Bhatt
2021-05-21 13:37   ` Manivannan Sadhasivam
2021-05-06 19:51 ` [PATCH v4 3/6] bus: mhi: Add MMIO region length to controller structure Bhaumik Bhatt
2021-05-21 13:38   ` Manivannan Sadhasivam
2021-05-06 19:51 ` [PATCH v4 4/6] ath11k: set register access length for MHI driver Bhaumik Bhatt
2021-05-21 13:51   ` Manivannan Sadhasivam
2021-06-14 16:02     ` Kalle Valo
2021-06-14 17:49       ` Bhaumik Bhatt
2021-06-16 17:38         ` Bhaumik Bhatt
2021-06-18  6:45           ` Manivannan Sadhasivam
2021-06-23 17:29             ` Kalle Valo
2021-06-24  6:09               ` Manivannan Sadhasivam
2021-06-23 17:34         ` Kalle Valo
2021-06-23 21:33           ` Bhaumik Bhatt
2021-05-06 19:51 ` [PATCH v4 5/6] bus: mhi: pci_generic: Set " Bhaumik Bhatt
2021-05-21 13:52   ` Manivannan Sadhasivam
2021-05-06 19:51 ` [PATCH v4 6/6] bus: mhi: core: Add range checks for BHI and BHIe Bhaumik Bhatt
2021-05-07  2:33   ` Hemant Kumar
2021-05-21 13:54   ` Manivannan Sadhasivam
2021-06-25  5:28 ` [PATCH v4 0/6] BHI/BHIe improvements for MHI power purposes Manivannan Sadhasivam
  -- strict thread matches above, loose matches on Subject: below --
2021-05-06 19:32 Bhaumik Bhatt
2021-05-06 19:32 ` [PATCH v4 2/6] bus: mhi: core: Set BHI and BHIe pointers to NULL in clean-up 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).