| Message ID | 20210825114213.7429-1-vincent.whitchurch@axis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: dw_mmc: Only inject fault before done/error | expand |
On 8/25/21 8:42 PM, Vincent Whitchurch wrote: > The fault injection function can set EVENT_DATA_ERROR but skip the > setting of ->data_status to an error status if it hits just after a data > over interrupt. This confuses the tasklet which can later end up > triggering the WARN_ON(host->cmd || ..) in dw_mci_request_end() since > dw_mci_data_complete() would return success. > > Prevent the fault injection function from doing this since this is not a > real case, and ensure that the fault injection doesn't race with a real > error either. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > --- > drivers/mmc/host/dw_mmc.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 6578cc64ae9e..380f9aa56eb2 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1802,10 +1802,15 @@ static enum hrtimer_restart dw_mci_fault_timer(struct hrtimer *t) > > spin_lock_irqsave(&host->irq_lock, flags); > > - if (!host->data_status) > + /* > + * Only inject an error if we haven't already got an error or data over > + * interrupt. > + */ > + if (!host->data_status) { > host->data_status = SDMMC_INT_DCRC; > - set_bit(EVENT_DATA_ERROR, &host->pending_events); > - tasklet_schedule(&host->tasklet); > + set_bit(EVENT_DATA_ERROR, &host->pending_events); > + tasklet_schedule(&host->tasklet); > + } > > spin_unlock_irqrestore(&host->irq_lock, flags); > > @@ -2721,12 +2726,16 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > } > > if (pending & DW_MCI_DATA_ERROR_FLAGS) { > + spin_lock(&host->irq_lock); > + > /* if there is an error report DATA_ERROR */ > mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); > host->data_status = pending; > smp_wmb(); /* drain writebuffer */ > set_bit(EVENT_DATA_ERROR, &host->pending_events); > tasklet_schedule(&host->tasklet); > + > + spin_unlock(&host->irq_lock); > } > > if (pending & SDMMC_INT_DATA_OVER) { >
On Wed, 25 Aug 2021 at 13:42, Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > The fault injection function can set EVENT_DATA_ERROR but skip the > setting of ->data_status to an error status if it hits just after a data > over interrupt. This confuses the tasklet which can later end up > triggering the WARN_ON(host->cmd || ..) in dw_mci_request_end() since > dw_mci_data_complete() would return success. > > Prevent the fault injection function from doing this since this is not a > real case, and ensure that the fault injection doesn't race with a real > error either. > > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> Applied for fixes and by adding a fixes tag, thanks! Kind regards Uffe > --- > drivers/mmc/host/dw_mmc.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 6578cc64ae9e..380f9aa56eb2 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -1802,10 +1802,15 @@ static enum hrtimer_restart dw_mci_fault_timer(struct hrtimer *t) > > spin_lock_irqsave(&host->irq_lock, flags); > > - if (!host->data_status) > + /* > + * Only inject an error if we haven't already got an error or data over > + * interrupt. > + */ > + if (!host->data_status) { > host->data_status = SDMMC_INT_DCRC; > - set_bit(EVENT_DATA_ERROR, &host->pending_events); > - tasklet_schedule(&host->tasklet); > + set_bit(EVENT_DATA_ERROR, &host->pending_events); > + tasklet_schedule(&host->tasklet); > + } > > spin_unlock_irqrestore(&host->irq_lock, flags); > > @@ -2721,12 +2726,16 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) > } > > if (pending & DW_MCI_DATA_ERROR_FLAGS) { > + spin_lock(&host->irq_lock); > + > /* if there is an error report DATA_ERROR */ > mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); > host->data_status = pending; > smp_wmb(); /* drain writebuffer */ > set_bit(EVENT_DATA_ERROR, &host->pending_events); > tasklet_schedule(&host->tasklet); > + > + spin_unlock(&host->irq_lock); > } > > if (pending & SDMMC_INT_DATA_OVER) { > -- > 2.28.0 >
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 6578cc64ae9e..380f9aa56eb2 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1802,10 +1802,15 @@ static enum hrtimer_restart dw_mci_fault_timer(struct hrtimer *t) spin_lock_irqsave(&host->irq_lock, flags); - if (!host->data_status) + /* + * Only inject an error if we haven't already got an error or data over + * interrupt. + */ + if (!host->data_status) { host->data_status = SDMMC_INT_DCRC; - set_bit(EVENT_DATA_ERROR, &host->pending_events); - tasklet_schedule(&host->tasklet); + set_bit(EVENT_DATA_ERROR, &host->pending_events); + tasklet_schedule(&host->tasklet); + } spin_unlock_irqrestore(&host->irq_lock, flags); @@ -2721,12 +2726,16 @@ static irqreturn_t dw_mci_interrupt(int irq, void *dev_id) } if (pending & DW_MCI_DATA_ERROR_FLAGS) { + spin_lock(&host->irq_lock); + /* if there is an error report DATA_ERROR */ mci_writel(host, RINTSTS, DW_MCI_DATA_ERROR_FLAGS); host->data_status = pending; smp_wmb(); /* drain writebuffer */ set_bit(EVENT_DATA_ERROR, &host->pending_events); tasklet_schedule(&host->tasklet); + + spin_unlock(&host->irq_lock); } if (pending & SDMMC_INT_DATA_OVER) {
The fault injection function can set EVENT_DATA_ERROR but skip the setting of ->data_status to an error status if it hits just after a data over interrupt. This confuses the tasklet which can later end up triggering the WARN_ON(host->cmd || ..) in dw_mci_request_end() since dw_mci_data_complete() would return success. Prevent the fault injection function from doing this since this is not a real case, and ensure that the fault injection doesn't race with a real error either. Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> --- drivers/mmc/host/dw_mmc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)