* [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant @ 2019-06-03 15:55 Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 1/3] mmc: mmci: fix read status for busy detect Ludovic Barre ` (3 more replies) 0 siblings, 4 replies; 12+ messages in thread From: Ludovic Barre @ 2019-06-03 15:55 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree, linux-mmc, linux-stm32, Ludovic Barre From: Ludovic Barre <ludovic.barre@st.com> This patch series adds busy detect for stm32 sdmmc variant. Some adaptations are required: -Clear busy status bit if busy_detect_flag and busy_detect_mask are different. -Add hardware busy timeout with MMCIDATATIMER register. V3: -rebase on latest mmc next -replace re-read by status parameter. V2: -mmci_cmd_irq cleanup in separate patch. -simplify the busy_detect_flag exclude -replace sdmmc specific comment in "mmc: mmci: avoid fake busy polling in mmci_irq" to focus on common behavior Ludovic Barre (3): mmc: mmci: fix read status for busy detect mmc: mmci: add hardware busy timeout feature mmc: mmci: add busy detect for stm32 sdmmc variant drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- drivers/mmc/host/mmci.h | 3 +++ 2 files changed, 44 insertions(+), 8 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V3 1/3] mmc: mmci: fix read status for busy detect 2019-06-03 15:55 [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre @ 2019-06-03 15:55 ` Ludovic Barre 2019-07-15 16:31 ` Ulf Hansson 2019-06-03 15:55 ` [PATCH V3 2/3] mmc: mmci: add hardware busy timeout feature Ludovic Barre ` (2 subsequent siblings) 3 siblings, 1 reply; 12+ messages in thread From: Ludovic Barre @ 2019-06-03 15:55 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree, linux-mmc, linux-stm32, Ludovic Barre From: Ludovic Barre <ludovic.barre@st.com> "busy_detect_flag" is used to read & clear busy value of mmci status. "busy_detect_mask" is used to manage busy irq of mmci mask. So to read mmci status the busy_detect_flag must be take account. if the variant does not support busy detect feature the flag is null and there is no impact. Not need to re-read the status register in mmci_cmd_irq, the status parameter can be used. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- drivers/mmc/host/mmci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 356833a..5b5cc45 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, */ if (!host->busy_status && !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + (status & host->variant->busy_detect_flag)) { /* Clear the busy start IRQ */ writel(host->variant->busy_detect_mask, @@ -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; if (host->variant->busy_detect) writel(status & ~host->variant->busy_detect_mask, host->base + MMCICLEAR); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V3 1/3] mmc: mmci: fix read status for busy detect 2019-06-03 15:55 ` [PATCH V3 1/3] mmc: mmci: fix read status for busy detect Ludovic Barre @ 2019-07-15 16:31 ` Ulf Hansson 2019-07-26 9:41 ` Ludovic BARRE 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2019-07-15 16:31 UTC (permalink / raw) To: Ludovic Barre Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 On Mon, 3 Jun 2019 at 17:55, Ludovic Barre <ludovic.Barre@st.com> wrote: > > From: Ludovic Barre <ludovic.barre@st.com> > > "busy_detect_flag" is used to read & clear busy value of mmci status. > "busy_detect_mask" is used to manage busy irq of mmci mask. > So to read mmci status the busy_detect_flag must be take account. > if the variant does not support busy detect feature the flag is null > and there is no impact. By reading the changelog, it doesn't tell me the purpose of this change. When going forward, please work harder on your changelogs. Make sure to answer the questions, *why* is this change needed, *what/how* does the change do. > > Not need to re-read the status register in mmci_cmd_irq, the > status parameter can be used. > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > --- > drivers/mmc/host/mmci.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index 356833a..5b5cc45 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, > */ > if (!host->busy_status && > !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && > - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { > + (status & host->variant->busy_detect_flag)) { I suggested you to do this change through some of my earlier comments, however I think it should be made as a stand alone change. Anyway, when looking at the details in your series, I decided to try to help out a bit, so I have prepared a couple of related patches for cleaning up and clarifying the busy detection code/comments in mmci. I have incorporated the above change, so let me post them asap. > > /* Clear the busy start IRQ */ > writel(host->variant->busy_detect_mask, > @@ -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; As I told earlier in the review, this looks wrong to me. It means that you will add the bit for the ->busy_detect_flag to the status field we have just read from the MMCISTATUS register. That means the busy status may be set when it shouldn't. > if (host->variant->busy_detect) > writel(status & ~host->variant->busy_detect_mask, > host->base + MMCICLEAR); > -- > 2.7.4 > By looking at the other changes in the series, I assume @subject patch is intended to prepare for the other changes on top. But it's not really clear. Anyway, in that regards, the below is my observations of what seems to be important part, when supporting busy detection for the stm32 sdmmc variant (except the timeout things in patch2, which I intend to comment separately on). I figured, these are the involved register bits/masks: MMCISTATUS: MCI_STM32_BUSYD0 BIT(20) MCI_STM32_BUSYD0END BIT(21) MMCIMASK0: MCI_STM32_BUSYD0ENDMASK BIT(21) For the legacy ST variant, there is only one register bit in MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)). There is no dedicated busy-end bit for the busy-end IRQ, which I believe is the reason to why the current code also is bit messy. It seems like the stm32 sdmmc variant have a separate status bit for the busy-end IRQ, correct? If I understand correctly by looking at patch3, you don't use the dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why not? Thoughts? Kind regards Uffe ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V3 1/3] mmc: mmci: fix read status for busy detect 2019-07-15 16:31 ` Ulf Hansson @ 2019-07-26 9:41 ` Ludovic BARRE 2019-08-05 7:33 ` [Linux-stm32] " Ludovic BARRE 0 siblings, 1 reply; 12+ messages in thread From: Ludovic BARRE @ 2019-07-26 9:41 UTC (permalink / raw) To: Ulf Hansson Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 hi Ulf Thanks to your "Clarify comments ..." commit, like is closes I resumed upstream of this series. On 7/15/19 6:31 PM, Ulf Hansson wrote: > On Mon, 3 Jun 2019 at 17:55, Ludovic Barre <ludovic.Barre@st.com> wrote: >> >> From: Ludovic Barre <ludovic.barre@st.com> >> >> "busy_detect_flag" is used to read & clear busy value of mmci status. >> "busy_detect_mask" is used to manage busy irq of mmci mask. >> So to read mmci status the busy_detect_flag must be take account. >> if the variant does not support busy detect feature the flag is null >> and there is no impact. > > By reading the changelog, it doesn't tell me the purpose of this > change. When going forward, please work harder on your changelogs. > > Make sure to answer the questions, *why* is this change needed, > *what/how* does the change do. Ok, I will explain the differences with the legacy and the needs of sdmmc variant about busy detection. > >> >> Not need to re-read the status register in mmci_cmd_irq, the >> status parameter can be used. >> >> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >> --- >> drivers/mmc/host/mmci.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >> index 356833a..5b5cc45 100644 >> --- a/drivers/mmc/host/mmci.c >> +++ b/drivers/mmc/host/mmci.c >> @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, >> */ >> if (!host->busy_status && >> !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && >> - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { >> + (status & host->variant->busy_detect_flag)) { > > I suggested you to do this change through some of my earlier comments, > however I think it should be made as a stand alone change. > > Anyway, when looking at the details in your series, I decided to try > to help out a bit, so I have prepared a couple of related patches for > cleaning up and clarifying the busy detection code/comments in mmci. I > have incorporated the above change, so let me post them asap. > >> >> /* Clear the busy start IRQ */ >> writel(host->variant->busy_detect_mask, >> @@ -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; > > As I told earlier in the review, this looks wrong to me. > > It means that you will add the bit for the ->busy_detect_flag to the > status field we have just read from the MMCISTATUS register. That > means the busy status may be set when it shouldn't. > >> if (host->variant->busy_detect) >> writel(status & ~host->variant->busy_detect_mask, >> host->base + MMCICLEAR); >> -- >> 2.7.4 >> > > By looking at the other changes in the series, I assume @subject patch > is intended to prepare for the other changes on top. But it's not > really clear. > > Anyway, in that regards, the below is my observations of what seems to > be important part, when supporting busy detection for the stm32 sdmmc > variant (except the timeout things in patch2, which I intend to > comment separately on). > > I figured, these are the involved register bits/masks: > > MMCISTATUS: > MCI_STM32_BUSYD0 BIT(20) > MCI_STM32_BUSYD0END BIT(21) > > MMCIMASK0: > MCI_STM32_BUSYD0ENDMASK BIT(21) it's exact: MCI_STM32_BUSYD0 BIT(20): This is a hardware status flag only (inverted value of d0 line), it does not generate an interrupt, and so no mask bit. MCI_STM32_BUSYD0ENDMASK BIT(21): This indicates only end of busy following a CMD response. On busy to Not busy changes, an interrupt is generated (if unmask) and BUSYD0END status flag is set. status flag is cleared by writing corresponding interrupt clear bit in MMCICLEAR. > > For the legacy ST variant, there is only one register bit in > MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)). > There is no dedicated busy-end bit for the busy-end IRQ, which I > believe is the reason to why the current code also is bit messy. yes > > It seems like the stm32 sdmmc variant have a separate status bit for > the busy-end IRQ, correct? yes > > If I understand correctly by looking at patch3, you don't use the > dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why > not? like your are clarify in previous series, the busy detection is done in 3 steps. if I use: .busy_detect_flag = MCI_STM32_BUSYD0ENDMASK, .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, the sdmmc request will be not correctly completed, because the third step can't be happen. chronologies: step1: when busyd0end change to 1 => busyd0end interrupt is unmasked => busy_status = cmd_sent | respend => return to mmci_irq step2: busyd0end is yet to 1 => clear the busyd0end interrupt => the hardware clear busyd0end status flag on interrupt clear => return to mmci_irq like MCI_STM32_BUSYD0END interrupt is generated only on change busy to not busy, when the interrupt is cleared (status is 0) the step 3 can't happen (no irq pending to re-enter in mmci_cmd_irq). sdmmc can't complete the request. If I use: .busy_detect_flag = MCI_STM32_BUSYD0, .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, Like there is no MCI_STM32_BUSYD0 irq mask, the status read in mmci_irq "status &= readl(host->base + MMCIMASK0)" can't take account the busy_detect_flag (for sdmmc). So the step 2 can't be passed. However we could share re-read between step 1 and step 2. proposal: + + u32 busy_val = readl(base + MMCISTATUS) & + host->variant->busy_detect_flag; + if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && - (readl(base + MMCISTATUS) & host->variant->busy_detect_flag)) { + !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && busy_val) { writel(readl(base + MMCIMASK0) | host->variant->busy_detect_mask, @@ -1262,8 +1265,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * both the start and the end interrupts needs to be cleared, * one after the other. So, clear the busy start IRQ here. */ - if (host->busy_status && - (status & host->variant->busy_detect_flag)) { + if (host->busy_status && busy_val) { what do you think about it ? > > Thoughts? > > Kind regards > Uffe > Regards Ludo ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Linux-stm32] [PATCH V3 1/3] mmc: mmci: fix read status for busy detect 2019-07-26 9:41 ` Ludovic BARRE @ 2019-08-05 7:33 ` Ludovic BARRE 0 siblings, 0 replies; 12+ messages in thread From: Ludovic BARRE @ 2019-08-05 7:33 UTC (permalink / raw) To: Ulf Hansson Cc: DTML, linux-mmc, Linux Kernel Mailing List, Rob Herring, Srinivas Kandagatla, Maxime Coquelin, linux-stm32, Linux ARM hi Ulf On 7/26/19 11:41 AM, Ludovic BARRE wrote: > hi Ulf > > Thanks to your "Clarify comments ..." commit, like is closes > I resumed upstream of this series. > > On 7/15/19 6:31 PM, Ulf Hansson wrote: >> On Mon, 3 Jun 2019 at 17:55, Ludovic Barre <ludovic.Barre@st.com> wrote: >>> >>> From: Ludovic Barre <ludovic.barre@st.com> >>> >>> "busy_detect_flag" is used to read & clear busy value of mmci status. >>> "busy_detect_mask" is used to manage busy irq of mmci mask. >>> So to read mmci status the busy_detect_flag must be take account. >>> if the variant does not support busy detect feature the flag is null >>> and there is no impact. >> >> By reading the changelog, it doesn't tell me the purpose of this >> change. When going forward, please work harder on your changelogs. >> >> Make sure to answer the questions, *why* is this change needed, >> *what/how* does the change do. > > Ok, I will explain the differences with the legacy and the needs of > sdmmc variant about busy detection. > >> >>> >>> Not need to re-read the status register in mmci_cmd_irq, the >>> status parameter can be used. >>> >>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >>> --- >>> drivers/mmc/host/mmci.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >>> index 356833a..5b5cc45 100644 >>> --- a/drivers/mmc/host/mmci.c >>> +++ b/drivers/mmc/host/mmci.c >>> @@ -1240,7 +1240,7 @@ mmci_cmd_irq(struct mmci_host *host, struct >>> mmc_command *cmd, >>> */ >>> if (!host->busy_status && >>> !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && >>> - (readl(base + MMCISTATUS) & >>> host->variant->busy_detect_flag)) { >>> + (status & host->variant->busy_detect_flag)) { >> >> I suggested you to do this change through some of my earlier comments, >> however I think it should be made as a stand alone change. >> >> Anyway, when looking at the details in your series, I decided to try >> to help out a bit, so I have prepared a couple of related patches for >> cleaning up and clarifying the busy detection code/comments in mmci. I >> have incorporated the above change, so let me post them asap. >> >>> >>> /* Clear the busy start IRQ */ >>> writel(host->variant->busy_detect_mask, >>> @@ -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; >> >> As I told earlier in the review, this looks wrong to me. >> >> It means that you will add the bit for the ->busy_detect_flag to the >> status field we have just read from the MMCISTATUS register. That >> means the busy status may be set when it shouldn't. >> >>> if (host->variant->busy_detect) >>> writel(status & >>> ~host->variant->busy_detect_mask, >>> host->base + MMCICLEAR); >>> -- >>> 2.7.4 >>> >> >> By looking at the other changes in the series, I assume @subject patch >> is intended to prepare for the other changes on top. But it's not >> really clear. >> >> Anyway, in that regards, the below is my observations of what seems to >> be important part, when supporting busy detection for the stm32 sdmmc >> variant (except the timeout things in patch2, which I intend to >> comment separately on). >> >> I figured, these are the involved register bits/masks: >> >> MMCISTATUS: >> MCI_STM32_BUSYD0 BIT(20) >> MCI_STM32_BUSYD0END BIT(21) >> >> MMCIMASK0: >> MCI_STM32_BUSYD0ENDMASK BIT(21) > > it's exact: > MCI_STM32_BUSYD0 BIT(20): This is a hardware status flag only (inverted > value of d0 line), it does not generate an interrupt, and so no mask > bit. > > MCI_STM32_BUSYD0ENDMASK BIT(21): This indicates only end of busy > following a CMD response. On busy to Not busy changes, an interrupt > is generated (if unmask) and BUSYD0END status flag is set. > status flag is cleared by writing corresponding interrupt clear bit in > MMCICLEAR. > >> >> For the legacy ST variant, there is only one register bit in >> MMCISTATUS that is used for indicating busy (MCI_ST_CARDBUSY BIT(24)). >> There is no dedicated busy-end bit for the busy-end IRQ, which I >> believe is the reason to why the current code also is bit messy. > > yes > >> >> It seems like the stm32 sdmmc variant have a separate status bit for >> the busy-end IRQ, correct? > > yes > >> >> If I understand correctly by looking at patch3, you don't use the >> dedicated busy-end status bit (MCI_STM32_BUSYD0END), right? Then why >> not? > > like your are clarify in previous series, the busy detection is done > in 3 steps. > > if I use: > .busy_detect_flag = MCI_STM32_BUSYD0ENDMASK, > .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, > > the sdmmc request will be not correctly completed, because the third > step can't be happen. > > chronologies: > step1: when busyd0end change to 1 > => busyd0end interrupt is unmasked > => busy_status = cmd_sent | respend > => return to mmci_irq > step2: busyd0end is yet to 1 > => clear the busyd0end interrupt > => the hardware clear busyd0end status flag on interrupt clear > => return to mmci_irq > > like MCI_STM32_BUSYD0END interrupt is generated only on change > busy to not busy, when the interrupt is cleared (status is 0) > the step 3 can't happen (no irq pending to re-enter in mmci_cmd_irq). > sdmmc can't complete the request. > > If I use: > .busy_detect_flag = MCI_STM32_BUSYD0, > .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, > > Like there is no MCI_STM32_BUSYD0 irq mask, the status read in mmci_irq > "status &= readl(host->base + MMCIMASK0)" can't take account the > busy_detect_flag (for sdmmc). So the step 2 can't be passed. > However we could share re-read between step 1 and step 2. > > proposal: > > + > + u32 busy_val = readl(base + MMCISTATUS) & > + host->variant->busy_detect_flag; > + > if (!host->busy_status && > - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && > - (readl(base + MMCISTATUS) & > host->variant->busy_detect_flag)) { > + !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && busy_val) { > > writel(readl(base + MMCIMASK0) | > host->variant->busy_detect_mask, > @@ -1262,8 +1265,7 @@ mmci_cmd_irq(struct mmci_host *host, struct > mmc_command *cmd, > * both the start and the end interrupts needs to be cleared, > * one after the other. So, clear the busy start IRQ here. > */ > - if (host->busy_status && > - (status & host->variant->busy_detect_flag)) { > + if (host->busy_status && busy_val) { > > > what do you think about it ? > I give up this proposal for a new version based on mmci_host_ops callback to done the busy completion. >> >> Thoughts? >> >> Kind regards >> Uffe >> > > Regards > Ludo > _______________________________________________ > Linux-stm32 mailing list > Linux-stm32@st-md-mailman.stormreply.com > https://st-md-mailman.stormreply.com/mailman/listinfo/linux-stm32 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH V3 2/3] mmc: mmci: add hardware busy timeout feature 2019-06-03 15:55 [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 1/3] mmc: mmci: fix read status for busy detect Ludovic Barre @ 2019-06-03 15:55 ` Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 3/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-06-13 13:02 ` [PATCH V3 0/3] " Ludovic BARRE 3 siblings, 0 replies; 12+ messages in thread From: Ludovic Barre @ 2019-06-03 15:55 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree, linux-mmc, linux-stm32, Ludovic Barre From: Ludovic Barre <ludovic.barre@st.com> In some variants, the data timer is enabled when the DPSM is in busy state (while data transfer or MMC_RSP_BUSY), and could generate a data timeout error if the counter reach 0. -Define max_busy_timeout (in ms) according to clock. -Set data timer register if the command has rsp_busy flag. If busy_timeout is not defined by framework, the busy length after Data Burst is defined as 1 second (refer: 4.6.2.2 Write of sd specification part1 v6-0). -Add MCI_DATATIMEOUT error management in mmci_cmd_irq. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- drivers/mmc/host/mmci.c | 40 ++++++++++++++++++++++++++++++++++------ drivers/mmc/host/mmci.h | 2 ++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 5b5cc45..5a8b232 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -1078,6 +1078,7 @@ static void mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) { void __iomem *base = host->base; + unsigned long long clks = 0; dev_dbg(mmc_dev(host->mmc), "op %02x arg %08x flags %08x\n", cmd->opcode, cmd->arg, cmd->flags); @@ -1100,6 +1101,19 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) else c |= host->variant->cmdreg_srsp; } + + if (host->variant->busy_timeout && !cmd->data) { + if (cmd->flags & MMC_RSP_BUSY) { + if (!cmd->busy_timeout) + cmd->busy_timeout = 1000; + + clks = (unsigned long long)cmd->busy_timeout; + clks *= host->cclk; + do_div(clks, MSEC_PER_SEC); + } + writel_relaxed(clks, host->base + MMCIDATATIMER); + } + if (/*interrupt*/0) c |= MCI_CPSM_INTERRUPT; @@ -1206,6 +1220,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, { void __iomem *base = host->base; bool sbc, busy_resp; + u32 err_msk; if (!cmd) return; @@ -1218,8 +1233,12 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * handling. Note that we tag on any latent IRQs postponed * due to waiting for busy status. */ - if (!((status|host->busy_status) & - (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT|MCI_CMDSENT|MCI_CMDRESPEND))) + err_msk = MCI_CMDCRCFAIL | MCI_CMDTIMEOUT; + if (host->variant->busy_timeout && busy_resp) + err_msk |= MCI_DATATIMEOUT; + + if (!((status | host->busy_status) & + (err_msk | MCI_CMDSENT | MCI_CMDRESPEND))) return; /* @@ -1228,7 +1247,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, if (busy_resp && host->variant->busy_detect) { /* We are busy with a command, return */ - if (host->busy_status && + if (host->busy_status && !(status & (err_msk)) && (status & host->variant->busy_detect_flag)) return; @@ -1238,8 +1257,7 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, * that the special busy status bit is still set before * proceeding. */ - if (!host->busy_status && - !(status & (MCI_CMDCRCFAIL|MCI_CMDTIMEOUT)) && + if (!host->busy_status && !(status & (err_msk)) && (status & host->variant->busy_detect_flag)) { /* Clear the busy start IRQ */ @@ -1282,6 +1300,9 @@ mmci_cmd_irq(struct mmci_host *host, struct mmc_command *cmd, cmd->error = -ETIMEDOUT; } else if (status & MCI_CMDCRCFAIL && cmd->flags & MMC_RSP_CRC) { cmd->error = -EILSEQ; + } else if (host->variant->busy_timeout && busy_resp && + status & MCI_DATATIMEOUT) { + cmd->error = -ETIMEDOUT; } else { cmd->resp[0] = readl(base + MMCIRESPONSE0); cmd->resp[1] = readl(base + MMCIRESPONSE1); @@ -1543,6 +1564,7 @@ static irqreturn_t mmci_irq(int irq, void *dev_id) status &= ~host->variant->busy_detect_flag; ret = 1; + } while (status); spin_unlock(&host->lock); @@ -1947,6 +1969,8 @@ static int mmci_probe(struct amba_device *dev, * Enable busy detection. */ if (variant->busy_detect) { + u32 max_busy_timeout = 0; + mmci_ops.card_busy = mmci_card_busy; /* * Not all variants have a flag to enable busy detection @@ -1956,7 +1980,11 @@ static int mmci_probe(struct amba_device *dev, mmci_write_datactrlreg(host, host->variant->busy_dpsm_flag); mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY; - mmc->max_busy_timeout = 0; + + if (variant->busy_timeout) + max_busy_timeout = ~0UL / (mmc->f_max / MSEC_PER_SEC); + + mmc->max_busy_timeout = max_busy_timeout; } /* Prepare a CMD12 - needed to clear the DPSM on some variants. */ diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 4f071bd..b43a958 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -290,6 +290,7 @@ struct mmci_host; * @signal_direction: input/out direction of bus signals can be indicated * @pwrreg_clkgate: MMCIPOWER register must be used to gate the clock * @busy_detect: true if the variant supports busy detection on DAT0. + * @busy_timeout: true if the variant supports hardware busy timeout on R1B. * @busy_dpsm_flag: bitmask enabling busy detection in the DPSM * @busy_detect_flag: bitmask identifying the bit in the MMCISTATUS register * indicating that the card is busy @@ -336,6 +337,7 @@ struct variant_data { u8 signal_direction:1; u8 pwrreg_clkgate:1; u8 busy_detect:1; + u8 busy_timeout:1; u32 busy_dpsm_flag; u32 busy_detect_flag; u32 busy_detect_mask; -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH V3 3/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-06-03 15:55 [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 1/3] mmc: mmci: fix read status for busy detect Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 2/3] mmc: mmci: add hardware busy timeout feature Ludovic Barre @ 2019-06-03 15:55 ` Ludovic Barre 2019-06-13 13:02 ` [PATCH V3 0/3] " Ludovic BARRE 3 siblings, 0 replies; 12+ messages in thread From: Ludovic Barre @ 2019-06-03 15:55 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree, linux-mmc, linux-stm32, Ludovic Barre From: Ludovic Barre <ludovic.barre@st.com> This patch enables busy detection for stm32 sdmmc which requires to set data timer to define the busy timeout. sdmmc has 2 flags: -busyd0: inverted value of d0 line. -busyd0end which indicates only end of busy following a cmd response. Only one interrupt on busyd0end. Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- drivers/mmc/host/mmci.c | 4 ++++ drivers/mmc/host/mmci.h | 1 + 2 files changed, 5 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 5a8b232..6210f1f 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -264,6 +264,10 @@ static struct variant_data variant_stm32_sdmmc = { .datalength_bits = 25, .datactrl_blocksz = 14, .stm32_idmabsize_mask = GENMASK(12, 5), + .busy_detect = true, + .busy_timeout = true, + .busy_detect_flag = MCI_STM32_BUSYD0, + .busy_detect_mask = MCI_STM32_BUSYD0ENDMASK, .init = sdmmc_variant_init, }; diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index b43a958..ac19de8 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -167,6 +167,7 @@ #define MCI_ST_CARDBUSY (1 << 24) /* Extended status bits for the STM32 variants */ #define MCI_STM32_BUSYD0 BIT(20) +#define MCI_STM32_BUSYD0END BIT(21) #define MMCICLEAR 0x038 #define MCI_CMDCRCFAILCLR (1 << 0) -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-06-03 15:55 [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre ` (2 preceding siblings ...) 2019-06-03 15:55 ` [PATCH V3 3/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre @ 2019-06-13 13:02 ` Ludovic BARRE 2019-06-13 13:13 ` Ulf Hansson 3 siblings, 1 reply; 12+ messages in thread From: Ludovic BARRE @ 2019-06-13 13:02 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: srinivas.kandagatla, Maxime Coquelin, Alexandre Torgue, linux-arm-kernel, linux-kernel, devicetree, linux-mmc, linux-stm32 hi Ulf Just a "gentleman ping" about this series. I know you are busy, it's just to be sure you do not forget me :-) Regards Ludo On 6/3/19 5:55 PM, Ludovic Barre wrote: > From: Ludovic Barre <ludovic.barre@st.com> > > This patch series adds busy detect for stm32 sdmmc variant. > Some adaptations are required: > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > different. > -Add hardware busy timeout with MMCIDATATIMER register. > > V3: > -rebase on latest mmc next > -replace re-read by status parameter. > > V2: > -mmci_cmd_irq cleanup in separate patch. > -simplify the busy_detect_flag exclude > -replace sdmmc specific comment in > "mmc: mmci: avoid fake busy polling in mmci_irq" > to focus on common behavior > > Ludovic Barre (3): > mmc: mmci: fix read status for busy detect > mmc: mmci: add hardware busy timeout feature > mmc: mmci: add busy detect for stm32 sdmmc variant > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > drivers/mmc/host/mmci.h | 3 +++ > 2 files changed, 44 insertions(+), 8 deletions(-) > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-06-13 13:02 ` [PATCH V3 0/3] " Ludovic BARRE @ 2019-06-13 13:13 ` Ulf Hansson 2019-06-20 13:50 ` Ulf Hansson 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2019-06-13 13:13 UTC (permalink / raw) To: Ludovic BARRE Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote: > > hi Ulf > > Just a "gentleman ping" about this series. > I know you are busy, it's just to be sure you do not forget me :-) Thanks! I started briefly to review, but got distracted again. I will come to it, but it just seems to take more time than it should, my apologies. Br Uffe > > Regards > Ludo > > On 6/3/19 5:55 PM, Ludovic Barre wrote: > > From: Ludovic Barre <ludovic.barre@st.com> > > > > This patch series adds busy detect for stm32 sdmmc variant. > > Some adaptations are required: > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > > different. > > -Add hardware busy timeout with MMCIDATATIMER register. > > > > V3: > > -rebase on latest mmc next > > -replace re-read by status parameter. > > > > V2: > > -mmci_cmd_irq cleanup in separate patch. > > -simplify the busy_detect_flag exclude > > -replace sdmmc specific comment in > > "mmc: mmci: avoid fake busy polling in mmci_irq" > > to focus on common behavior > > > > Ludovic Barre (3): > > mmc: mmci: fix read status for busy detect > > mmc: mmci: add hardware busy timeout feature > > mmc: mmci: add busy detect for stm32 sdmmc variant > > > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > > drivers/mmc/host/mmci.h | 3 +++ > > 2 files changed, 44 insertions(+), 8 deletions(-) > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-06-13 13:13 ` Ulf Hansson @ 2019-06-20 13:50 ` Ulf Hansson 2019-07-15 7:39 ` Ludovic BARRE 0 siblings, 1 reply; 12+ messages in thread From: Ulf Hansson @ 2019-06-20 13:50 UTC (permalink / raw) To: Ludovic BARRE Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 Hi Ludovic, On Thu, 13 Jun 2019 at 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote: > > > > hi Ulf > > > > Just a "gentleman ping" about this series. > > I know you are busy, it's just to be sure you do not forget me :-) > > Thanks! I started briefly to review, but got distracted again. I will > come to it, but it just seems to take more time than it should, my > apologies. Alright, so I planned to review this this week - but failed. I have been overwhelmed with work lately (as usual when vacation is getting closer). I need to gently request to come back to this as of week 28, when I will give this the highest prio. Again apologize for the delays! Kind regards Uffe > > Br > Uffe > > > > > Regards > > Ludo > > > > On 6/3/19 5:55 PM, Ludovic Barre wrote: > > > From: Ludovic Barre <ludovic.barre@st.com> > > > > > > This patch series adds busy detect for stm32 sdmmc variant. > > > Some adaptations are required: > > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > > > different. > > > -Add hardware busy timeout with MMCIDATATIMER register. > > > > > > V3: > > > -rebase on latest mmc next > > > -replace re-read by status parameter. > > > > > > V2: > > > -mmci_cmd_irq cleanup in separate patch. > > > -simplify the busy_detect_flag exclude > > > -replace sdmmc specific comment in > > > "mmc: mmci: avoid fake busy polling in mmci_irq" > > > to focus on common behavior > > > > > > Ludovic Barre (3): > > > mmc: mmci: fix read status for busy detect > > > mmc: mmci: add hardware busy timeout feature > > > mmc: mmci: add busy detect for stm32 sdmmc variant > > > > > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > > > drivers/mmc/host/mmci.h | 3 +++ > > > 2 files changed, 44 insertions(+), 8 deletions(-) > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-06-20 13:50 ` Ulf Hansson @ 2019-07-15 7:39 ` Ludovic BARRE 2019-07-15 8:01 ` Ulf Hansson 0 siblings, 1 reply; 12+ messages in thread From: Ludovic BARRE @ 2019-07-15 7:39 UTC (permalink / raw) To: Ulf Hansson Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre TORGUE, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 Hi Ulf like scheduled, I send you a "gentleman ping" about this series. Regards, Ludo ________________________________________ De : Ulf Hansson <ulf.hansson@linaro.org> Envoyé : jeudi 20 juin 2019 15:50 À : Ludovic BARRE Cc : Rob Herring; Srinivas Kandagatla; Maxime Coquelin; Alexandre TORGUE; Linux ARM; Linux Kernel Mailing List; DTML; linux-mmc@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com Objet : Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Hi Ludovic, On Thu, 13 Jun 2019 at 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote: > > > > hi Ulf > > > > Just a "gentleman ping" about this series. > > I know you are busy, it's just to be sure you do not forget me :-) > > Thanks! I started briefly to review, but got distracted again. I will > come to it, but it just seems to take more time than it should, my > apologies. Alright, so I planned to review this this week - but failed. I have been overwhelmed with work lately (as usual when vacation is getting closer). I need to gently request to come back to this as of week 28, when I will give this the highest prio. Again apologize for the delays! Kind regards Uffe > > Br > Uffe > > > > > Regards > > Ludo > > > > On 6/3/19 5:55 PM, Ludovic Barre wrote: > > > From: Ludovic Barre <ludovic.barre@st.com> > > > > > > This patch series adds busy detect for stm32 sdmmc variant. > > > Some adaptations are required: > > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > > > different. > > > -Add hardware busy timeout with MMCIDATATIMER register. > > > > > > V3: > > > -rebase on latest mmc next > > > -replace re-read by status parameter. > > > > > > V2: > > > -mmci_cmd_irq cleanup in separate patch. > > > -simplify the busy_detect_flag exclude > > > -replace sdmmc specific comment in > > > "mmc: mmci: avoid fake busy polling in mmci_irq" > > > to focus on common behavior > > > > > > Ludovic Barre (3): > > > mmc: mmci: fix read status for busy detect > > > mmc: mmci: add hardware busy timeout feature > > > mmc: mmci: add busy detect for stm32 sdmmc variant > > > > > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > > > drivers/mmc/host/mmci.h | 3 +++ > > > 2 files changed, 44 insertions(+), 8 deletions(-) > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant 2019-07-15 7:39 ` Ludovic BARRE @ 2019-07-15 8:01 ` Ulf Hansson 0 siblings, 0 replies; 12+ messages in thread From: Ulf Hansson @ 2019-07-15 8:01 UTC (permalink / raw) To: Ludovic BARRE Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre TORGUE, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc, linux-stm32 On Mon, 15 Jul 2019 at 09:39, Ludovic BARRE <ludovic.barre@st.com> wrote: > > Hi Ulf > > like scheduled, I send you a "gentleman ping" about this series. Thanks, I am just looking at it, again. Kind regards Uffe > > Regards, > Ludo > ________________________________________ > De : Ulf Hansson <ulf.hansson@linaro.org> > Envoyé : jeudi 20 juin 2019 15:50 > À : Ludovic BARRE > Cc : Rob Herring; Srinivas Kandagatla; Maxime Coquelin; Alexandre TORGUE; Linux ARM; Linux Kernel Mailing List; DTML; linux-mmc@vger.kernel.org; linux-stm32@st-md-mailman.stormreply.com > Objet : Re: [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant > > Hi Ludovic, > > On Thu, 13 Jun 2019 at 15:13, Ulf Hansson <ulf.hansson@linaro.org> wrote: > > > > On Thu, 13 Jun 2019 at 15:02, Ludovic BARRE <ludovic.barre@st.com> wrote: > > > > > > hi Ulf > > > > > > Just a "gentleman ping" about this series. > > > I know you are busy, it's just to be sure you do not forget me :-) > > > > Thanks! I started briefly to review, but got distracted again. I will > > come to it, but it just seems to take more time than it should, my > > apologies. > > Alright, so I planned to review this this week - but failed. I have > been overwhelmed with work lately (as usual when vacation is getting > closer). > > I need to gently request to come back to this as of week 28, when I > will give this the highest prio. Again apologize for the delays! > > Kind regards > Uffe > > > > > Br > > Uffe > > > > > > > > Regards > > > Ludo > > > > > > On 6/3/19 5:55 PM, Ludovic Barre wrote: > > > > From: Ludovic Barre <ludovic.barre@st.com> > > > > > > > > This patch series adds busy detect for stm32 sdmmc variant. > > > > Some adaptations are required: > > > > -Clear busy status bit if busy_detect_flag and busy_detect_mask are > > > > different. > > > > -Add hardware busy timeout with MMCIDATATIMER register. > > > > > > > > V3: > > > > -rebase on latest mmc next > > > > -replace re-read by status parameter. > > > > > > > > V2: > > > > -mmci_cmd_irq cleanup in separate patch. > > > > -simplify the busy_detect_flag exclude > > > > -replace sdmmc specific comment in > > > > "mmc: mmci: avoid fake busy polling in mmci_irq" > > > > to focus on common behavior > > > > > > > > Ludovic Barre (3): > > > > mmc: mmci: fix read status for busy detect > > > > mmc: mmci: add hardware busy timeout feature > > > > mmc: mmci: add busy detect for stm32 sdmmc variant > > > > > > > > drivers/mmc/host/mmci.c | 49 +++++++++++++++++++++++++++++++++++++++++-------- > > > > drivers/mmc/host/mmci.h | 3 +++ > > > > 2 files changed, 44 insertions(+), 8 deletions(-) > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-08-05 7:33 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-03 15:55 [PATCH V3 0/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 1/3] mmc: mmci: fix read status for busy detect Ludovic Barre 2019-07-15 16:31 ` Ulf Hansson 2019-07-26 9:41 ` Ludovic BARRE 2019-08-05 7:33 ` [Linux-stm32] " Ludovic BARRE 2019-06-03 15:55 ` [PATCH V3 2/3] mmc: mmci: add hardware busy timeout feature Ludovic Barre 2019-06-03 15:55 ` [PATCH V3 3/3] mmc: mmci: add busy detect for stm32 sdmmc variant Ludovic Barre 2019-06-13 13:02 ` [PATCH V3 0/3] " Ludovic BARRE 2019-06-13 13:13 ` Ulf Hansson 2019-06-20 13:50 ` Ulf Hansson 2019-07-15 7:39 ` Ludovic BARRE 2019-07-15 8:01 ` Ulf Hansson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).