Messages in this thread | | | Date | Fri, 30 Oct 2020 20:29:31 +0530 | From | rojay@codeauro ... | Subject | Re: [PATCH V5 3/3] i2c: i2c-qcom-geni: Add shutdown callback for i2c |
| |
Hi Stephen,
On 2020-10-03 07:09, Stephen Boyd wrote: > Quoting Roja Rani Yarubandi (2020-10-01 01:44:25) >> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c >> b/drivers/i2c/busses/i2c-qcom-geni.c >> index aee2a1dd2c62..56d3fbfe7eb6 100644 >> --- a/drivers/i2c/busses/i2c-qcom-geni.c >> +++ b/drivers/i2c/busses/i2c-qcom-geni.c >> @@ -380,6 +380,36 @@ static void geni_i2c_tx_msg_cleanup(struct >> geni_i2c_dev *gi2c, >> } >> } >> >> +static void geni_i2c_stop_xfer(struct geni_i2c_dev *gi2c) >> +{ >> + int ret; >> + u32 geni_status; >> + unsigned long flags; >> + struct i2c_msg *cur; >> + >> + /* Resume device, runtime suspend can happen anytime during >> transfer */ >> + ret = pm_runtime_get_sync(gi2c->se.dev); >> + if (ret < 0) { >> + dev_err(gi2c->se.dev, "Failed to resume device: %d\n", >> ret); >> + return; >> + } >> + >> + spin_lock_irqsave(&gi2c->lock, flags); > > We grab the lock here. > >> + geni_status = readl_relaxed(gi2c->se.base + SE_GENI_STATUS); >> + if (!(geni_status & M_GENI_CMD_ACTIVE)) >> + goto out; >> + >> + cur = gi2c->cur; >> + geni_i2c_abort_xfer(gi2c); > > But it looks like this function takes the lock again? Did you test this > with lockdep enabled? It should hang even without lockdep, so it seems > like this path of code has not been tested. >
Tested with lockdep enabled, and fixed the unsafe lock order issue here. And to be more proper moved spin_lock/unlock to cleanup functions.
>> + if (cur->flags & I2C_M_RD) >> + geni_i2c_rx_msg_cleanup(gi2c, cur); >> + else >> + geni_i2c_tx_msg_cleanup(gi2c, cur); >> + spin_unlock_irqrestore(&gi2c->lock, flags); >> +out: >> + pm_runtime_put_sync_suspend(gi2c->se.dev); >> +} >> + >> static int geni_i2c_rx_one_msg(struct geni_i2c_dev *gi2c, struct >> i2c_msg *msg, >> u32 m_param) >> {
| |