| Message ID | 20230405-pl180-busydetect-fix-v2-10-eeb10323b546@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix busydetect on Ux500 PL180/MMCI | expand |
On Sun, 9 Apr 2023 at 00:00, Linus Walleij <linus.walleij@linaro.org> wrote: > > If we have a .busy_complete() callback, then check if > the state machine triggered from the busy detect interrupts > is busy: then we are certainly busy. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > ChangeLog v1->v2: > - Do this in a safer way that falls back to reading busy > status from the hardware if the state machine is NOT > busy. > --- > drivers/mmc/host/mmci.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index 9a7f441ec9d6..180a7b719347 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -339,6 +339,12 @@ static int mmci_card_busy(struct mmc_host *mmc) > unsigned long flags; > int busy = 0; > > + /* If we are waiting for IRQs we are certainly busy */ > + if (host->ops->busy_complete && > + host->busy_state != MMCI_BUSY_IDLE && > + host->busy_state != MMCI_BUSY_DONE) > + return 1; This looks fishy to me. If this is needed, that means that the mmc core is calling the ->card_busy() ops in the middle of a request that has not been completed yet. This shouldn't happen - unless I am misunderstanding some part of the internal new state machine. > + > spin_lock_irqsave(&host->lock, flags); > if (readl(host->base + MMCISTATUS) & host->variant->busy_detect_flag) > busy = 1; > > -- > 2.39.2 > Kind regards Uffe
On Mon, Apr 17, 2023 at 4:49 PM Ulf Hansson <ulf.hansson@linaro.org> wrote: > On Sun, 9 Apr 2023 at 00:00, Linus Walleij <linus.walleij@linaro.org> wrote: > > > > If we have a .busy_complete() callback, then check if > > the state machine triggered from the busy detect interrupts > > is busy: then we are certainly busy. > > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > --- > > ChangeLog v1->v2: > > - Do this in a safer way that falls back to reading busy > > status from the hardware if the state machine is NOT > > busy. > > --- > > drivers/mmc/host/mmci.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > > index 9a7f441ec9d6..180a7b719347 100644 > > --- a/drivers/mmc/host/mmci.c > > +++ b/drivers/mmc/host/mmci.c > > @@ -339,6 +339,12 @@ static int mmci_card_busy(struct mmc_host *mmc) > > unsigned long flags; > > int busy = 0; > > > > + /* If we are waiting for IRQs we are certainly busy */ > > + if (host->ops->busy_complete && > > + host->busy_state != MMCI_BUSY_IDLE && > > + host->busy_state != MMCI_BUSY_DONE) > > + return 1; > > This looks fishy to me. > > If this is needed, that means that the mmc core is calling the > ->card_busy() ops in the middle of a request that has not been > completed yet. This shouldn't happen - unless I am misunderstanding > some part of the internal new state machine. You're probably right about that, I have no idea when the core can and cannot call ->card_busy, I just assumed it could be called at any time (even while waiting for busy interrupts). If you say it won't get called then this patch isn't needed. Yours, Linus Walleij
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 9a7f441ec9d6..180a7b719347 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -339,6 +339,12 @@ static int mmci_card_busy(struct mmc_host *mmc) unsigned long flags; int busy = 0; + /* If we are waiting for IRQs we are certainly busy */ + if (host->ops->busy_complete && + host->busy_state != MMCI_BUSY_IDLE && + host->busy_state != MMCI_BUSY_DONE) + return 1; + spin_lock_irqsave(&host->lock, flags); if (readl(host->base + MMCISTATUS) & host->variant->busy_detect_flag) busy = 1;
If we have a .busy_complete() callback, then check if the state machine triggered from the busy detect interrupts is busy: then we are certainly busy. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- ChangeLog v1->v2: - Do this in a safer way that falls back to reading busy status from the hardware if the state machine is NOT busy. --- drivers/mmc/host/mmci.c | 6 ++++++ 1 file changed, 6 insertions(+)