Quoting Dan Carpenter (2024-05-08 07:49:34)
> Hello Stephen Boyd,
>
> Commit 2bc20f3c8487 ("soc: qcom: rpmh-rsc: Sleep waiting for tcs
> slots to be free") from Jul 24, 2020 (linux-next), leads to the
> following Smatch static checker warning:
>
> drivers/soc/qcom/rpmh-rsc.c:658 rpmh_rsc_send_data()
> warn: mixing irqsave and irq
>
> drivers/soc/qcom/rpmh-rsc.c
> 645 int rpmh_rsc_send_data(struct rsc_drv *drv, const struct tcs_request *msg)
> 646 {
> 647 struct tcs_group *tcs;
> 648 int tcs_id;
> 649 unsigned long flags;
> 650
> 651 tcs = get_tcs_for_msg(drv, msg);
> 652 if (IS_ERR(tcs))
> 653 return PTR_ERR(tcs);
> 654
> 655 spin_lock_irqsave(&drv->lock, flags);
>
> flags saves if this is called with IRQs disabled. I don't think it is.
>
> 656
> 657 /* Wait forever for a free tcs. It better be there eventually! */
> --> 658 wait_event_lock_irq(drv->tcs_wait,
> 659 (tcs_id = claim_tcs_for_req(drv, tcs, msg)) >= 0,
> 660 drv->lock);
>
> This will enable IRQs and then disable them again. If this were called
> with IRQs disabled then this would probably be bad. (But again, I don't
> think it is).
>
> 661
> 662 tcs->req[tcs_id - tcs->offset] = msg;
> 663 set_bit(tcs_id, drv->tcs_in_use);
> 664 if (msg->state == RPMH_ACTIVE_ONLY_STATE && tcs->type != ACTIVE_TCS) {
> 665 /*
> 666 * Clear previously programmed WAKE commands in selected
> 667 * repurposed TCS to avoid triggering them. tcs->slots will be
> 668 * cleaned from rpmh_flush() by invoking rpmh_rsc_invalidate()
> 669 */
> 670 write_tcs_reg_sync(drv, drv->regs[RSC_DRV_CMD_ENABLE], tcs_id, 0);
> 671 enable_tcs_irq(drv, tcs_id, true);
> 672 }
> 673 spin_unlock_irqrestore(&drv->lock, flags);
>
> And then it sets it back to whatever it was when it was called. So
> that's fine.
>
I see. I think you want this sort of patch so that it is clearer that
this can't be called with interrupts disabled? Would Smatch be happier?
---8<----
diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
index a021dc71807b..568d0b8c52d6 100644
--- a/drivers/soc/qcom/rpmh-rsc.c
+++ b/drivers/soc/qcom/rpmh-rsc.c
@@ -645,13 +645,14 @@ int rpmh_rsc_send_data(struct rsc_drv *drv,
const struct tcs_request *msg)
{
struct tcs_group *tcs;
int tcs_id;
- unsigned long flags;
+
+ might_sleep();
tcs = get_tcs_for_msg(drv, msg);
if (IS_ERR(tcs))
return PTR_ERR(tcs);
- spin_lock_irqsave(&drv->lock, flags);
+ spin_lock_irq(&drv->lock);
/* Wait forever for a free tcs. It better be there eventually! */
wait_event_lock_irq(drv->tcs_wait,
@@ -669,7 +670,7 @@ int rpmh_rsc_send_data(struct rsc_drv *drv, const
struct tcs_request *msg)
write_tcs_reg_sync(drv, drv->regs[RSC_DRV_CMD_ENABLE], tcs_id, 0);
enable_tcs_irq(drv, tcs_id, true);
}
- spin_unlock_irqrestore(&drv->lock, flags);
+ spin_unlock_irq(&drv->lock);
/*
* These two can be done after the lock is released because:
