On Wed, 29 May 2019 at 11:20, Ludovic BARRE <ludovic.barre@xxxxxx> wrote: > > hi Ulf > > On 5/27/19 8:17 PM, Ulf Hansson wrote: > > On Fri, 26 Apr 2019 at 09:46, Ludovic Barre <ludovic.Barre@xxxxxx> wrote: > >> > >> From: Ludovic Barre <ludovic.barre@xxxxxx> > >> > >> The "busy_detect_flag" is used to read/clear busy value of > >> mmci status. The "busy_detect_mask" is used to manage busy irq of > >> mmci mask. > >> For sdmmc variant, the 2 properties have not the same offset. > >> To clear the busyd0 status bit, we must add busy detect flag, > >> the mmci mask is not enough. > >> > >> Signed-off-by: Ludovic Barre <ludovic.barre@xxxxxx> > > > > Ludovic, again, apologies for the delay. > > > >> --- > >> drivers/mmc/host/mmci.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > >> index a040f54..3cd52e8 100644 > >> --- a/drivers/mmc/host/mmci.c > >> +++ b/drivers/mmc/host/mmci.c > >> @@ -1517,7 +1517,8 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) > >> * to make sure that both start and end interrupts are always > >> * cleared one after the other. > >> */ > >> - status &= readl(host->base + MMCIMASK0); > >> + status &= readl(host->base + MMCIMASK0) | > >> + host->variant->busy_detect_flag; > > > > I think this is not entirely correct, because it would mean we check > > for busy even if we haven't unmasked the busy IRQ via the > > variant->busy_detect_mask. > > if the variant is busy_detect false: > => no problem because the busy_detect_flag or busy_detect_mask is not > defined. Right. > > if variant is busy_detect true: > the busy handle is split in 3 steps (see mmci_cmd_irq): > step 1: detection of busy line => unmasked the busy irq end > step 2: in busy wait => ignore cmd irq while current busy flag is > enabled. > step 3: end of busy => clear and mask busy irq > > To detect the first step (see mmci_cmd_irq: which unmasks the busy irq) > we need to know the current busy state. Actually, the status register is > re-read in mmci_cmd_irq, why not used the status read in mmci_irq and in > parameter ? Right, I see your point. On the other hand, that re-read of the status registers should really not be needed. Maybe it's a leftover from my initial version of the code, but in any case we should remove that. > > Regards, > Ludo > > > > > I suggest to store a new bool in the host (call it > > "busy_detect_unmasked" or whatever makes sense to you), to track > > whether we have unmasked the busy IRQ or not. Then take this flag into > > account, before ORing the value of host->variant->busy_detect_flag, > > according to above. > > > >> if (host->variant->busy_detect) > >> writel(status & ~host->variant->busy_detect_mask, > >> host->base + MMCICLEAR); > >> -- > >> 2.7.4 Kind regards Uffe
