* [PATCH v3 0/2] MHI MMIO register write updates @ 2022-04-13 15:56 Jeffrey Hugo 2022-04-13 15:56 ` [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails Jeffrey Hugo 2022-04-13 15:56 ` [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method Jeffrey Hugo 0 siblings, 2 replies; 5+ messages in thread From: Jeffrey Hugo @ 2022-04-13 15:56 UTC (permalink / raw) To: loic.poulain, mani, quic_hemantk, quic_bbhatt Cc: mhi, linux-arm-msm, linux-kernel, Jeffrey Hugo In case of MHI MMIO writes, the register field write function needs to do reads before writes are performed. Propagate read failures such that callers are made aware of those and can take appropriate action instead of running blind. Optimizing the MMIO initialization function to use mhi_write_reg() in most cases should also be done to improve design. These patches were tested on X86_64 architecture with Ubuntu 18.04 and SDX65 attach. v3: Noticed this was reviewed but never picked up. Rebased to -next v2: -Fix testbot reported missing set of changes from pm.c Bhaumik Bhatt (2): bus: mhi: host: Bail on writing register fields if read fails bus: mhi: host: Optimize and update MMIO register write method drivers/bus/mhi/host/boot.c | 21 +++++++++---- drivers/bus/mhi/host/init.c | 68 ++++++++++++++++++++++++----------------- drivers/bus/mhi/host/internal.h | 7 +++-- drivers/bus/mhi/host/main.c | 9 ++++-- drivers/bus/mhi/host/pm.c | 15 ++++++--- 5 files changed, 76 insertions(+), 44 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails 2022-04-13 15:56 [PATCH v3 0/2] MHI MMIO register write updates Jeffrey Hugo @ 2022-04-13 15:56 ` Jeffrey Hugo 2022-04-18 6:05 ` Manivannan Sadhasivam 2022-04-13 15:56 ` [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method Jeffrey Hugo 1 sibling, 1 reply; 5+ messages in thread From: Jeffrey Hugo @ 2022-04-13 15:56 UTC (permalink / raw) To: loic.poulain, mani, quic_hemantk, quic_bbhatt Cc: mhi, linux-arm-msm, linux-kernel, Bhaumik Bhatt, Jeffrey Hugo From: Bhaumik Bhatt <bbhatt@codeaurora.org> 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> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> --- drivers/bus/mhi/host/boot.c | 21 +++++++++++++++------ drivers/bus/mhi/host/init.c | 22 +++++++++++++++++----- drivers/bus/mhi/host/internal.h | 7 ++++--- drivers/bus/mhi/host/main.c | 9 ++++++--- drivers/bus/mhi/host/pm.c | 15 +++++++++++---- 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c index b0da7ca..16551a3 100644 --- a/drivers/bus/mhi/host/boot.c +++ b/drivers/bus/mhi/host/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,11 +46,16 @@ 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, sequence_id); + ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_RXVECDB_OFFS, + BHIE_RXVECDB_SEQNUM_BMSK, 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 */ @@ -198,10 +204,13 @@ 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, sequence_id); + ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS, + BHIE_TXVECDB_SEQNUM_BMSK, 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/host/init.c b/drivers/bus/mhi/host/init.c index a665b8e..9ac93b7 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -533,9 +533,14 @@ 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].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].val); + if (ret) { + dev_err(dev, "Unable to write to MMIO registers"); + return ret; + } + } return 0; } @@ -1103,8 +1108,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/host/internal.h b/drivers/bus/mhi/host/internal.h index b47d8ef..01fd10a 100644 --- a/drivers/bus/mhi/host/internal.h +++ b/drivers/bus/mhi/host/internal.h @@ -324,8 +324,9 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, 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 val); +int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl, + void __iomem *base, u32 offset, u32 mask, + 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); @@ -339,7 +340,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); diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c index 9021be7..142eea1 100644 --- a/drivers/bus/mhi/host/main.c +++ b/drivers/bus/mhi/host/main.c @@ -65,19 +65,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 val) +int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl, + void __iomem *base, u32 offset, u32 mask, + u32 val) { int ret; u32 tmp; ret = mhi_read_reg(mhi_cntrl, base, offset, &tmp); if (ret) - return; + return ret; tmp &= ~mask; tmp |= (val << __ffs(mask)); 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/host/pm.c b/drivers/bus/mhi/host/pm.c index 3d90b8e..a0e91bd 100644 --- a/drivers/bus/mhi/host/pm.c +++ b/drivers/bus/mhi/host/pm.c @@ -129,13 +129,20 @@ 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, 1); + ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, + MHICTRL_RESET_MASK, 1); } else { - mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, - MHICTRL_MHISTATE_MASK, state); + ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, + MHICTRL_MHISTATE_MASK, state); } + + if (ret) + dev_err(dev, "Failed to set MHI state to: %s", + mhi_state_str(state)); } /* NOP for backward compatibility, host allowed to ring DB in M2 state */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails 2022-04-13 15:56 ` [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails Jeffrey Hugo @ 2022-04-18 6:05 ` Manivannan Sadhasivam 0 siblings, 0 replies; 5+ messages in thread From: Manivannan Sadhasivam @ 2022-04-18 6:05 UTC (permalink / raw) To: Jeffrey Hugo Cc: loic.poulain, quic_hemantk, quic_bbhatt, mhi, linux-arm-msm, linux-kernel, Bhaumik Bhatt On Wed, Apr 13, 2022 at 09:56:45AM -0600, Jeffrey Hugo wrote: > From: Bhaumik Bhatt <bbhatt@codeaurora.org> > > 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> > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > --- > drivers/bus/mhi/host/boot.c | 21 +++++++++++++++------ > drivers/bus/mhi/host/init.c | 22 +++++++++++++++++----- > drivers/bus/mhi/host/internal.h | 7 ++++--- > drivers/bus/mhi/host/main.c | 9 ++++++--- > drivers/bus/mhi/host/pm.c | 15 +++++++++++---- > 5 files changed, 53 insertions(+), 21 deletions(-) > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c > index b0da7ca..16551a3 100644 > --- a/drivers/bus/mhi/host/boot.c > +++ b/drivers/bus/mhi/host/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,11 +46,16 @@ 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, sequence_id); > + ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_RXVECDB_OFFS, > + BHIE_RXVECDB_SEQNUM_BMSK, 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); Leave a new line before return please. > + return 0; > } > > /* Collect RDDM buffer during kernel panic */ > @@ -198,10 +204,13 @@ 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, sequence_id); > + ret = mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS, > + BHIE_TXVECDB_SEQNUM_BMSK, 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/host/init.c b/drivers/bus/mhi/host/init.c > index a665b8e..9ac93b7 100644 > --- a/drivers/bus/mhi/host/init.c > +++ b/drivers/bus/mhi/host/init.c > @@ -533,9 +533,14 @@ 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].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].val); > + if (ret) { > + dev_err(dev, "Unable to write to MMIO registers"); dev_err() missing newline (\n) here and below. Thanks, Mani > + return ret; > + } > + } > > return 0; > } > @@ -1103,8 +1108,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/host/internal.h b/drivers/bus/mhi/host/internal.h > index b47d8ef..01fd10a 100644 > --- a/drivers/bus/mhi/host/internal.h > +++ b/drivers/bus/mhi/host/internal.h > @@ -324,8 +324,9 @@ int __must_check mhi_poll_reg_field(struct mhi_controller *mhi_cntrl, > 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 val); > +int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl, > + void __iomem *base, u32 offset, u32 mask, > + 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); > @@ -339,7 +340,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); > > diff --git a/drivers/bus/mhi/host/main.c b/drivers/bus/mhi/host/main.c > index 9021be7..142eea1 100644 > --- a/drivers/bus/mhi/host/main.c > +++ b/drivers/bus/mhi/host/main.c > @@ -65,19 +65,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 val) > +int __must_check mhi_write_reg_field(struct mhi_controller *mhi_cntrl, > + void __iomem *base, u32 offset, u32 mask, > + u32 val) > { > int ret; > u32 tmp; > > ret = mhi_read_reg(mhi_cntrl, base, offset, &tmp); > if (ret) > - return; > + return ret; > > tmp &= ~mask; > tmp |= (val << __ffs(mask)); > 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/host/pm.c b/drivers/bus/mhi/host/pm.c > index 3d90b8e..a0e91bd 100644 > --- a/drivers/bus/mhi/host/pm.c > +++ b/drivers/bus/mhi/host/pm.c > @@ -129,13 +129,20 @@ 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, 1); > + ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, > + MHICTRL_RESET_MASK, 1); > } else { > - mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, > - MHICTRL_MHISTATE_MASK, state); > + ret = mhi_write_reg_field(mhi_cntrl, mhi_cntrl->regs, MHICTRL, > + MHICTRL_MHISTATE_MASK, state); > } > + > + if (ret) > + dev_err(dev, "Failed to set MHI state to: %s", > + mhi_state_str(state)); > } > > /* NOP for backward compatibility, host allowed to ring DB in M2 state */ > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method 2022-04-13 15:56 [PATCH v3 0/2] MHI MMIO register write updates Jeffrey Hugo 2022-04-13 15:56 ` [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails Jeffrey Hugo @ 2022-04-13 15:56 ` Jeffrey Hugo 2022-04-18 6:08 ` Manivannan Sadhasivam 1 sibling, 1 reply; 5+ messages in thread From: Jeffrey Hugo @ 2022-04-13 15:56 UTC (permalink / raw) To: loic.poulain, mani, quic_hemantk, quic_bbhatt Cc: mhi, linux-arm-msm, linux-kernel, Bhaumik Bhatt, Jeffrey Hugo From: Bhaumik Bhatt <bbhatt@codeaurora.org> 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> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> --- drivers/bus/mhi/host/init.c | 62 ++++++++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c index 9ac93b7..04c409b 100644 --- a/drivers/bus/mhi/host/init.c +++ b/drivers/bus/mhi/host/init.c @@ -425,74 +425,65 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl) struct device *dev = &mhi_cntrl->mhi_dev->dev; struct { u32 offset; - u32 mask; u32 val; } reg_info[] = { { - CCABAP_HIGHER, U32_MAX, + CCABAP_HIGHER, upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr), }, { - CCABAP_LOWER, U32_MAX, + CCABAP_LOWER, lower_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr), }, { - ECABAP_HIGHER, U32_MAX, + ECABAP_HIGHER, upper_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr), }, { - ECABAP_LOWER, U32_MAX, + ECABAP_LOWER, lower_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr), }, { - CRCBAP_HIGHER, U32_MAX, + CRCBAP_HIGHER, upper_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr), }, { - CRCBAP_LOWER, U32_MAX, + CRCBAP_LOWER, lower_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr), }, { - MHICFG, MHICFG_NER_MASK, - mhi_cntrl->total_ev_rings, - }, - { - MHICFG, MHICFG_NHWER_MASK, - mhi_cntrl->hw_ev_rings, - }, - { - MHICTRLBASE_HIGHER, U32_MAX, + MHICTRLBASE_HIGHER, upper_32_bits(mhi_cntrl->iova_start), }, { - MHICTRLBASE_LOWER, U32_MAX, + MHICTRLBASE_LOWER, lower_32_bits(mhi_cntrl->iova_start), }, { - MHIDATABASE_HIGHER, U32_MAX, + MHIDATABASE_HIGHER, upper_32_bits(mhi_cntrl->iova_start), }, { - MHIDATABASE_LOWER, U32_MAX, + MHIDATABASE_LOWER, lower_32_bits(mhi_cntrl->iova_start), }, { - MHICTRLLIMIT_HIGHER, U32_MAX, + MHICTRLLIMIT_HIGHER, upper_32_bits(mhi_cntrl->iova_stop), }, { - MHICTRLLIMIT_LOWER, U32_MAX, + MHICTRLLIMIT_LOWER, lower_32_bits(mhi_cntrl->iova_stop), }, { - MHIDATALIMIT_HIGHER, U32_MAX, + MHIDATALIMIT_HIGHER, upper_32_bits(mhi_cntrl->iova_stop), }, { - MHIDATALIMIT_LOWER, U32_MAX, + MHIDATALIMIT_LOWER, lower_32_bits(mhi_cntrl->iova_stop), }, - { 0, 0, 0 } + {0, 0} }; dev_dbg(dev, "Initializing MHI registers\n"); @@ -533,13 +524,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].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, + 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, + mhi_cntrl->hw_ev_rings); + if (ret) { + dev_err(dev, "Unable to read MHICFG register\n"); + return ret; } return 0; -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method 2022-04-13 15:56 ` [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method Jeffrey Hugo @ 2022-04-18 6:08 ` Manivannan Sadhasivam 0 siblings, 0 replies; 5+ messages in thread From: Manivannan Sadhasivam @ 2022-04-18 6:08 UTC (permalink / raw) To: Jeffrey Hugo Cc: loic.poulain, quic_hemantk, quic_bbhatt, mhi, linux-arm-msm, linux-kernel, Bhaumik Bhatt On Wed, Apr 13, 2022 at 09:56:46AM -0600, Jeffrey Hugo wrote: > From: Bhaumik Bhatt <bbhatt@codeaurora.org> > > 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> > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> > --- > drivers/bus/mhi/host/init.c | 62 ++++++++++++++++++++++----------------------- > 1 file changed, 31 insertions(+), 31 deletions(-) > > diff --git a/drivers/bus/mhi/host/init.c b/drivers/bus/mhi/host/init.c > index 9ac93b7..04c409b 100644 > --- a/drivers/bus/mhi/host/init.c > +++ b/drivers/bus/mhi/host/init.c > @@ -425,74 +425,65 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl) > struct device *dev = &mhi_cntrl->mhi_dev->dev; > struct { > u32 offset; > - u32 mask; > u32 val; > } reg_info[] = { > { > - CCABAP_HIGHER, U32_MAX, > + CCABAP_HIGHER, > upper_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr), > }, > { > - CCABAP_LOWER, U32_MAX, > + CCABAP_LOWER, > lower_32_bits(mhi_cntrl->mhi_ctxt->chan_ctxt_addr), > }, > { > - ECABAP_HIGHER, U32_MAX, > + ECABAP_HIGHER, > upper_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr), > }, > { > - ECABAP_LOWER, U32_MAX, > + ECABAP_LOWER, > lower_32_bits(mhi_cntrl->mhi_ctxt->er_ctxt_addr), > }, > { > - CRCBAP_HIGHER, U32_MAX, > + CRCBAP_HIGHER, > upper_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr), > }, > { > - CRCBAP_LOWER, U32_MAX, > + CRCBAP_LOWER, > lower_32_bits(mhi_cntrl->mhi_ctxt->cmd_ctxt_addr), > }, > { > - MHICFG, MHICFG_NER_MASK, > - mhi_cntrl->total_ev_rings, > - }, > - { > - MHICFG, MHICFG_NHWER_MASK, > - mhi_cntrl->hw_ev_rings, > - }, > - { > - MHICTRLBASE_HIGHER, U32_MAX, > + MHICTRLBASE_HIGHER, > upper_32_bits(mhi_cntrl->iova_start), > }, > { > - MHICTRLBASE_LOWER, U32_MAX, > + MHICTRLBASE_LOWER, > lower_32_bits(mhi_cntrl->iova_start), > }, > { > - MHIDATABASE_HIGHER, U32_MAX, > + MHIDATABASE_HIGHER, > upper_32_bits(mhi_cntrl->iova_start), > }, > { > - MHIDATABASE_LOWER, U32_MAX, > + MHIDATABASE_LOWER, > lower_32_bits(mhi_cntrl->iova_start), > }, > { > - MHICTRLLIMIT_HIGHER, U32_MAX, > + MHICTRLLIMIT_HIGHER, > upper_32_bits(mhi_cntrl->iova_stop), > }, > { > - MHICTRLLIMIT_LOWER, U32_MAX, > + MHICTRLLIMIT_LOWER, > lower_32_bits(mhi_cntrl->iova_stop), > }, > { > - MHIDATALIMIT_HIGHER, U32_MAX, > + MHIDATALIMIT_HIGHER, > upper_32_bits(mhi_cntrl->iova_stop), > }, > { > - MHIDATALIMIT_LOWER, U32_MAX, > + MHIDATALIMIT_LOWER, > lower_32_bits(mhi_cntrl->iova_stop), > }, > - { 0, 0, 0 } > + {0, 0} > }; > > dev_dbg(dev, "Initializing MHI registers\n"); > @@ -533,13 +524,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].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, > + mhi_cntrl->total_ev_rings); > + if (ret) { > + dev_err(dev, "Unable to read MHICFG register\n"); "Unable to write"? > + return ret; > + } > + > + ret = mhi_write_reg_field(mhi_cntrl, base, MHICFG, MHICFG_NHWER_MASK, > + mhi_cntrl->hw_ev_rings); > + if (ret) { > + dev_err(dev, "Unable to read MHICFG register\n"); Same here. Thanks, Mani > + return ret; > } > > return 0; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-04-18 6:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-13 15:56 [PATCH v3 0/2] MHI MMIO register write updates Jeffrey Hugo 2022-04-13 15:56 ` [PATCH v3 1/2] bus: mhi: host: Bail on writing register fields if read fails Jeffrey Hugo 2022-04-18 6:05 ` Manivannan Sadhasivam 2022-04-13 15:56 ` [PATCH v3 2/2] bus: mhi: host: Optimize and update MMIO register write method Jeffrey Hugo 2022-04-18 6:08 ` Manivannan Sadhasivam
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).