* [PATCH V3 0/2] mmc: mmci: add stop command @ 2018-12-06 15:13 Ludovic Barre 2018-12-06 15:13 ` [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit Ludovic Barre 2018-12-06 15:13 ` [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm Ludovic Barre 0 siblings, 2 replies; 9+ messages in thread From: Ludovic Barre @ 2018-12-06 15:13 UTC (permalink / raw) To: Ulf Hansson, Rob Herring Cc: devicetree, Alexandre Torgue, linux-mmc, linux-kernel, srinivas.kandagatla, Ludovic Barre, Maxime Coquelin, linux-stm32, linux-arm-kernel From: Ludovic Barre <ludovic.barre@st.com> This patch series adds variant property to: -Set cmdstop bit on STOP_TRANSMISSION command. -On command or data error, send a stop command. to clear DPSM if it's yet activated. change v3: -Move the cmdstop bit in separate patch. -Use Ulf re-wording for patch 2/2. -Move specific part in a separate function. Ludovic Barre (2): mmc: mmci: add variant property to set command stop bit mmc: mmci: send stop command to clear the dpsm drivers/mmc/host/mmci.c | 43 +++++++++++++++++++++++++++++++++++++++++++ drivers/mmc/host/mmci.h | 4 ++++ 2 files changed, 47 insertions(+) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2018-12-06 15:13 [PATCH V3 0/2] mmc: mmci: add stop command Ludovic Barre @ 2018-12-06 15:13 ` Ludovic Barre 2018-12-11 9:47 ` Ulf Hansson 2018-12-06 15:13 ` [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm Ludovic Barre 1 sibling, 1 reply; 9+ messages in thread From: Ludovic Barre @ 2018-12-06 15:13 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> On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set cmdstop bit in command register. The CPSM ("Command Path State Machine") treats the command as a Stop Transmission command and signals abort to the DPSM ("Data Path State Machine"). Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- drivers/mmc/host/mmci.c | 6 ++++++ drivers/mmc/host/mmci.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index 13fa640..e352f5a 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -21,6 +21,7 @@ #include <linux/err.h> #include <linux/highmem.h> #include <linux/log2.h> +#include <linux/mmc/mmc.h> #include <linux/mmc/pm.h> #include <linux/mmc/host.h> #include <linux/mmc/card.h> @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, .cmdreg_srsp = MCI_CPSM_STM32_SRSP, + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, .datactrl_first = true, @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) mmci_reg_delay(host); } + if (host->variant->cmdreg_stop && + cmd->opcode == MMC_STOP_TRANSMISSION) + c |= host->variant->cmdreg_stop; + c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; if (cmd->flags & MMC_RSP_PRESENT) { if (cmd->flags & MMC_RSP_136) diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 550dd39..2422909 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -264,6 +264,7 @@ struct mmci_host; * @cmdreg_lrsp_crc: enable value for long response with crc * @cmdreg_srsp_crc: enable value for short response with crc * @cmdreg_srsp: enable value for short response without crc + * @cmdreg_stop: enable value for stop and abort transmission * @datalength_bits: number of bits in the MMCIDATALENGTH register * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY * is asserted (likewise for RX) @@ -316,6 +317,7 @@ struct variant_data { unsigned int cmdreg_lrsp_crc; unsigned int cmdreg_srsp_crc; unsigned int cmdreg_srsp; + unsigned int cmdreg_stop; unsigned int datalength_bits; unsigned int fifosize; unsigned int fifohalfsize; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2018-12-06 15:13 ` [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit Ludovic Barre @ 2018-12-11 9:47 ` Ulf Hansson 2018-12-11 9:53 ` Ludovic BARRE 0 siblings, 1 reply; 9+ messages in thread From: Ulf Hansson @ 2018-12-11 9:47 UTC (permalink / raw) To: 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 On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: > > From: Ludovic Barre <ludovic.barre@st.com> > > On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set > cmdstop bit in command register. The CPSM ("Command Path State Machine") > treats the command as a Stop Transmission command and signals > abort to the DPSM ("Data Path State Machine"). > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> Applied for next, thanks! Withholding patch2 for a while, as I need some more time to review it. Kind regards Uffe > --- > drivers/mmc/host/mmci.c | 6 ++++++ > drivers/mmc/host/mmci.h | 2 ++ > 2 files changed, 8 insertions(+) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index 13fa640..e352f5a 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -21,6 +21,7 @@ > #include <linux/err.h> > #include <linux/highmem.h> > #include <linux/log2.h> > +#include <linux/mmc/mmc.h> > #include <linux/mmc/pm.h> > #include <linux/mmc/host.h> > #include <linux/mmc/card.h> > @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { > .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, > .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, > .cmdreg_srsp = MCI_CPSM_STM32_SRSP, > + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, > .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, > .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, > .datactrl_first = true, > @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) > mmci_reg_delay(host); > } > > + if (host->variant->cmdreg_stop && > + cmd->opcode == MMC_STOP_TRANSMISSION) > + c |= host->variant->cmdreg_stop; > + > c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; > if (cmd->flags & MMC_RSP_PRESENT) { > if (cmd->flags & MMC_RSP_136) > diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h > index 550dd39..2422909 100644 > --- a/drivers/mmc/host/mmci.h > +++ b/drivers/mmc/host/mmci.h > @@ -264,6 +264,7 @@ struct mmci_host; > * @cmdreg_lrsp_crc: enable value for long response with crc > * @cmdreg_srsp_crc: enable value for short response with crc > * @cmdreg_srsp: enable value for short response without crc > + * @cmdreg_stop: enable value for stop and abort transmission > * @datalength_bits: number of bits in the MMCIDATALENGTH register > * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY > * is asserted (likewise for RX) > @@ -316,6 +317,7 @@ struct variant_data { > unsigned int cmdreg_lrsp_crc; > unsigned int cmdreg_srsp_crc; > unsigned int cmdreg_srsp; > + unsigned int cmdreg_stop; > unsigned int datalength_bits; > unsigned int fifosize; > unsigned int fifohalfsize; > -- > 2.7.4 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2018-12-11 9:47 ` Ulf Hansson @ 2018-12-11 9:53 ` Ludovic BARRE 2019-01-03 10:35 ` Ludovic BARRE 0 siblings, 1 reply; 9+ messages in thread From: Ludovic BARRE @ 2018-12-11 9:53 UTC (permalink / raw) To: Ulf Hansson Cc: DTML, Alexandre Torgue, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Rob Herring, Srinivas Kandagatla, Maxime Coquelin, linux-stm32, Linux ARM On 12/11/18 10:47 AM, Ulf Hansson wrote: > On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: >> >> From: Ludovic Barre <ludovic.barre@st.com> >> >> On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set >> cmdstop bit in command register. The CPSM ("Command Path State Machine") >> treats the command as a Stop Transmission command and signals >> abort to the DPSM ("Data Path State Machine"). >> >> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > > Applied for next, thanks! thanks > > Withholding patch2 for a while, as I need some more time to review it. No problem, Regards Ludo > > Kind regards > Uffe > >> --- >> drivers/mmc/host/mmci.c | 6 ++++++ >> drivers/mmc/host/mmci.h | 2 ++ >> 2 files changed, 8 insertions(+) >> >> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >> index 13fa640..e352f5a 100644 >> --- a/drivers/mmc/host/mmci.c >> +++ b/drivers/mmc/host/mmci.c >> @@ -21,6 +21,7 @@ >> #include <linux/err.h> >> #include <linux/highmem.h> >> #include <linux/log2.h> >> +#include <linux/mmc/mmc.h> >> #include <linux/mmc/pm.h> >> #include <linux/mmc/host.h> >> #include <linux/mmc/card.h> >> @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { >> .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, >> .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, >> .cmdreg_srsp = MCI_CPSM_STM32_SRSP, >> + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, >> .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, >> .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, >> .datactrl_first = true, >> @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c) >> mmci_reg_delay(host); >> } >> >> + if (host->variant->cmdreg_stop && >> + cmd->opcode == MMC_STOP_TRANSMISSION) >> + c |= host->variant->cmdreg_stop; >> + >> c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; >> if (cmd->flags & MMC_RSP_PRESENT) { >> if (cmd->flags & MMC_RSP_136) >> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h >> index 550dd39..2422909 100644 >> --- a/drivers/mmc/host/mmci.h >> +++ b/drivers/mmc/host/mmci.h >> @@ -264,6 +264,7 @@ struct mmci_host; >> * @cmdreg_lrsp_crc: enable value for long response with crc >> * @cmdreg_srsp_crc: enable value for short response with crc >> * @cmdreg_srsp: enable value for short response without crc >> + * @cmdreg_stop: enable value for stop and abort transmission >> * @datalength_bits: number of bits in the MMCIDATALENGTH register >> * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY >> * is asserted (likewise for RX) >> @@ -316,6 +317,7 @@ struct variant_data { >> unsigned int cmdreg_lrsp_crc; >> unsigned int cmdreg_srsp_crc; >> unsigned int cmdreg_srsp; >> + unsigned int cmdreg_stop; >> unsigned int datalength_bits; >> unsigned int fifosize; >> unsigned int fifohalfsize; >> -- >> 2.7.4 >> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2018-12-11 9:53 ` Ludovic BARRE @ 2019-01-03 10:35 ` Ludovic BARRE 2019-01-24 15:03 ` [Linux-stm32] " Ludovic BARRE 0 siblings, 1 reply; 9+ messages in thread From: Ludovic BARRE @ 2019-01-03 10:35 UTC (permalink / raw) To: Ulf Hansson Cc: Rob Herring, Srinivas Kandagatla, Maxime Coquelin, Alexandre Torgue, Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc@vger.kernel.org, linux-stm32 hi Ulf happy new years. Just a gentleman ping about patch2 of this series "mmc: mmci: send stop command to clear the dpsm." Regards Ludo On 12/11/18 10:53 AM, Ludovic BARRE wrote: > > > On 12/11/18 10:47 AM, Ulf Hansson wrote: >> On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: >>> >>> From: Ludovic Barre <ludovic.barre@st.com> >>> >>> On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set >>> cmdstop bit in command register. The CPSM ("Command Path State Machine") >>> treats the command as a Stop Transmission command and signals >>> abort to the DPSM ("Data Path State Machine"). >>> >>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >> >> Applied for next, thanks! > > thanks > >> >> Withholding patch2 for a while, as I need some more time to review it. > > No problem, > > Regards > Ludo > >> >> Kind regards >> Uffe >> >>> --- >>> drivers/mmc/host/mmci.c | 6 ++++++ >>> drivers/mmc/host/mmci.h | 2 ++ >>> 2 files changed, 8 insertions(+) >>> >>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >>> index 13fa640..e352f5a 100644 >>> --- a/drivers/mmc/host/mmci.c >>> +++ b/drivers/mmc/host/mmci.c >>> @@ -21,6 +21,7 @@ >>> #include <linux/err.h> >>> #include <linux/highmem.h> >>> #include <linux/log2.h> >>> +#include <linux/mmc/mmc.h> >>> #include <linux/mmc/pm.h> >>> #include <linux/mmc/host.h> >>> #include <linux/mmc/card.h> >>> @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { >>> .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, >>> .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, >>> .cmdreg_srsp = MCI_CPSM_STM32_SRSP, >>> + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, >>> .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, >>> .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, >>> .datactrl_first = true, >>> @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, >>> struct mmc_command *cmd, u32 c) >>> mmci_reg_delay(host); >>> } >>> >>> + if (host->variant->cmdreg_stop && >>> + cmd->opcode == MMC_STOP_TRANSMISSION) >>> + c |= host->variant->cmdreg_stop; >>> + >>> c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; >>> if (cmd->flags & MMC_RSP_PRESENT) { >>> if (cmd->flags & MMC_RSP_136) >>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h >>> index 550dd39..2422909 100644 >>> --- a/drivers/mmc/host/mmci.h >>> +++ b/drivers/mmc/host/mmci.h >>> @@ -264,6 +264,7 @@ struct mmci_host; >>> * @cmdreg_lrsp_crc: enable value for long response with crc >>> * @cmdreg_srsp_crc: enable value for short response with crc >>> * @cmdreg_srsp: enable value for short response without crc >>> + * @cmdreg_stop: enable value for stop and abort transmission >>> * @datalength_bits: number of bits in the MMCIDATALENGTH register >>> * @fifosize: number of bytes that can be written when >>> MMCI_TXFIFOEMPTY >>> * is asserted (likewise for RX) >>> @@ -316,6 +317,7 @@ struct variant_data { >>> unsigned int cmdreg_lrsp_crc; >>> unsigned int cmdreg_srsp_crc; >>> unsigned int cmdreg_srsp; >>> + unsigned int cmdreg_stop; >>> unsigned int datalength_bits; >>> unsigned int fifosize; >>> unsigned int fifohalfsize; >>> -- >>> 2.7.4 >>> ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Linux-stm32] [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2019-01-03 10:35 ` Ludovic BARRE @ 2019-01-24 15:03 ` Ludovic BARRE 2019-01-24 15:12 ` Ulf Hansson 0 siblings, 1 reply; 9+ messages in thread From: Ludovic BARRE @ 2019-01-24 15:03 UTC (permalink / raw) To: Ulf Hansson Cc: DTML, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Rob Herring, Srinivas Kandagatla, Maxime Coquelin, linux-stm32, Linux ARM hi Ulf I don't think you've seen my previous mail :-( what is your feeling about "mmc: mmci: send stop command to clear the dpsm" Regards Ludo On 1/3/19 11:35 AM, Ludovic BARRE wrote: > hi Ulf > > happy new years. > > Just a gentleman ping about patch2 of this series > "mmc: mmci: send stop command to clear the dpsm." > > Regards > Ludo > > On 12/11/18 10:53 AM, Ludovic BARRE wrote: >> >> >> On 12/11/18 10:47 AM, Ulf Hansson wrote: >>> On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: >>>> >>>> From: Ludovic Barre <ludovic.barre@st.com> >>>> >>>> On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set >>>> cmdstop bit in command register. The CPSM ("Command Path State >>>> Machine") >>>> treats the command as a Stop Transmission command and signals >>>> abort to the DPSM ("Data Path State Machine"). >>>> >>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> >>> >>> Applied for next, thanks! >> >> thanks >> >>> >>> Withholding patch2 for a while, as I need some more time to review it. >> >> No problem, >> >> Regards >> Ludo >> >>> >>> Kind regards >>> Uffe >>> >>>> --- >>>> drivers/mmc/host/mmci.c | 6 ++++++ >>>> drivers/mmc/host/mmci.h | 2 ++ >>>> 2 files changed, 8 insertions(+) >>>> >>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c >>>> index 13fa640..e352f5a 100644 >>>> --- a/drivers/mmc/host/mmci.c >>>> +++ b/drivers/mmc/host/mmci.c >>>> @@ -21,6 +21,7 @@ >>>> #include <linux/err.h> >>>> #include <linux/highmem.h> >>>> #include <linux/log2.h> >>>> +#include <linux/mmc/mmc.h> >>>> #include <linux/mmc/pm.h> >>>> #include <linux/mmc/host.h> >>>> #include <linux/mmc/card.h> >>>> @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { >>>> .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, >>>> .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, >>>> .cmdreg_srsp = MCI_CPSM_STM32_SRSP, >>>> + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, >>>> .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, >>>> .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, >>>> .datactrl_first = true, >>>> @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, >>>> struct mmc_command *cmd, u32 c) >>>> mmci_reg_delay(host); >>>> } >>>> >>>> + if (host->variant->cmdreg_stop && >>>> + cmd->opcode == MMC_STOP_TRANSMISSION) >>>> + c |= host->variant->cmdreg_stop; >>>> + >>>> c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; >>>> if (cmd->flags & MMC_RSP_PRESENT) { >>>> if (cmd->flags & MMC_RSP_136) >>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h >>>> index 550dd39..2422909 100644 >>>> --- a/drivers/mmc/host/mmci.h >>>> +++ b/drivers/mmc/host/mmci.h >>>> @@ -264,6 +264,7 @@ struct mmci_host; >>>> * @cmdreg_lrsp_crc: enable value for long response with crc >>>> * @cmdreg_srsp_crc: enable value for short response with crc >>>> * @cmdreg_srsp: enable value for short response without crc >>>> + * @cmdreg_stop: enable value for stop and abort transmission >>>> * @datalength_bits: number of bits in the MMCIDATALENGTH register >>>> * @fifosize: number of bytes that can be written when >>>> MMCI_TXFIFOEMPTY >>>> * is asserted (likewise for RX) >>>> @@ -316,6 +317,7 @@ struct variant_data { >>>> unsigned int cmdreg_lrsp_crc; >>>> unsigned int cmdreg_srsp_crc; >>>> unsigned int cmdreg_srsp; >>>> + unsigned int cmdreg_stop; >>>> unsigned int datalength_bits; >>>> unsigned int fifosize; >>>> unsigned int fifohalfsize; >>>> -- >>>> 2.7.4 >>>> > _______________________________________________ > 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] 9+ messages in thread
* Re: [Linux-stm32] [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit 2019-01-24 15:03 ` [Linux-stm32] " Ludovic BARRE @ 2019-01-24 15:12 ` Ulf Hansson 0 siblings, 0 replies; 9+ messages in thread From: Ulf Hansson @ 2019-01-24 15:12 UTC (permalink / raw) To: Ludovic BARRE Cc: DTML, linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Rob Herring, Srinivas Kandagatla, Maxime Coquelin, linux-stm32, Linux ARM On Thu, 24 Jan 2019 at 16:03, Ludovic BARRE <ludovic.barre@st.com> wrote: > > hi Ulf > > I don't think you've seen my previous mail :-( > what is your feeling about "mmc: mmci: send stop command to clear the dpsm" Apologize for the delay. I wanted to check this in detail so I applied your patch locally and started to play/test it. However, a couple of other regressions was reported for v5.0 rcs, so I got sidetracked. Back on track by now, so I will have look asap. Thanks for pinging me! Kind regards Uffe > > Regards > Ludo > > On 1/3/19 11:35 AM, Ludovic BARRE wrote: > > hi Ulf > > > > happy new years. > > > > Just a gentleman ping about patch2 of this series > > "mmc: mmci: send stop command to clear the dpsm." > > > > Regards > > Ludo > > > > On 12/11/18 10:53 AM, Ludovic BARRE wrote: > >> > >> > >> On 12/11/18 10:47 AM, Ulf Hansson wrote: > >>> On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: > >>>> > >>>> From: Ludovic Barre <ludovic.barre@st.com> > >>>> > >>>> On cmd12 (STOP_TRANSMISSION), STM32 sdmmc variant needs to set > >>>> cmdstop bit in command register. The CPSM ("Command Path State > >>>> Machine") > >>>> treats the command as a Stop Transmission command and signals > >>>> abort to the DPSM ("Data Path State Machine"). > >>>> > >>>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > >>> > >>> Applied for next, thanks! > >> > >> thanks > >> > >>> > >>> Withholding patch2 for a while, as I need some more time to review it. > >> > >> No problem, > >> > >> Regards > >> Ludo > >> > >>> > >>> Kind regards > >>> Uffe > >>> > >>>> --- > >>>> drivers/mmc/host/mmci.c | 6 ++++++ > >>>> drivers/mmc/host/mmci.h | 2 ++ > >>>> 2 files changed, 8 insertions(+) > >>>> > >>>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > >>>> index 13fa640..e352f5a 100644 > >>>> --- a/drivers/mmc/host/mmci.c > >>>> +++ b/drivers/mmc/host/mmci.c > >>>> @@ -21,6 +21,7 @@ > >>>> #include <linux/err.h> > >>>> #include <linux/highmem.h> > >>>> #include <linux/log2.h> > >>>> +#include <linux/mmc/mmc.h> > >>>> #include <linux/mmc/pm.h> > >>>> #include <linux/mmc/host.h> > >>>> #include <linux/mmc/card.h> > >>>> @@ -274,6 +275,7 @@ static struct variant_data variant_stm32_sdmmc = { > >>>> .cmdreg_lrsp_crc = MCI_CPSM_STM32_LRSP_CRC, > >>>> .cmdreg_srsp_crc = MCI_CPSM_STM32_SRSP_CRC, > >>>> .cmdreg_srsp = MCI_CPSM_STM32_SRSP, > >>>> + .cmdreg_stop = MCI_CPSM_STM32_CMDSTOP, > >>>> .data_cmd_enable = MCI_CPSM_STM32_CMDTRANS, > >>>> .irq_pio_mask = MCI_IRQ_PIO_STM32_MASK, > >>>> .datactrl_first = true, > >>>> @@ -1100,6 +1102,10 @@ mmci_start_command(struct mmci_host *host, > >>>> struct mmc_command *cmd, u32 c) > >>>> mmci_reg_delay(host); > >>>> } > >>>> > >>>> + if (host->variant->cmdreg_stop && > >>>> + cmd->opcode == MMC_STOP_TRANSMISSION) > >>>> + c |= host->variant->cmdreg_stop; > >>>> + > >>>> c |= cmd->opcode | host->variant->cmdreg_cpsm_enable; > >>>> if (cmd->flags & MMC_RSP_PRESENT) { > >>>> if (cmd->flags & MMC_RSP_136) > >>>> diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h > >>>> index 550dd39..2422909 100644 > >>>> --- a/drivers/mmc/host/mmci.h > >>>> +++ b/drivers/mmc/host/mmci.h > >>>> @@ -264,6 +264,7 @@ struct mmci_host; > >>>> * @cmdreg_lrsp_crc: enable value for long response with crc > >>>> * @cmdreg_srsp_crc: enable value for short response with crc > >>>> * @cmdreg_srsp: enable value for short response without crc > >>>> + * @cmdreg_stop: enable value for stop and abort transmission > >>>> * @datalength_bits: number of bits in the MMCIDATALENGTH register > >>>> * @fifosize: number of bytes that can be written when > >>>> MMCI_TXFIFOEMPTY > >>>> * is asserted (likewise for RX) > >>>> @@ -316,6 +317,7 @@ struct variant_data { > >>>> unsigned int cmdreg_lrsp_crc; > >>>> unsigned int cmdreg_srsp_crc; > >>>> unsigned int cmdreg_srsp; > >>>> + unsigned int cmdreg_stop; > >>>> unsigned int datalength_bits; > >>>> unsigned int fifosize; > >>>> unsigned int fifohalfsize; > >>>> -- > >>>> 2.7.4 > >>>> > > _______________________________________________ > > 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] 9+ messages in thread
* [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm 2018-12-06 15:13 [PATCH V3 0/2] mmc: mmci: add stop command Ludovic Barre 2018-12-06 15:13 ` [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit Ludovic Barre @ 2018-12-06 15:13 ` Ludovic Barre 2019-01-29 14:31 ` Ulf Hansson 1 sibling, 1 reply; 9+ messages in thread From: Ludovic Barre @ 2018-12-06 15:13 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> The current approach with sending a CMD12 (STOP_TRANSMISSION) to complete a data transfer request, either because of using the open ended transmission type or because of receiving an error during a data transfer, isn't sufficient for the STM32 sdmmc variant. More precisely, for STM32 sdmmc the DPSM ("Data Path State Machine") needs to be cleared by sending a CMD12, also for the so called ADTC commands. For this reason, let the driver send a CMD12 to complete ADTC commands, in case it's set (depend of cmdreg_stop variant property). Signed-off-by: Ludovic Barre <ludovic.barre@st.com> --- drivers/mmc/host/mmci.c | 37 +++++++++++++++++++++++++++++++++++++ drivers/mmc/host/mmci.h | 2 ++ 2 files changed, 39 insertions(+) diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c index e352f5a..4e5643d 100644 --- a/drivers/mmc/host/mmci.c +++ b/drivers/mmc/host/mmci.c @@ -58,6 +58,8 @@ void sdmmc_variant_init(struct mmci_host *host); #else static inline void sdmmc_variant_init(struct mmci_host *host) {} #endif +static void +mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c); static unsigned int fmax = 515633; @@ -572,9 +574,37 @@ void mmci_dma_error(struct mmci_host *host) host->ops->dma_error(host); } +static int mmci_stop_command(struct mmci_host *host, struct mmc_request *mrq) +{ + u32 dpsm; + + /* + * If an error happens on command or data transmission + * the DPSM stay enabled. The CPSM required a stop command + * to reinitialize the DPSM. + */ + dpsm = readl_relaxed(host->base + MMCISTATUS); + dpsm &= MCI_STM32_DPSMACTIVE; + + if (dpsm && ((mrq->cmd && mrq->cmd->error) || + (mrq->data && mrq->data->error))) { + mmci_start_command(host, &host->stop_abort, 0); + return -EBUSY; + } + + return 0; +} + static void mmci_request_end(struct mmci_host *host, struct mmc_request *mrq) { + /* + * For variant with cmdstop bit, a stop command could be needed + * to finish the request. + */ + if (host->variant->cmdreg_stop && mmci_stop_command(host, mrq)) + return; + writel(0, host->base + MMCICOMMAND); BUG_ON(host->data); @@ -1956,6 +1986,13 @@ static int mmci_probe(struct amba_device *dev, mmc->max_busy_timeout = 0; } + /* prepare the stop command, used to abort and reinitialized the DPSM */ + if (variant->cmdreg_stop) { + host->stop_abort.opcode = MMC_STOP_TRANSMISSION; + host->stop_abort.arg = 0; + host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC; + } + mmc->ops = &mmci_ops; /* We support these PM capabilities. */ diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h index 2422909..35372cd 100644 --- a/drivers/mmc/host/mmci.h +++ b/drivers/mmc/host/mmci.h @@ -161,6 +161,7 @@ #define MCI_ST_CEATAEND (1 << 23) #define MCI_ST_CARDBUSY (1 << 24) /* Extended status bits for the STM32 variants */ +#define MCI_STM32_DPSMACTIVE BIT(12) #define MCI_STM32_BUSYD0 BIT(20) #define MMCICLEAR 0x038 @@ -377,6 +378,7 @@ struct mmci_host { void __iomem *base; struct mmc_request *mrq; struct mmc_command *cmd; + struct mmc_command stop_abort; struct mmc_data *data; struct mmc_host *mmc; struct clk *clk; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm 2018-12-06 15:13 ` [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm Ludovic Barre @ 2019-01-29 14:31 ` Ulf Hansson 0 siblings, 0 replies; 9+ messages in thread From: Ulf Hansson @ 2019-01-29 14: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@vger.kernel.org, linux-stm32 On Thu, 6 Dec 2018 at 16:13, Ludovic Barre <ludovic.Barre@st.com> wrote: > > From: Ludovic Barre <ludovic.barre@st.com> > > The current approach with sending a CMD12 (STOP_TRANSMISSION) to > complete a data transfer request, either because of using the open > ended transmission type or because of receiving an error during a data > transfer, isn't sufficient for the STM32 sdmmc variant. > > More precisely, for STM32 sdmmc the DPSM ("Data Path State Machine") > needs to be cleared by sending a CMD12, also for the so called ADTC > commands. For this reason, let the driver send a CMD12 to complete > ADTC commands, in case it's set (depend of cmdreg_stop variant property). > > Signed-off-by: Ludovic Barre <ludovic.barre@st.com> > --- > drivers/mmc/host/mmci.c | 37 +++++++++++++++++++++++++++++++++++++ > drivers/mmc/host/mmci.h | 2 ++ > 2 files changed, 39 insertions(+) > > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c > index e352f5a..4e5643d 100644 > --- a/drivers/mmc/host/mmci.c > +++ b/drivers/mmc/host/mmci.c > @@ -58,6 +58,8 @@ void sdmmc_variant_init(struct mmci_host *host); > #else > static inline void sdmmc_variant_init(struct mmci_host *host) {} > #endif > +static void > +mmci_start_command(struct mmci_host *host, struct mmc_command *cmd, u32 c); > > static unsigned int fmax = 515633; > > @@ -572,9 +574,37 @@ void mmci_dma_error(struct mmci_host *host) > host->ops->dma_error(host); > } > > +static int mmci_stop_command(struct mmci_host *host, struct mmc_request *mrq) > +{ > + u32 dpsm; > + > + /* > + * If an error happens on command or data transmission > + * the DPSM stay enabled. The CPSM required a stop command > + * to reinitialize the DPSM. > + */ > + dpsm = readl_relaxed(host->base + MMCISTATUS); > + dpsm &= MCI_STM32_DPSMACTIVE; > + > + if (dpsm && ((mrq->cmd && mrq->cmd->error) || > + (mrq->data && mrq->data->error))) { > + mmci_start_command(host, &host->stop_abort, 0); > + return -EBUSY; > + } Unless I have got it wrong, I think there are several problems with the above code and how you call it. 1) We may be reading the MMCISTATUS register when we don't need to (because there are no errors). 2) Nothing prevents keep sending the CMD12 over and over again, for the same request. This could happen, as long as the MCI_STM32_DPSMACTIVE remains set. I guess it simply shouldn't happen, but I rather prevent this being possible altogether, as to avoid a potential hang. It's better to propagate errors. 3) There is a scenario when the DPSM has been enabled, while we fail with the "sbc" command, this isn't covered, but I think should, right? 4) host->stop_abort.error needs to be reset to zero, in-between sending the internal CMD12 command. Otherwise, we may end up re-using an error code from a failed CMD12 command, over and over again. > + > + return 0; > +} > + > static void > mmci_request_end(struct mmci_host *host, struct mmc_request *mrq) > { > + /* > + * For variant with cmdstop bit, a stop command could be needed > + * to finish the request. > + */ > + if (host->variant->cmdreg_stop && mmci_stop_command(host, mrq)) > + return; > + > writel(0, host->base + MMCICOMMAND); > > BUG_ON(host->data); > @@ -1956,6 +1986,13 @@ static int mmci_probe(struct amba_device *dev, > mmc->max_busy_timeout = 0; > } > > + /* prepare the stop command, used to abort and reinitialized the DPSM */ > + if (variant->cmdreg_stop) { > + host->stop_abort.opcode = MMC_STOP_TRANSMISSION; > + host->stop_abort.arg = 0; > + host->stop_abort.flags = MMC_RSP_R1B | MMC_CMD_AC; > + } > + > mmc->ops = &mmci_ops; > > /* We support these PM capabilities. */ > diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h > index 2422909..35372cd 100644 > --- a/drivers/mmc/host/mmci.h > +++ b/drivers/mmc/host/mmci.h > @@ -161,6 +161,7 @@ > #define MCI_ST_CEATAEND (1 << 23) > #define MCI_ST_CARDBUSY (1 << 24) > /* Extended status bits for the STM32 variants */ > +#define MCI_STM32_DPSMACTIVE BIT(12) > #define MCI_STM32_BUSYD0 BIT(20) > > #define MMCICLEAR 0x038 > @@ -377,6 +378,7 @@ struct mmci_host { > void __iomem *base; > struct mmc_request *mrq; > struct mmc_command *cmd; > + struct mmc_command stop_abort; > struct mmc_data *data; > struct mmc_host *mmc; > struct clk *clk; > -- > 2.7.4 > To fix the issues I pointed out above, I decided to try out something myself. So, I will post a patch in a few minutes, can you please give it try at your end? Kind regards Uffe ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-01-29 14:31 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-06 15:13 [PATCH V3 0/2] mmc: mmci: add stop command Ludovic Barre 2018-12-06 15:13 ` [PATCH V3 1/2] mmc: mmci: add variant property to set command stop bit Ludovic Barre 2018-12-11 9:47 ` Ulf Hansson 2018-12-11 9:53 ` Ludovic BARRE 2019-01-03 10:35 ` Ludovic BARRE 2019-01-24 15:03 ` [Linux-stm32] " Ludovic BARRE 2019-01-24 15:12 ` Ulf Hansson 2018-12-06 15:13 ` [PATCH V3 2/2] mmc: mmci: send stop command to clear the dpsm Ludovic Barre 2019-01-29 14:31 ` 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).