* [PATCH V2 0/2] mmc: mmci_sdmmc: fixes and improvements
@ 2020-03-25 14:34 Ludovic Barre
2020-03-25 14:34 ` [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag Ludovic Barre
2020-03-25 14:34 ` [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values Ludovic Barre
0 siblings, 2 replies; 6+ messages in thread
From: Ludovic Barre @ 2020-03-25 14:34 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
This patch series fixes some problems:
-Clear busyd0end irq flag when a R1B busy is completed.
-Initialize pwr|clk|datactrl_reg with their hardware values at probe.
Ludovic Barre (2):
mmc: mmci_sdmmc: fix clear busyd0end irq flag
mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values
drivers/mmc/host/mmci.c | 4 ++++
drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++--
2 files changed, 6 insertions(+), 2 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag
2020-03-25 14:34 [PATCH V2 0/2] mmc: mmci_sdmmc: fixes and improvements Ludovic Barre
@ 2020-03-25 14:34 ` Ludovic Barre
2020-03-26 14:28 ` Ulf Hansson
2020-03-25 14:34 ` [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values Ludovic Barre
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Barre @ 2020-03-25 14:34 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
The busyd0 line transition can be very fast. The busy request
may be completed by busy_d0end without wait the busy_d0 steps.
The busyd0end irq flag must be cleared even if no busy_status.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index f76e82f8f12f..d33e62bd6153 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -358,11 +358,11 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
if (host->busy_status) {
writel_relaxed(mask & ~host->variant->busy_detect_mask,
base + MMCIMASK0);
- writel_relaxed(host->variant->busy_detect_mask,
- base + MMCICLEAR);
host->busy_status = 0;
}
+ writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR);
+
return true;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values
2020-03-25 14:34 [PATCH V2 0/2] mmc: mmci_sdmmc: fixes and improvements Ludovic Barre
2020-03-25 14:34 ` [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag Ludovic Barre
@ 2020-03-25 14:34 ` Ludovic Barre
2020-03-26 14:27 ` Ulf Hansson
1 sibling, 1 reply; 6+ messages in thread
From: Ludovic Barre @ 2020-03-25 14:34 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
In mmci_write_pwr|clk|datactrlreg functions, if the desired value
is equal to corresponding variable (pwr_reg|clk_reg|datactrl_reg),
the value is not written in the register.
At probe pwr|clk|datactrl_reg of mmci_host structure are initialized
to 0 (kzalloc of mmc_alloc_host). But they does not necessarily reflect
hardware value of these registers, if they are used while boot level.
This is problematic, if we want to write 0 in these registers.
This patch initializes pwr|clk|datactrl_reg variables with their
hardware values while probing.
Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
---
drivers/mmc/host/mmci.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 647567def612..f378ae18d5dc 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -2085,6 +2085,10 @@ static int mmci_probe(struct amba_device *dev,
else if (plat->ocr_mask)
dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
+ host->pwr_reg = readl_relaxed(host->base + MMCIPOWER);
+ host->clk_reg = readl_relaxed(host->base + MMCICLOCK);
+ host->datactrl_reg = readl_relaxed(host->base + MMCIDATACTRL);
+
/* We support these capabilities. */
mmc->caps |= MMC_CAP_CMD23;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values
2020-03-25 14:34 ` [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values Ludovic Barre
@ 2020-03-26 14:27 ` Ulf Hansson
2020-04-01 14:56 ` Ludovic BARRE
0 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2020-03-26 14:27 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Srini Kandagatla, Maxime Coquelin, Alexandre Torgue,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc,
linux-stm32
On Wed, 25 Mar 2020 at 15:34, Ludovic Barre <ludovic.barre@st.com> wrote:
>
> In mmci_write_pwr|clk|datactrlreg functions, if the desired value
> is equal to corresponding variable (pwr_reg|clk_reg|datactrl_reg),
> the value is not written in the register.
>
> At probe pwr|clk|datactrl_reg of mmci_host structure are initialized
> to 0 (kzalloc of mmc_alloc_host). But they does not necessarily reflect
> hardware value of these registers, if they are used while boot level.
> This is problematic, if we want to write 0 in these registers.
It could be, but I don't see that we ever needs to do that - at least
not before we have written some other non-zero values to them (through
the helper functions).
>
> This patch initializes pwr|clk|datactrl_reg variables with their
> hardware values while probing.
Hmm, so in principle this change seems quite okay, but I am hesitant
to pick it up - unless it really addresses a problem that you have
observed.
The reason is, this change may lead to avoiding to re-write the
register with the same value it got during boot - and who knows what
is best here.
Kind regards
Uffe
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
> ---
> drivers/mmc/host/mmci.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 647567def612..f378ae18d5dc 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -2085,6 +2085,10 @@ static int mmci_probe(struct amba_device *dev,
> else if (plat->ocr_mask)
> dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
>
> + host->pwr_reg = readl_relaxed(host->base + MMCIPOWER);
> + host->clk_reg = readl_relaxed(host->base + MMCICLOCK);
> + host->datactrl_reg = readl_relaxed(host->base + MMCIDATACTRL);
> +
> /* We support these capabilities. */
> mmc->caps |= MMC_CAP_CMD23;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag
2020-03-25 14:34 ` [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag Ludovic Barre
@ 2020-03-26 14:28 ` Ulf Hansson
0 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2020-03-26 14:28 UTC (permalink / raw)
To: Ludovic Barre
Cc: Rob Herring, Srini Kandagatla, Maxime Coquelin, Alexandre Torgue,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc,
linux-stm32
On Wed, 25 Mar 2020 at 15:34, Ludovic Barre <ludovic.barre@st.com> wrote:
>
> The busyd0 line transition can be very fast. The busy request
> may be completed by busy_d0end without wait the busy_d0 steps.
> The busyd0end irq flag must be cleared even if no busy_status.
>
> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
Applied for next, by adding a fixes+stable tag, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
> index f76e82f8f12f..d33e62bd6153 100644
> --- a/drivers/mmc/host/mmci_stm32_sdmmc.c
> +++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
> @@ -358,11 +358,11 @@ static bool sdmmc_busy_complete(struct mmci_host *host, u32 status, u32 err_msk)
> if (host->busy_status) {
> writel_relaxed(mask & ~host->variant->busy_detect_mask,
> base + MMCIMASK0);
> - writel_relaxed(host->variant->busy_detect_mask,
> - base + MMCICLEAR);
> host->busy_status = 0;
> }
>
> + writel_relaxed(host->variant->busy_detect_mask, base + MMCICLEAR);
> +
> return true;
> }
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values
2020-03-26 14:27 ` Ulf Hansson
@ 2020-04-01 14:56 ` Ludovic BARRE
0 siblings, 0 replies; 6+ messages in thread
From: Ludovic BARRE @ 2020-04-01 14:56 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Srini Kandagatla, Maxime Coquelin, Alexandre Torgue,
Linux ARM, Linux Kernel Mailing List, DTML, linux-mmc,
linux-stm32
hi Ulf
Le 3/26/20 à 3:27 PM, Ulf Hansson a écrit :
> On Wed, 25 Mar 2020 at 15:34, Ludovic Barre <ludovic.barre@st.com> wrote:
>>
>> In mmci_write_pwr|clk|datactrlreg functions, if the desired value
>> is equal to corresponding variable (pwr_reg|clk_reg|datactrl_reg),
>> the value is not written in the register.
>>
>> At probe pwr|clk|datactrl_reg of mmci_host structure are initialized
>> to 0 (kzalloc of mmc_alloc_host). But they does not necessarily reflect
>> hardware value of these registers, if they are used while boot level.
>> This is problematic, if we want to write 0 in these registers.
>
> It could be, but I don't see that we ever needs to do that - at least
> not before we have written some other non-zero values to them (through
> the helper functions).
>
The sdmmc variant is slightly different on pwr_ctrl field
of MMCIPOWER register.
In classic mmci we have 3 or 2 values:
MMCI_PWR_OFF(0x0), MMCI_PWR_UP(0x2)optional, MMCI_PWR_ON(0x3)
-When you switch the external power supply off, the software set
power-off (0x0).
-When you switch the external power supply on, the software first enters
the power-up(0x2) phase, and waits until the supply output is stable
before moving to the power-on (0x3)phase.
On sdmmc we have 3 values:
MMCI_PWR_OFF(0x0), MCI_STM32_PWR_CYC(0x2), MMCI_PWR_ON(0x3)
-When you switch the external power supply off, the software must
MCI_STM32_PWR_CYC(0x2) => This will make that the SDMMC_D[7:0],
SDMMC_CMD and SDMMC_CK are driven low, to prevent the card from being
supplied through the signal lines.
-When you switch the external power supply on, when supply output is
stable the MMCI_PWR_OFF(0x0) state can be apply (minimum 1ms) => The
SDMMC_D[7:0], SDMMC_CMD and SDMMC_CK are set to drive “1”. After
MMCI_PWR_ON(0x3) stat could be set.
In fact the last value of power off sequence is different between
classic and sdmmc:
The classic mmci finish the power off sequence by 0x0, But the sdmmc
finish by 0x2, and we must write 0x0 in power on sequence before set
MMCI_PWR_ON.
The 0x0 value is not written due to kzalloc value of pwr_reg
(which not reflect hardware value of pwr register).
>>
>> This patch initializes pwr|clk|datactrl_reg variables with their
>> hardware values while probing.
>
> Hmm, so in principle this change seems quite okay, but I am hesitant
> to pick it up - unless it really addresses a problem that you have
> observed.
>
> The reason is, this change may lead to avoiding to re-write the
> register with the same value it got during boot - and who knows what
> is best here.
I understand your hesitation.
If you prefer, I can move the pwr_reg initialisation
in sdmmc_variant_init ?
Regards
Ludo
>
> Kind regards
> Uffe
>
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre@st.com>
>> ---
>> drivers/mmc/host/mmci.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
>> index 647567def612..f378ae18d5dc 100644
>> --- a/drivers/mmc/host/mmci.c
>> +++ b/drivers/mmc/host/mmci.c
>> @@ -2085,6 +2085,10 @@ static int mmci_probe(struct amba_device *dev,
>> else if (plat->ocr_mask)
>> dev_warn(mmc_dev(mmc), "Platform OCR mask is ignored\n");
>>
>> + host->pwr_reg = readl_relaxed(host->base + MMCIPOWER);
>> + host->clk_reg = readl_relaxed(host->base + MMCICLOCK);
>> + host->datactrl_reg = readl_relaxed(host->base + MMCIDATACTRL);
>> +
>> /* We support these capabilities. */
>> mmc->caps |= MMC_CAP_CMD23;
>>
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-01 14:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-25 14:34 [PATCH V2 0/2] mmc: mmci_sdmmc: fixes and improvements Ludovic Barre
2020-03-25 14:34 ` [PATCH V2 1/2] mmc: mmci_sdmmc: fix clear busyd0end irq flag Ludovic Barre
2020-03-26 14:28 ` Ulf Hansson
2020-03-25 14:34 ` [PATCH V2 2/2] mmc: mmci: initialize pwr|clk|datactrl_reg with their hardware values Ludovic Barre
2020-03-26 14:27 ` Ulf Hansson
2020-04-01 14:56 ` Ludovic BARRE
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).