* [PATCH v2 1/2] bus: mhi: core: Bail on writing register fields if read fails
2021-08-19 17:15 [PATCH v2 0/2] MHI MMIO register write updates Bhaumik Bhatt
@ 2021-08-19 17:15 ` Bhaumik Bhatt
2021-08-19 17:15 ` [PATCH v2 2/2] bus: mhi: core: Optimize and update MMIO register write method Bhaumik Bhatt
1 sibling, 0 replies; 3+ messages in thread
From: Bhaumik Bhatt @ 2021-08-19 17:15 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: linux-arm-msm, hemantk, linux-kernel, loic.poulain, quic_jhugo,
Bhaumik Bhatt
Helper API to write register fields relies on successful reads
of the register/address prior to the write. Bail out if a failure
is seen when reading the register before the actual write is
performed.
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
---
drivers/bus/mhi/core/boot.c | 25 +++++++++++++++++--------
drivers/bus/mhi/core/init.c | 24 ++++++++++++++++++------
drivers/bus/mhi/core/internal.h | 7 ++++---
drivers/bus/mhi/core/main.c | 9 ++++++---
drivers/bus/mhi/core/pm.c | 18 +++++++++++++-----
5 files changed, 58 insertions(+), 25 deletions(-)
diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 0a97262..13eacda 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -19,8 +19,8 @@
#include "internal.h"
/* Setup RDDM vector table for RDDM transfer and program RXVEC */
-void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
- struct image_info *img_info)
+int mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
+ struct image_info *img_info)
{
struct mhi_buf *mhi_buf = img_info->mhi_buf;
struct bhi_vec_entry *bhi_vec = img_info->bhi_vec;
@@ -28,6 +28,7 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
struct device *dev = &mhi_cntrl->mhi_dev->dev;
u32 sequence_id;
unsigned int i;
+ int ret;
for (i = 0; i < img_info->entries - 1; i++, mhi_buf++, bhi_vec++) {
bhi_vec->dma_addr = mhi_buf->dma_addr;
@@ -45,12 +46,17 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
mhi_write_reg(mhi_cntrl, base, BHIE_RXVECSIZE_OFFS, mhi_buf->len);
sequence_id = MHI_RANDOM_U32_NONZERO(BHIE_RXVECSTATUS_SEQNUM_BMSK);
- mhi_write_reg_field(mhi_cntrl, base, BHIE_RXVECDB_OFFS,
- BHIE_RXVECDB_SEQNUM_BMSK, BHIE_RXVECDB_SEQNUM_SHFT,
- sequence_id);
+ ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_RXVECDB_OFFS,
+ BHIE_RXVECDB_SEQNUM_BMSK,
+ BHIE_RXVECDB_SEQNUM_SHFT, sequence_id);
+ if (ret) {
+ dev_err(dev, "Failed to write sequence ID for BHIE_RXVECDB\n");
+ return ret;
+ }
dev_dbg(dev, "Address: %p and len: 0x%zx sequence: %u\n",
&mhi_buf->dma_addr, mhi_buf->len, sequence_id);
+ return 0;
}
/* Collect RDDM buffer during kernel panic */
@@ -202,11 +208,14 @@ static int mhi_fw_load_bhie(struct mhi_controller *mhi_cntrl,
mhi_write_reg(mhi_cntrl, base, BHIE_TXVECSIZE_OFFS, mhi_buf->len);
- mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS,
- BHIE_TXVECDB_SEQNUM_BMSK, BHIE_TXVECDB_SEQNUM_SHFT,
- sequence_id);
+ ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS,
+ BHIE_TXVECDB_SEQNUM_BMSK,
+ BHIE_TXVECDB_SEQNUM_SHFT, sequence_id);
read_unlock_bh(pm_lock);
+ if (ret)
+ return ret;
+
/* Wait for the image download to complete */
ret = wait_event_timeout(mhi_cntrl->state_event,
MHI_PM_IN_ERROR_STATE(mhi_cntrl->pm_state) ||
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 5aaca6d..0917465 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -544,10 +544,15 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
mhi_cntrl->mhi_cmd[PRIMARY_CMD_RING].ring.db_addr = base + CRDB_LOWER;
/* Write to MMIO registers */
- for (i = 0; reg_info[i].offset; i++)
- mhi_write_reg_field(mhi_cntrl, base, reg_info[i].offset,
- reg_info[i].mask, reg_info[i].shift,
- reg_info[i].val);
+ for (i = 0; reg_info[i].offset; i++) {
+ ret = mhi_write_reg_field(mhi_cntrl, base, reg_info[i].offset,
+ reg_info[i].mask, reg_info[i].shift,
+ reg_info[i].val);
+ if (ret) {
+ dev_err(dev, "Unable to write to MMIO registers");
+ return ret;
+ }
+ }
return 0;
}
@@ -1118,8 +1123,15 @@ int mhi_prepare_for_power_up(struct mhi_controller *mhi_cntrl)
*/
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);
+ if (mhi_cntrl->rddm_image) {
+ ret = mhi_rddm_prepare(mhi_cntrl,
+ mhi_cntrl->rddm_image);
+ if (ret) {
+ mhi_free_bhie_table(mhi_cntrl,
+ mhi_cntrl->rddm_image);
+ goto error_reg_offset;
+ }
+ }
}
mutex_unlock(&mhi_cntrl->pm_mutex);
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 721739c..3d17ec3 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -663,8 +663,9 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl,
u32 shift, u32 val, u32 delayus);
void mhi_write_reg(struct mhi_controller *mhi_cntrl, void __iomem *base,
u32 offset, u32 val);
-void mhi_write_reg_field(struct mhi_controller *mhi_cntrl, void __iomem *base,
- u32 offset, u32 mask, u32 shift, u32 val);
+int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl,
+ void __iomem *base, u32 offset,
+ u32 mask, u32 shift, u32 val);
void mhi_ring_er_db(struct mhi_event *mhi_event);
void mhi_write_db(struct mhi_controller *mhi_cntrl, void __iomem *db_addr,
dma_addr_t db_val);
@@ -678,7 +679,7 @@ int mhi_init_dev_ctxt(struct mhi_controller *mhi_cntrl);
void mhi_deinit_dev_ctxt(struct mhi_controller *mhi_cntrl);
int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl);
void mhi_deinit_free_irq(struct mhi_controller *mhi_cntrl);
-void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
+int mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
struct image_info *img_info);
void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl);
int mhi_prepare_channel(struct mhi_controller *mhi_cntrl,
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index c01ec2f..902d854 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -66,19 +66,22 @@ void mhi_write_reg(struct mhi_controller *mhi_cntrl, void __iomem *base,
mhi_cntrl->write_reg(mhi_cntrl, base + offset, val);
}
-void mhi_write_reg_field(struct mhi_controller *mhi_cntrl, void __iomem *base,
- u32 offset, u32 mask, u32 shift, u32 val)
+int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl,
+ void __iomem *base, u32 offset,
+ u32 mask, u32 shift, u32 val)
{
int ret;
u32 tmp;
ret = mhi_read_reg(mhi_cntrl, base, offset, &tmp);
if (ret)
- return;
+ return ret;
tmp &= ~mask;
tmp |= (val << shift);
mhi_write_reg(mhi_cntrl, base, offset, tmp);
+
+ return 0;
}
void mhi_write_db(struct mhi_controller *mhi_cntrl, void __iomem *db_addr,
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index fb99e37..ba45abe 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -129,14 +129,22 @@ enum mhi_pm_state __must_check mhi_tryset_pm_state(struct mhi_controller *mhi_cn
void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl, enum mhi_state state)
{
+ struct device *dev = &mhi_cntrl->mhi_dev->dev;
+ int ret;
+
if (state == MHI_STATE_RESET) {
- mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
- MHICTRL_RESET_MASK, MHICTRL_RESET_SHIFT, 1);
+ ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
+ MHICTRL_RESET_MASK,
+ MHICTRL_RESET_SHIFT, 1);
} else {
- mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
- MHICTRL_MHISTATE_MASK,
- MHICTRL_MHISTATE_SHIFT, state);
+ ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL,
+ MHICTRL_MHISTATE_MASK,
+ MHICTRL_MHISTATE_SHIFT, state);
}
+
+ if (ret)
+ dev_err(dev, "Failed to set MHI state to: %s",
+ TO_MHI_STATE_STR(state));
}
/* NOP for backward compatibility, host allowed to ring DB in M2 state */
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH v2 2/2] bus: mhi: core: Optimize and update MMIO register write method
2021-08-19 17:15 [PATCH v2 0/2] MHI MMIO register write updates Bhaumik Bhatt
2021-08-19 17:15 ` [PATCH v2 1/2] bus: mhi: core: Bail on writing register fields if read fails Bhaumik Bhatt
@ 2021-08-19 17:15 ` Bhaumik Bhatt
1 sibling, 0 replies; 3+ messages in thread
From: Bhaumik Bhatt @ 2021-08-19 17:15 UTC (permalink / raw)
To: manivannan.sadhasivam
Cc: linux-arm-msm, hemantk, linux-kernel, loic.poulain, quic_jhugo,
Bhaumik Bhatt
As of now, MMIO writes done after ready state transition use the
mhi_write_reg_field() API even though the whole register is being
written in most cases. Optimize this process by using mhi_write_reg()
API instead for those writes and use the mhi_write_reg_field()
API for MHI config registers only.
Signed-off-by: Bhaumik Bhatt <bbhatt@codeaurora.org>
Reviewed-by: Hemant Kumar <hemantk@codeaurora.org>
Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
drivers/bus/mhi/core/init.c | 64 ++++++++++++++++++++++-----------------------
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 0917465..e4be171 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -433,75 +433,65 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
struct device *dev = &mhi_cntrl->mhi_dev->dev;
struct {
u32 offset;
- u32 mask;
- u32 shift;
u32 val;
} reg_info[] = {
{
- CCABAP_HIGHER, U32_MAX, 0,
+ CCABAP_HIGHER,
upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
},
{
- CCABAP_LOWER, U32_MAX, 0,
+ CCABAP_LOWER,
lower_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr),
},
{
- ECABAP_HIGHER, U32_MAX, 0,
+ ECABAP_HIGHER,
upper_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr),
},
{
- ECABAP_LOWER, U32_MAX, 0,
+ ECABAP_LOWER,
lower_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr),
},
{
- CRCBAP_HIGHER, U32_MAX, 0,
+ CRCBAP_HIGHER,
upper_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr),
},
{
- CRCBAP_LOWER, U32_MAX, 0,
+ CRCBAP_LOWER,
lower_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr),
},
{
- MHICFG, MHICFG_NER_MASK, MHICFG_NER_SHIFT,
- mhi_cntrl->total_ev_rings,
- },
- {
- MHICFG, MHICFG_NHWER_MASK, MHICFG_NHWER_SHIFT,
- mhi_cntrl->hw_ev_rings,
- },
- {
- MHICTRLBASE_HIGHER, U32_MAX, 0,
+ MHICTRLBASE_HIGHER,
upper_32_bits(mhi_cntrl->iova_start),
},
{
- MHICTRLBASE_LOWER, U32_MAX, 0,
+ MHICTRLBASE_LOWER,
lower_32_bits(mhi_cntrl->iova_start),
},
{
- MHIDATABASE_HIGHER, U32_MAX, 0,
+ MHIDATABASE_HIGHER,
upper_32_bits(mhi_cntrl->iova_start),
},
{
- MHIDATABASE_LOWER, U32_MAX, 0,
+ MHIDATABASE_LOWER,
lower_32_bits(mhi_cntrl->iova_start),
},
{
- MHICTRLLIMIT_HIGHER, U32_MAX, 0,
+ MHICTRLLIMIT_HIGHER,
upper_32_bits(mhi_cntrl->iova_stop),
},
{
- MHICTRLLIMIT_LOWER, U32_MAX, 0,
+ MHICTRLLIMIT_LOWER,
lower_32_bits(mhi_cntrl->iova_stop),
},
{
- MHIDATALIMIT_HIGHER, U32_MAX, 0,
+ MHIDATALIMIT_HIGHER,
upper_32_bits(mhi_cntrl->iova_stop),
},
{
- MHIDATALIMIT_LOWER, U32_MAX, 0,
+ MHIDATALIMIT_LOWER,
lower_32_bits(mhi_cntrl->iova_stop),
},
- { 0, 0, 0 }
+ {0, 0}
};
dev_dbg(dev, "Initializing MHI registers\n");
@@ -544,14 +534,22 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
mhi_cntrl->mhi_cmd[PRIMARY_CMD_RING].ring.db_addr = base + CRDB_LOWER;
/* Write to MMIO registers */
- for (i = 0; reg_info[i].offset; i++) {
- ret = mhi_write_reg_field(mhi_cntrl, base, reg_info[i].offset,
- reg_info[i].mask, reg_info[i].shift,
- reg_info[i].val);
- if (ret) {
- dev_err(dev, "Unable to write to MMIO registers");
- return ret;
- }
+ for (i = 0; reg_info[i].offset; i++)
+ mhi_write_reg(mhi_cntrl, base, reg_info[i].offset,
+ reg_info[i].val);
+
+ ret = mhi_write_reg_field(mhi_cntrl, base, MHICFG, MHICFG_NER_MASK,
+ MHICFG_NER_SHIFT, mhi_cntrl->total_ev_rings);
+ if (ret) {
+ dev_err(dev, "Unable to read MHICFG register\n");
+ return ret;
+ }
+
+ ret = mhi_write_reg_field(mhi_cntrl, base, MHICFG, MHICFG_NHWER_MASK,
+ MHICFG_NHWER_SHIFT, mhi_cntrl->hw_ev_rings);
+ if (ret) {
+ dev_err(dev, "Unable to read MHICFG register\n");
+ return ret;
}
return 0;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 3+ messages in thread