From: Ulf Hansson <ulf.hansson@linaro.org> To: Ludovic BARRE <ludovic.barre@st.com> Cc: Rob Herring <robh+dt@kernel.org>, Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Alexandre Torgue <alexandre.torgue@st.com>, Linux ARM <linux-arm-kernel@lists.infradead.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, DTML <devicetree@vger.kernel.org>, "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>, linux-stm32@st-md-mailman.stormreply.com Subject: Re: [PATCH V2 3/5] mmc: mmci: fix clear of busy detect status Date: Wed, 29 May 2019 12:36:50 +0200 [thread overview] Message-ID: <CAPDyKFqzNy3hDzovSxb6pd37u7SxWqYoJeu9+tf_qDF7gs0euA@mail.gmail.com> (raw) In-Reply-To: <a6a74431-50a6-6298-b77d-9d27a77fd6cc@st.com> On Wed, 29 May 2019 at 11:20, Ludovic BARRE <ludovic.barre@st.com> 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@st.com> wrote: > >> > >> From: Ludovic Barre <ludovic.barre@st.com> > >> > >> 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@st.com> > > > > 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
WARNING: multiple messages have this Message-ID (diff)
From: Ulf Hansson <ulf.hansson@linaro.org> To: Ludovic BARRE <ludovic.barre@st.com> Cc: DTML <devicetree@vger.kernel.org>, Alexandre Torgue <alexandre.torgue@st.com>, "linux-mmc@vger.kernel.org" <linux-mmc@vger.kernel.org>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>, Srinivas Kandagatla <srinivas.kandagatla@linaro.org>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, linux-stm32@st-md-mailman.stormreply.com, Linux ARM <linux-arm-kernel@lists.infradead.org> Subject: Re: [PATCH V2 3/5] mmc: mmci: fix clear of busy detect status Date: Wed, 29 May 2019 12:36:50 +0200 [thread overview] Message-ID: <CAPDyKFqzNy3hDzovSxb6pd37u7SxWqYoJeu9+tf_qDF7gs0euA@mail.gmail.com> (raw) In-Reply-To: <a6a74431-50a6-6298-b77d-9d27a77fd6cc@st.com> On Wed, 29 May 2019 at 11:20, Ludovic BARRE <ludovic.barre@st.com> 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@st.com> wrote: > >> > >> From: Ludovic Barre <ludovic.barre@st.com> > >> > >> 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@st.com> > > > > 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 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-05-29 10:37 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-04-26 7:46 [PATCH V2 0/5] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` [PATCH V2 1/5] mmc: mmci: cleanup mmci_cmd_irq for busy detect feature Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` [PATCH V2 2/5] mmc: mmci: avoid fake busy polling in mmci_irq Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` [PATCH V2 3/5] mmc: mmci: fix clear of busy detect status Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-05-27 18:17 ` Ulf Hansson 2019-05-27 18:17 ` Ulf Hansson 2019-05-29 9:20 ` Ludovic BARRE 2019-05-29 9:20 ` Ludovic BARRE 2019-05-29 9:20 ` Ludovic BARRE 2019-05-29 10:36 ` Ulf Hansson [this message] 2019-05-29 10:36 ` Ulf Hansson 2019-04-26 7:46 ` [PATCH V2 4/5] mmc: mmci: add hardware busy timeout feature Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` [PATCH V2 5/5] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-26 7:46 ` Ludovic Barre 2019-04-30 11:13 ` [PATCH V2 0/5] " Ulf Hansson 2019-04-30 11:13 ` Ulf Hansson 2019-04-30 12:06 ` Ludovic BARRE 2019-04-30 12:06 ` Ludovic BARRE 2019-04-30 12:06 ` Ludovic BARRE 2019-05-03 13:29 ` Ulf Hansson 2019-05-03 13:29 ` Ulf Hansson 2019-05-03 15:15 ` Ludovic BARRE 2019-05-03 15:15 ` Ludovic BARRE 2019-05-03 15:15 ` Ludovic BARRE 2019-05-21 7:38 ` Ludovic BARRE 2019-05-21 7:38 ` Ludovic BARRE 2019-05-21 7:38 ` Ludovic BARRE 2019-05-21 7:56 ` Ulf Hansson 2019-05-21 7:56 ` Ulf Hansson 2019-05-21 9:17 ` Ludovic BARRE 2019-05-21 9:17 ` Ludovic BARRE 2019-05-21 9:17 ` Ludovic BARRE
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAPDyKFqzNy3hDzovSxb6pd37u7SxWqYoJeu9+tf_qDF7gs0euA@mail.gmail.com \ --to=ulf.hansson@linaro.org \ --cc=alexandre.torgue@st.com \ --cc=devicetree@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mmc@vger.kernel.org \ --cc=linux-stm32@st-md-mailman.stormreply.com \ --cc=ludovic.barre@st.com \ --cc=mcoquelin.stm32@gmail.com \ --cc=robh+dt@kernel.org \ --cc=srinivas.kandagatla@linaro.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.