| Message ID | 20190722215340.3071-1-ilina@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V2,1/4] drivers: qcom: rpmh-rsc: simplify TCS locking | expand |
Quoting Lina Iyer (2019-07-22 14:53:37) > From: "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org> > > The tcs->lock was introduced to serialize access with in TCS group. But, > drv->lock is still needed to synchronize core aspects of the > communication. This puts the drv->lock in the critical and high latency > path of sending a request. drv->lock provides the all necessary > synchronization. So remove locking around TCS group and simply use the > drv->lock instead. This doesn't talk about removing the irq saving and restoring though. Can you keep irq saving and restoring in this patch and then remove that in the next patch with reasoning? It probably isn't safe if the lock is taken in interrupt context anyway. > > Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org> > [ilina: split patch into multiple files, update commit text] > Signed-off-by: Lina Iyer <ilina@codeaurora.org> > diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h > index a7bbbb67991c..969d5030860e 100644 > --- a/drivers/soc/qcom/rpmh-internal.h > +++ b/drivers/soc/qcom/rpmh-internal.h > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c > index e278fc11fe5c..5ede8d6de3ad 100644 > --- a/drivers/soc/qcom/rpmh-rsc.c > +++ b/drivers/soc/qcom/rpmh-rsc.c > @@ -106,26 +106,26 @@ static int tcs_invalidate(struct rsc_drv *drv, int type) > { > int m; > struct tcs_group *tcs; > + int ret = 0; > > tcs = get_tcs_of_type(drv, type); > > - spin_lock(&tcs->lock); > - if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { > - spin_unlock(&tcs->lock); > - return 0; > - } > + spin_lock(&drv->lock); > + if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) > + goto done_invalidate; > > for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { > if (!tcs_is_free(drv, m)) { > - spin_unlock(&tcs->lock); > - return -EAGAIN; > + ret = -EAGAIN; > + goto done_invalidate; > } > write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0); > write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0); > } > bitmap_zero(tcs->slots, MAX_TCS_SLOTS); > - spin_unlock(&tcs->lock); > > +done_invalidate: > + spin_unlock(&drv->lock); > return 0; return ret now? > } > > @@ -349,41 +349,35 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) > { > struct tcs_group *tcs; > int tcs_id; > - unsigned long flags; > int ret; > > tcs = get_tcs_for_msg(drv, msg); > if (IS_ERR(tcs)) > return PTR_ERR(tcs); > > - spin_lock_irqsave(&tcs->lock, flags); > spin_lock(&drv->lock); > /* > * The h/w does not like if we send a request to the same address, > * when one is already in-flight or being processed. > */ > ret = check_for_req_inflight(drv, tcs, msg); > - if (ret) { > - spin_unlock(&drv->lock); > + if (ret) > goto done_write; > - } > > tcs_id = find_free_tcs(tcs); > if (tcs_id < 0) { > ret = tcs_id; > - spin_unlock(&drv->lock); > goto done_write; > } > > tcs->req[tcs_id - tcs->offset] = msg; > set_bit(tcs_id, drv->tcs_in_use); > - spin_unlock(&drv->lock); > > __tcs_buffer_write(drv, tcs_id, 0, msg); > __tcs_trigger(drv, tcs_id); > > done_write: > - spin_unlock_irqrestore(&tcs->lock, flags); > + spin_unlock(&drv->lock); > return ret; > } > > @@ -481,19 +475,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) > { > struct tcs_group *tcs; > int tcs_id = 0, cmd_id = 0; > - unsigned long flags; > int ret; > > tcs = get_tcs_for_msg(drv, msg); > if (IS_ERR(tcs)) > return PTR_ERR(tcs); > > - spin_lock_irqsave(&tcs->lock, flags); > + spin_lock(&drv->lock); > /* find the TCS id and the command in the TCS to write to */ > ret = find_slots(tcs, msg, &tcs_id, &cmd_id); > if (!ret) > __tcs_buffer_write(drv, tcs_id, cmd_id, msg); > - spin_unlock_irqrestore(&tcs->lock, flags); > + spin_unlock(&drv->lock); > These ones, just leave them doing the irq save restore for now?
On Tue, Jul 23 2019 at 12:22 -0600, Stephen Boyd wrote: >Quoting Lina Iyer (2019-07-22 14:53:37) >> From: "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org> >> >> The tcs->lock was introduced to serialize access with in TCS group. But, >> drv->lock is still needed to synchronize core aspects of the >> communication. This puts the drv->lock in the critical and high latency >> path of sending a request. drv->lock provides the all necessary >> synchronization. So remove locking around TCS group and simply use the >> drv->lock instead. > >This doesn't talk about removing the irq saving and restoring though. You mean for drv->lock? It was not an _irqsave/_irqrestore anyways and we were only removing the tcs->lock. >Can you keep irq saving and restoring in this patch and then remove that >in the next patch with reasoning? It probably isn't safe if the lock is >taken in interrupt context anyway. > Yes, the drv->lock should have been irqsave/irqrestore, but it hasn't been changed by this patch. >> >> Signed-off-by: Raju P.L.S.S.S.N <rplsssn@codeaurora.org> >> [ilina: split patch into multiple files, update commit text] >> Signed-off-by: Lina Iyer <ilina@codeaurora.org> > >> diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h >> index a7bbbb67991c..969d5030860e 100644 >> --- a/drivers/soc/qcom/rpmh-internal.h >> +++ b/drivers/soc/qcom/rpmh-internal.h >> diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c >> index e278fc11fe5c..5ede8d6de3ad 100644 >> --- a/drivers/soc/qcom/rpmh-rsc.c >> +++ b/drivers/soc/qcom/rpmh-rsc.c >> @@ -106,26 +106,26 @@ static int tcs_invalidate(struct rsc_drv *drv, int type) >> { >> int m; >> struct tcs_group *tcs; >> + int ret = 0; >> >> tcs = get_tcs_of_type(drv, type); >> >> - spin_lock(&tcs->lock); >> - if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { >> - spin_unlock(&tcs->lock); >> - return 0; >> - } >> + spin_lock(&drv->lock); >> + if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) >> + goto done_invalidate; >> >> for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { >> if (!tcs_is_free(drv, m)) { >> - spin_unlock(&tcs->lock); >> - return -EAGAIN; >> + ret = -EAGAIN; >> + goto done_invalidate; >> } >> write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0); >> write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0); >> } >> bitmap_zero(tcs->slots, MAX_TCS_SLOTS); >> - spin_unlock(&tcs->lock); >> >> +done_invalidate: >> + spin_unlock(&drv->lock); >> return 0; > >return ret now? > Yes, will do. >> } >> >> @@ -349,41 +349,35 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) >> { >> struct tcs_group *tcs; >> int tcs_id; >> - unsigned long flags; >> int ret; >> >> tcs = get_tcs_for_msg(drv, msg); >> if (IS_ERR(tcs)) >> return PTR_ERR(tcs); >> >> - spin_lock_irqsave(&tcs->lock, flags); >> spin_lock(&drv->lock); >> /* >> * The h/w does not like if we send a request to the same address, >> * when one is already in-flight or being processed. >> */ >> ret = check_for_req_inflight(drv, tcs, msg); >> - if (ret) { >> - spin_unlock(&drv->lock); >> + if (ret) >> goto done_write; >> - } >> >> tcs_id = find_free_tcs(tcs); >> if (tcs_id < 0) { >> ret = tcs_id; >> - spin_unlock(&drv->lock); >> goto done_write; >> } >> >> tcs->req[tcs_id - tcs->offset] = msg; >> set_bit(tcs_id, drv->tcs_in_use); >> - spin_unlock(&drv->lock); >> >> __tcs_buffer_write(drv, tcs_id, 0, msg); >> __tcs_trigger(drv, tcs_id); >> >> done_write: >> - spin_unlock_irqrestore(&tcs->lock, flags); >> + spin_unlock(&drv->lock); >> return ret; >> } >> >> @@ -481,19 +475,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) >> { >> struct tcs_group *tcs; >> int tcs_id = 0, cmd_id = 0; >> - unsigned long flags; >> int ret; >> >> tcs = get_tcs_for_msg(drv, msg); >> if (IS_ERR(tcs)) >> return PTR_ERR(tcs); >> >> - spin_lock_irqsave(&tcs->lock, flags); >> + spin_lock(&drv->lock); >> /* find the TCS id and the command in the TCS to write to */ >> ret = find_slots(tcs, msg, &tcs_id, &cmd_id); >> if (!ret) >> __tcs_buffer_write(drv, tcs_id, cmd_id, msg); >> - spin_unlock_irqrestore(&tcs->lock, flags); >> + spin_unlock(&drv->lock); >> > >These ones, just leave them doing the irq save restore for now? > drv->lock ?? --Lina
Quoting Lina Iyer (2019-07-23 12:21:59) > On Tue, Jul 23 2019 at 12:22 -0600, Stephen Boyd wrote: > >Quoting Lina Iyer (2019-07-22 14:53:37) > >> From: "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org> > >> > >> The tcs->lock was introduced to serialize access with in TCS group. But, > >> drv->lock is still needed to synchronize core aspects of the > >> communication. This puts the drv->lock in the critical and high latency > >> path of sending a request. drv->lock provides the all necessary > >> synchronization. So remove locking around TCS group and simply use the > >> drv->lock instead. > > > >This doesn't talk about removing the irq saving and restoring though. > You mean for drv->lock? It was not an _irqsave/_irqrestore anyways and > we were only removing the tcs->lock. Yes drv->lock wasn't an irqsave/restore variant because it was a spinlock inside of an obviously already irqsaved region of code because the tcs->lock was outside the drv->lock and that was saving the irq flags. > > >Can you keep irq saving and restoring in this patch and then remove that > >in the next patch with reasoning? It probably isn't safe if the lock is > >taken in interrupt context anyway. > > > Yes, the drv->lock should have been irqsave/irqrestore, but it hasn't > been changed by this patch. It needs to be changed to maintain the irqsaving/restoring of the code. > >> @@ -349,41 +349,35 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) > >> { > >> struct tcs_group *tcs; > >> int tcs_id; > >> - unsigned long flags; > >> int ret; > >> > >> tcs = get_tcs_for_msg(drv, msg); > >> if (IS_ERR(tcs)) > >> return PTR_ERR(tcs); > >> > >> - spin_lock_irqsave(&tcs->lock, flags); > >> spin_lock(&drv->lock); > >> /* > >> * The h/w does not like if we send a request to the same address, > >> * when one is already in-flight or being processed. > >> */ > >> ret = check_for_req_inflight(drv, tcs, msg); > >> - if (ret) { > >> - spin_unlock(&drv->lock); > >> + if (ret) > >> goto done_write; > >> - } > >> > >> tcs_id = find_free_tcs(tcs); > >> if (tcs_id < 0) { > >> ret = tcs_id; > >> - spin_unlock(&drv->lock); > >> goto done_write; > >> } > >> > >> tcs->req[tcs_id - tcs->offset] = msg; > >> set_bit(tcs_id, drv->tcs_in_use); > >> - spin_unlock(&drv->lock); > >> > >> __tcs_buffer_write(drv, tcs_id, 0, msg); > >> __tcs_trigger(drv, tcs_id); > >> > >> done_write: > >> - spin_unlock_irqrestore(&tcs->lock, flags); > >> + spin_unlock(&drv->lock); > >> return ret; > >> } > >> > >> @@ -481,19 +475,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) > >> { > >> struct tcs_group *tcs; > >> int tcs_id = 0, cmd_id = 0; > >> - unsigned long flags; > >> int ret; > >> > >> tcs = get_tcs_for_msg(drv, msg); > >> if (IS_ERR(tcs)) > >> return PTR_ERR(tcs); > >> > >> - spin_lock_irqsave(&tcs->lock, flags); > >> + spin_lock(&drv->lock); > >> /* find the TCS id and the command in the TCS to write to */ > >> ret = find_slots(tcs, msg, &tcs_id, &cmd_id); > >> if (!ret) > >> __tcs_buffer_write(drv, tcs_id, cmd_id, msg); > >> - spin_unlock_irqrestore(&tcs->lock, flags); > >> + spin_unlock(&drv->lock); > >> > > > >These ones, just leave them doing the irq save restore for now? > > > drv->lock ?? > Yes, it should have irq save/restore still.
On Tue, Jul 23 2019 at 14:19 -0600, Stephen Boyd wrote: >Quoting Lina Iyer (2019-07-23 12:21:59) >> On Tue, Jul 23 2019 at 12:22 -0600, Stephen Boyd wrote: >> >Quoting Lina Iyer (2019-07-22 14:53:37) >> >> From: "Raju P.L.S.S.S.N" <rplsssn@codeaurora.org> >> >> >> >> The tcs->lock was introduced to serialize access with in TCS group. But, >> >> drv->lock is still needed to synchronize core aspects of the >> >> communication. This puts the drv->lock in the critical and high latency >> >> path of sending a request. drv->lock provides the all necessary >> >> synchronization. So remove locking around TCS group and simply use the >> >> drv->lock instead. >> > >> >This doesn't talk about removing the irq saving and restoring though. >> You mean for drv->lock? It was not an _irqsave/_irqrestore anyways and >> we were only removing the tcs->lock. > >Yes drv->lock wasn't an irqsave/restore variant because it was a >spinlock inside of an obviously already irqsaved region of code because >the tcs->lock was outside the drv->lock and that was saving the irq >flags. > Oh, right. >> >> >Can you keep irq saving and restoring in this patch and then remove that >> >in the next patch with reasoning? It probably isn't safe if the lock is >> >taken in interrupt context anyway. >> > >> Yes, the drv->lock should have been irqsave/irqrestore, but it hasn't >> been changed by this patch. > >It needs to be changed to maintain the irqsaving/restoring of the code. > May be I should club this with the following patch. Instead of adding irqsave and restore to drv->lock and then remvoing them again in the following patch. >> >> @@ -349,41 +349,35 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) >> >> { >> >> struct tcs_group *tcs; >> >> int tcs_id; >> >> - unsigned long flags; >> >> int ret; >> >> >> >> tcs = get_tcs_for_msg(drv, msg); >> >> if (IS_ERR(tcs)) >> >> return PTR_ERR(tcs); >> >> >> >> - spin_lock_irqsave(&tcs->lock, flags); >> >> spin_lock(&drv->lock); >> >> /* >> >> * The h/w does not like if we send a request to the same address, >> >> * when one is already in-flight or being processed. >> >> */ >> >> ret = check_for_req_inflight(drv, tcs, msg); >> >> - if (ret) { >> >> - spin_unlock(&drv->lock); >> >> + if (ret) >> >> goto done_write; >> >> - } >> >> >> >> tcs_id = find_free_tcs(tcs); >> >> if (tcs_id < 0) { >> >> ret = tcs_id; >> >> - spin_unlock(&drv->lock); >> >> goto done_write; >> >> } >> >> >> >> tcs->req[tcs_id - tcs->offset] = msg; >> >> set_bit(tcs_id, drv->tcs_in_use); >> >> - spin_unlock(&drv->lock); >> >> >> >> __tcs_buffer_write(drv, tcs_id, 0, msg); >> >> __tcs_trigger(drv, tcs_id); >> >> >> >> done_write: >> >> - spin_unlock_irqrestore(&tcs->lock, flags); >> >> + spin_unlock(&drv->lock); >> >> return ret; >> >> } >> >> >> >> @@ -481,19 +475,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) >> >> { >> >> struct tcs_group *tcs; >> >> int tcs_id = 0, cmd_id = 0; >> >> - unsigned long flags; >> >> int ret; >> >> >> >> tcs = get_tcs_for_msg(drv, msg); >> >> if (IS_ERR(tcs)) >> >> return PTR_ERR(tcs); >> >> >> >> - spin_lock_irqsave(&tcs->lock, flags); >> >> + spin_lock(&drv->lock); >> >> /* find the TCS id and the command in the TCS to write to */ >> >> ret = find_slots(tcs, msg, &tcs_id, &cmd_id); >> >> if (!ret) >> >> __tcs_buffer_write(drv, tcs_id, cmd_id, msg); >> >> - spin_unlock_irqrestore(&tcs->lock, flags); >> >> + spin_unlock(&drv->lock); >> >> >> > >> >These ones, just leave them doing the irq save restore for now? >> > >> drv->lock ?? >> > >Yes, it should have irq save/restore still. >
Quoting Lina Iyer (2019-07-24 07:54:52) > On Tue, Jul 23 2019 at 14:19 -0600, Stephen Boyd wrote: > >Quoting Lina Iyer (2019-07-23 12:21:59) > >> On Tue, Jul 23 2019 at 12:22 -0600, Stephen Boyd wrote: > >> >Can you keep irq saving and restoring in this patch and then remove that > >> >in the next patch with reasoning? It probably isn't safe if the lock is > >> >taken in interrupt context anyway. > >> > > >> Yes, the drv->lock should have been irqsave/irqrestore, but it hasn't > >> been changed by this patch. > > > >It needs to be changed to maintain the irqsaving/restoring of the code. > > > May be I should club this with the following patch. Instead of adding > irqsave and restore to drv->lock and then remvoing them again in the > following patch. > I suspect that gets us back to v1 of this patch series? I'd prefer you just keep the save/restore of irqs in this patch and then remove them later. Or if the order can be the other way, where we remove grabbing the lock in irq context comes first and then consolidate the locks into one it might work.
On Wed, Jul 24 2019 at 12:32 -0600, Stephen Boyd wrote: >Quoting Lina Iyer (2019-07-24 07:54:52) >> On Tue, Jul 23 2019 at 14:19 -0600, Stephen Boyd wrote: >> >Quoting Lina Iyer (2019-07-23 12:21:59) >> >> On Tue, Jul 23 2019 at 12:22 -0600, Stephen Boyd wrote: >> >> >Can you keep irq saving and restoring in this patch and then remove that >> >> >in the next patch with reasoning? It probably isn't safe if the lock is >> >> >taken in interrupt context anyway. >> >> > >> >> Yes, the drv->lock should have been irqsave/irqrestore, but it hasn't >> >> been changed by this patch. >> > >> >It needs to be changed to maintain the irqsaving/restoring of the code. >> > >> May be I should club this with the following patch. Instead of adding >> irqsave and restore to drv->lock and then remvoing them again in the >> following patch. >> > >I suspect that gets us back to v1 of this patch series? I'd prefer you >just keep the save/restore of irqs in this patch and then remove them >later. Or if the order can be the other way, where we remove grabbing >the lock in irq context comes first and then consolidate the locks into >one it might work. > Patches 1 and 3 need not be bundled. We can keep them separate to help understand the change better. This patch order - #2, #1, #3, #4 would work. --Lina
diff --git a/drivers/soc/qcom/rpmh-internal.h b/drivers/soc/qcom/rpmh-internal.h index a7bbbb67991c..969d5030860e 100644 --- a/drivers/soc/qcom/rpmh-internal.h +++ b/drivers/soc/qcom/rpmh-internal.h @@ -28,7 +28,6 @@ struct rsc_drv; * @offset: start of the TCS group relative to the TCSes in the RSC * @num_tcs: number of TCSes in this type * @ncpt: number of commands in each TCS - * @lock: lock for synchronizing this TCS writes * @req: requests that are sent from the TCS * @cmd_cache: flattened cache of cmds in sleep/wake TCS * @slots: indicates which of @cmd_addr are occupied @@ -40,7 +39,6 @@ struct tcs_group { u32 offset; int num_tcs; int ncpt; - spinlock_t lock; const struct tcs_request *req[MAX_TCS_PER_TYPE]; u32 *cmd_cache; DECLARE_BITMAP(slots, MAX_TCS_SLOTS); diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c index e278fc11fe5c..5ede8d6de3ad 100644 --- a/drivers/soc/qcom/rpmh-rsc.c +++ b/drivers/soc/qcom/rpmh-rsc.c @@ -106,26 +106,26 @@ static int tcs_invalidate(struct rsc_drv *drv, int type) { int m; struct tcs_group *tcs; + int ret = 0; tcs = get_tcs_of_type(drv, type); - spin_lock(&tcs->lock); - if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) { - spin_unlock(&tcs->lock); - return 0; - } + spin_lock(&drv->lock); + if (bitmap_empty(tcs->slots, MAX_TCS_SLOTS)) + goto done_invalidate; for (m = tcs->offset; m < tcs->offset + tcs->num_tcs; m++) { if (!tcs_is_free(drv, m)) { - spin_unlock(&tcs->lock); - return -EAGAIN; + ret = -EAGAIN; + goto done_invalidate; } write_tcs_reg_sync(drv, RSC_DRV_CMD_ENABLE, m, 0); write_tcs_reg_sync(drv, RSC_DRV_CMD_WAIT_FOR_CMPL, m, 0); } bitmap_zero(tcs->slots, MAX_TCS_SLOTS); - spin_unlock(&tcs->lock); +done_invalidate: + spin_unlock(&drv->lock); return 0; } @@ -349,41 +349,35 @@ static int tcs_write(struct rsc_drv *drv, const struct tcs_request *msg) { struct tcs_group *tcs; int tcs_id; - unsigned long flags; int ret; tcs = get_tcs_for_msg(drv, msg); if (IS_ERR(tcs)) return PTR_ERR(tcs); - spin_lock_irqsave(&tcs->lock, flags); spin_lock(&drv->lock); /* * The h/w does not like if we send a request to the same address, * when one is already in-flight or being processed. */ ret = check_for_req_inflight(drv, tcs, msg); - if (ret) { - spin_unlock(&drv->lock); + if (ret) goto done_write; - } tcs_id = find_free_tcs(tcs); if (tcs_id < 0) { ret = tcs_id; - spin_unlock(&drv->lock); goto done_write; } tcs->req[tcs_id - tcs->offset] = msg; set_bit(tcs_id, drv->tcs_in_use); - spin_unlock(&drv->lock); __tcs_buffer_write(drv, tcs_id, 0, msg); __tcs_trigger(drv, tcs_id); done_write: - spin_unlock_irqrestore(&tcs->lock, flags); + spin_unlock(&drv->lock); return ret; } @@ -481,19 +475,18 @@ static int tcs_ctrl_write(struct rsc_drv *drv, const struct tcs_request *msg) { struct tcs_group *tcs; int tcs_id = 0, cmd_id = 0; - unsigned long flags; int ret; tcs = get_tcs_for_msg(drv, msg); if (IS_ERR(tcs)) return PTR_ERR(tcs); - spin_lock_irqsave(&tcs->lock, flags); + spin_lock(&drv->lock); /* find the TCS id and the command in the TCS to write to */ ret = find_slots(tcs, msg, &tcs_id, &cmd_id); if (!ret) __tcs_buffer_write(drv, tcs_id, cmd_id, msg); - spin_unlock_irqrestore(&tcs->lock, flags); + spin_unlock(&drv->lock); return ret; } @@ -584,7 +577,6 @@ static int rpmh_probe_tcs_config(struct platform_device *pdev, tcs->type = tcs_cfg[i].type; tcs->num_tcs = tcs_cfg[i].n; tcs->ncpt = ncpt; - spin_lock_init(&tcs->lock); if (!tcs->num_tcs || tcs->type == CONTROL_TCS) continue;