lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 9 Dec 2020 18:29:10 +0530 From: Akash Asthana <akashast@...eaurora.org> To: Roja Rani Yarubandi <rojay@...eaurora.org>, wsa@...nel.org Cc: swboyd@...omium.org, dianders@...omium.org, saiprakash.ranjan@...eaurora.org, gregkh@...uxfoundation.org, mka@...omium.org, msavaliy@....qualcomm.com, skakit@...eaurora.org, vkaur@...eaurora.org, pyarlaga@...eaurora.org, rnayak@...eaurora.org, agross@...nel.org, bjorn.andersson@...aro.org, linux-arm-msm@...r.kernel.org, linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org, sumit.semwal@...aro.org, linux-media@...r.kernel.org Subject: Re: [RESEND PATCH V6 1/2] i2c: i2c-qcom-geni: Store DMA mapping data in geni_i2c_dev struct Hi Roja, On 12/3/2020 4:01 PM, Roja Rani Yarubandi wrote: > Store DMA mapping data in geni_i2c_dev struct to enhance DMA mapping > data scope. For example during shutdown callback to unmap DMA mapping, > this stored DMA mapping data can be used to call geni_se_tx_dma_unprep > and geni_se_rx_dma_unprep functions. > > Add two helper functions geni_i2c_rx_msg_cleanup and > geni_i2c_tx_msg_cleanup to unwrap the things after rx/tx FIFO/DMA > transfers, so that the same can be used in geni_i2c_stop_xfer() > function during shutdown callback. > > Signed-off-by: Roja Rani Yarubandi <rojay@...eaurora.org> > --- > Changes in V5: > - As per Stephen's comments separated this patch from shutdown > callback patch, gi2c->cur = NULL is not removed from > geni_i2c_abort_xfer(), and made a copy of gi2c->cur and passed > to cleanup functions. > > Changes in V6: > - Added spin_lock/unlock in geni_i2c_rx_msg_cleanup() and > geni_i2c_tx_msg_cleanup() functions. > > drivers/i2c/busses/i2c-qcom-geni.c | 69 +++++++++++++++++++++++------- > 1 file changed, 53 insertions(+), 16 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c > index dce75b85253c..bfbc80f65006 100644 > --- a/drivers/i2c/busses/i2c-qcom-geni.c > +++ b/drivers/i2c/busses/i2c-qcom-geni.c > @@ -86,6 +86,9 @@ struct geni_i2c_dev { > u32 clk_freq_out; > const struct geni_i2c_clk_fld *clk_fld; > int suspended; > + void *dma_buf; > + size_t xfer_len; > + dma_addr_t dma_addr; > }; > > struct geni_i2c_err_log { > @@ -348,14 +351,49 @@ static void geni_i2c_tx_fsm_rst(struct geni_i2c_dev *gi2c) > dev_err(gi2c->se.dev, "Timeout resetting TX_FSM\n"); > } > > +static void geni_i2c_rx_msg_cleanup(struct geni_i2c_dev *gi2c, > + struct i2c_msg *cur) > +{ > + struct geni_se *se = &gi2c->se; > + unsigned long flags; > + > + spin_lock_irqsave(&gi2c->lock, flags); > + gi2c->cur_rd = 0; > + if (gi2c->dma_buf) { > + if (gi2c->err) > + geni_i2c_rx_fsm_rst(gi2c); Which race we are trying to avoid here by holding spinlock? We cannot call any sleeping API by holding spinlock, geni_i2c_rx_fsm_rst calls *wait-for-completion*, which is a sleeping call. > + geni_se_rx_dma_unprep(se, gi2c->dma_addr, gi2c->xfer_len); > + i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err); > + } > + spin_unlock_irqrestore(&gi2c->lock, flags); > +} > + > +static void geni_i2c_tx_msg_cleanup(struct geni_i2c_dev *gi2c, > + struct i2c_msg *cur) > +{ > + struct geni_se *se = &gi2c->se; > + unsigned long flags; > + > + spin_lock_irqsave(&gi2c->lock, flags); > + gi2c->cur_wr = 0; > + if (gi2c->dma_buf) { > + if (gi2c->err) > + geni_i2c_tx_fsm_rst(gi2c); Same here Regards, Akash > + geni_se_tx_dma_unprep(se, gi2c->dma_addr, gi2c->xfer_len); > + i2c_put_dma_safe_msg_buf(gi2c->dma_buf, cur, !gi2c->err); > + } > + spin_unlock_irqrestore(&gi2c->lock, flags); > +} > + > static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, > u32 m_param) > { > - dma_addr_t rx_dma; > + dma_addr_t rx_dma = 0; > unsigned long time_left; > void *dma_buf = NULL; > struct geni_se *se = &gi2c->se; > size_t len = msg->len; > + struct i2c_msg *cur; > > if (!of_machine_is_compatible("lenovo,yoga-c630")) > dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); > @@ -372,19 +410,18 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, > geni_se_select_mode(se, GENI_SE_FIFO); > i2c_put_dma_safe_msg_buf(dma_buf, msg, false); > dma_buf = NULL; > + } else { > + gi2c->xfer_len = len; > + gi2c->dma_addr = rx_dma; > + gi2c->dma_buf = dma_buf; > } > > + cur = gi2c->cur; > time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); > if (!time_left) > geni_i2c_abort_xfer(gi2c); > > - gi2c->cur_rd = 0; > - if (dma_buf) { > - if (gi2c->err) > - geni_i2c_rx_fsm_rst(gi2c); > - geni_se_rx_dma_unprep(se, rx_dma, len); > - i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err); > - } > + geni_i2c_rx_msg_cleanup(gi2c, cur); > > return gi2c->err; > } > @@ -392,11 +429,12 @@ static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, > static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, > u32 m_param) > { > - dma_addr_t tx_dma; > + dma_addr_t tx_dma = 0; > unsigned long time_left; > void *dma_buf = NULL; > struct geni_se *se = &gi2c->se; > size_t len = msg->len; > + struct i2c_msg *cur; > > if (!of_machine_is_compatible("lenovo,yoga-c630")) > dma_buf = i2c_get_dma_safe_msg_buf(msg, 32); > @@ -413,22 +451,21 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg, > geni_se_select_mode(se, GENI_SE_FIFO); > i2c_put_dma_safe_msg_buf(dma_buf, msg, false); > dma_buf = NULL; > + } else { > + gi2c->xfer_len = len; > + gi2c->dma_addr = tx_dma; > + gi2c->dma_buf = dma_buf; > } > > if (!dma_buf) /* Get FIFO IRQ */ > writel_relaxed(1, se->base + SE_GENI_TX_WATERMARK_REG); > > + cur = gi2c->cur; > time_left = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT); > if (!time_left) > geni_i2c_abort_xfer(gi2c); > > - gi2c->cur_wr = 0; > - if (dma_buf) { > - if (gi2c->err) > - geni_i2c_tx_fsm_rst(gi2c); > - geni_se_tx_dma_unprep(se, tx_dma, len); > - i2c_put_dma_safe_msg_buf(dma_buf, msg, !gi2c->err); > - } > + geni_i2c_tx_msg_cleanup(gi2c, cur); > > return gi2c->err; > } -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,\na Linux Foundation Collaborative Project
Powered by blists - more mailing lists