STM32MP25 is a new SoC from STMicroelectronics. The machine was
pushed by Alexandre [1] in his tree.
On this new SoC, the SDMMC peripheral, using PL18x/MMCI driver
has been updated to v3.
The driver has been updated to manage this new version, and the new
features it supports:
* FIFO size increased from 64B to 1kB
* IDMA size alignment/mask updated
* New block gap hardware flow control
* Delay block updated and dependent on SoC
This series was pushed on top of next branch in Ulf's mmc tree, as it
requires feedback clock update patch [2].
[1] https://lore.kernel.org/lkml/[email protected]/T/
[2] https://lore.kernel.org/r/[email protected]
Yann Gautier (6):
dt-bindings: mmc: mmci: Add st,stm32mp25-sdmmc2 compatible
mmc: mmci: add stm32_idmabsize_align parameter
mmc: mmci: Add support for sdmmc variant revision v3.0
mmc: mmci: stm32: manage block gap hardware flow control
mmc: mmci: stm32: prepare other delay block support
mmc: mmci: stm32: add delay block support for STM32MP25
.../devicetree/bindings/mmc/arm,pl18x.yaml | 6 +
drivers/mmc/host/mmci.c | 35 ++++
drivers/mmc/host/mmci.h | 8 +-
drivers/mmc/host/mmci_stm32_sdmmc.c | 149 ++++++++++++++++--
4 files changed, 181 insertions(+), 17 deletions(-)
--
2.25.1
For STM32MP25, we'll need to distinguish how is managed the delay block.
This is done through a new comptible dedicated for this SoC, as the
delay block registers are located in SYSCFG peripheral.
Signed-off-by: Yann Gautier <[email protected]>
---
Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
index 1c96da04f0e53..e47b3418b6c77 100644
--- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
+++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
@@ -59,6 +59,12 @@ properties:
- const: st,stm32-sdmmc2
- const: arm,pl18x
- const: arm,primecell
+ - description: Entry for STMicroelectronics variant of PL18x for
+ STM32MP25. This dedicated compatible is used by bootloaders.
+ items:
+ - const: st,stm32mp25-sdmmc2
+ - const: arm,pl18x
+ - const: arm,primecell
clocks:
description: One or two clocks, the "apb_pclk" and the "MCLK"
--
2.25.1
The alignment for the IDMA size depends on the peripheral version, it
should then be configurable. Add stm32_idmabsize_align in the variant
structure.
And remove now unused (and wrong) MMCI_STM32_IDMABNDT_* macros.
Signed-off-by: Yann Gautier <[email protected]>
---
drivers/mmc/host/mmci.c | 2 ++
drivers/mmc/host/mmci.h | 3 +--
drivers/mmc/host/mmci_stm32_sdmmc.c | 4 ++--
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 726bf772b2e2d..eae3d1c8934cb 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -270,6 +270,7 @@ static struct variant_data variant_stm32_sdmmc = {
.datactrl_any_blocksz = true,
.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.stm32_idmabsize_mask = GENMASK(12, 5),
+ .stm32_idmabsize_align = BIT(5),
.busy_timeout = true,
.busy_detect = true,
.busy_detect_flag = MCI_STM32_BUSYD0,
@@ -296,6 +297,7 @@ static struct variant_data variant_stm32_sdmmcv2 = {
.datactrl_any_blocksz = true,
.datactrl_mask_sdio = MCI_DPSM_ST_SDIOEN,
.stm32_idmabsize_mask = GENMASK(16, 5),
+ .stm32_idmabsize_align = BIT(5),
.dma_lli = true,
.busy_timeout = true,
.busy_detect = true,
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 12a7bbd3ce263..b1968cafc58bb 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -227,8 +227,6 @@
#define MMCI_STM32_IDMALLIEN BIT(1)
#define MMCI_STM32_IDMABSIZER 0x054
-#define MMCI_STM32_IDMABNDT_SHIFT 5
-#define MMCI_STM32_IDMABNDT_MASK GENMASK(12, 5)
#define MMCI_STM32_IDMABASE0R 0x058
@@ -374,6 +372,7 @@ struct variant_data {
u32 opendrain;
u8 dma_lli:1;
u32 stm32_idmabsize_mask;
+ u32 stm32_idmabsize_align;
void (*init)(struct mmci_host *host);
};
diff --git a/drivers/mmc/host/mmci_stm32_sdmmc.c b/drivers/mmc/host/mmci_stm32_sdmmc.c
index 50292f9c69046..7f43506b9bb08 100644
--- a/drivers/mmc/host/mmci_stm32_sdmmc.c
+++ b/drivers/mmc/host/mmci_stm32_sdmmc.c
@@ -15,7 +15,6 @@
#include "mmci.h"
#define SDMMC_LLI_BUF_LEN PAGE_SIZE
-#define SDMMC_IDMA_BURST BIT(MMCI_STM32_IDMABNDT_SHIFT)
#define DLYB_CR 0x0
#define DLYB_CR_DEN BIT(0)
@@ -69,7 +68,8 @@ static int sdmmc_idma_validate_data(struct mmci_host *host,
idma->use_bounce_buffer = false;
for_each_sg(data->sg, sg, data->sg_len - 1, i) {
if (!IS_ALIGNED(sg->offset, sizeof(u32)) ||
- !IS_ALIGNED(sg->length, SDMMC_IDMA_BURST)) {
+ !IS_ALIGNED(sg->length,
+ host->variant->stm32_idmabsize_align)) {
dev_dbg(mmc_dev(host->mmc),
"unaligned scatterlist: ofst:%x length:%d\n",
data->sg->offset, data->sg->length);
--
2.25.1
On Thu, 15 Jun 2023 at 11:20, Yann Gautier <[email protected]> wrote:
>
> For STM32MP25, we'll need to distinguish how is managed the delay block.
> This is done through a new comptible dedicated for this SoC, as the
> delay block registers are located in SYSCFG peripheral.
>
> Signed-off-by: Yann Gautier <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> index 1c96da04f0e53..e47b3418b6c77 100644
> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> @@ -59,6 +59,12 @@ properties:
> - const: st,stm32-sdmmc2
> - const: arm,pl18x
> - const: arm,primecell
> + - description: Entry for STMicroelectronics variant of PL18x for
> + STM32MP25. This dedicated compatible is used by bootloaders.
What does this last sentence mean? Can we drop it?
> + items:
> + - const: st,stm32mp25-sdmmc2
> + - const: arm,pl18x
> + - const: arm,primecell
>
> clocks:
> description: One or two clocks, the "apb_pclk" and the "MCLK"
> --
> 2.25.1
>
Kind regards
Uffe
On 6/15/23 17:16, Yann Gautier wrote:
> On 6/15/23 15:20, Ulf Hansson wrote:
>> On Thu, 15 Jun 2023 at 11:20, Yann Gautier <[email protected]>
>> wrote:
>>>
>>> For STM32MP25, we'll need to distinguish how is managed the delay block.
>>> This is done through a new comptible dedicated for this SoC, as the
>>> delay block registers are located in SYSCFG peripheral.
>>>
>>> Signed-off-by: Yann Gautier <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> index 1c96da04f0e53..e47b3418b6c77 100644
>>> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>>> @@ -59,6 +59,12 @@ properties:
>>> - const: st,stm32-sdmmc2
>>> - const: arm,pl18x
>>> - const: arm,primecell
>>> + - description: Entry for STMicroelectronics variant of PL18x for
>>> + STM32MP25. This dedicated compatible is used by bootloaders.
>>
>> What does this last sentence mean? Can we drop it?
>
> Hi Ulf,
>
> I just copied (and added "for STM32MP25") what was done for STM32MP1x:
> - description: Entry for STMicroelectronics variant of PL18x.
> This dedicated compatible is used by bootloaders.
> items:
> - const: st,stm32-sdmmc2
> - const: arm,pl18x
> - const: arm,primecell
> - description: Entry for STMicroelectronics variant of PL18x for
> STM32MP25. This dedicated compatible is used by bootloaders.
> items:
> - const: st,stm32mp25-sdmmc2
> - const: arm,pl18x
> - const: arm,primecell
>
>
> Should I remove (or adapt) both descriptions?
>
>
> Best regards,
> Yann
>
At the time the patch was done it was really just used by bootloaders.
But as it is now used in the driver for delay block, I should remove the
second sentence.
Yann
>>
>>> + items:
>>> + - const: st,stm32mp25-sdmmc2
>>> + - const: arm,pl18x
>>> + - const: arm,primecell
>>>
>>> clocks:
>>> description: One or two clocks, the "apb_pclk" and the "MCLK"
>>> --
>>> 2.25.1
>>>
>>
>> Kind regards
>> Uffe
>
On 6/15/23 15:20, Ulf Hansson wrote:
> On Thu, 15 Jun 2023 at 11:20, Yann Gautier <[email protected]> wrote:
>>
>> For STM32MP25, we'll need to distinguish how is managed the delay block.
>> This is done through a new comptible dedicated for this SoC, as the
>> delay block registers are located in SYSCFG peripheral.
>>
>> Signed-off-by: Yann Gautier <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> index 1c96da04f0e53..e47b3418b6c77 100644
>> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
>> @@ -59,6 +59,12 @@ properties:
>> - const: st,stm32-sdmmc2
>> - const: arm,pl18x
>> - const: arm,primecell
>> + - description: Entry for STMicroelectronics variant of PL18x for
>> + STM32MP25. This dedicated compatible is used by bootloaders.
>
> What does this last sentence mean? Can we drop it?
Hi Ulf,
I just copied (and added "for STM32MP25") what was done for STM32MP1x:
- description: Entry for STMicroelectronics variant of PL18x.
This dedicated compatible is used by bootloaders.
items:
- const: st,stm32-sdmmc2
- const: arm,pl18x
- const: arm,primecell
- description: Entry for STMicroelectronics variant of PL18x for
STM32MP25. This dedicated compatible is used by bootloaders.
items:
- const: st,stm32mp25-sdmmc2
- const: arm,pl18x
- const: arm,primecell
Should I remove (or adapt) both descriptions?
Best regards,
Yann
>
>> + items:
>> + - const: st,stm32mp25-sdmmc2
>> + - const: arm,pl18x
>> + - const: arm,primecell
>>
>> clocks:
>> description: One or two clocks, the "apb_pclk" and the "MCLK"
>> --
>> 2.25.1
>>
>
> Kind regards
> Uffe
On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <[email protected]> wrote:
> > - description: Entry for STMicroelectronics variant of PL18x.
> > This dedicated compatible is used by bootloaders.
(...)
> > - description: Entry for STMicroelectronics variant of PL18x for
> > STM32MP25. This dedicated compatible is used by bootloaders.
(...)
> > Should I remove (or adapt) both descriptions?
> >
> >
>
> At the time the patch was done it was really just used by bootloaders.
> But as it is now used in the driver for delay block, I should remove the
> second sentence.
Remove both.
After "This dedicated compatible is used by bootloaders" there is
an implicit "in the SDK provided by ST Microelectronics", and that
is of no concern for DT bindings, which are (well, in theory) used by
e.g. BSD or other operating systems and who knows what they will
use and not, we don't put Linux specifics in there, neither Boot
loader specifics nor ST SDK specifics.
At least that is the little bureaucratic ambition we have.
Yours,
Linus Walleij
On 15/06/2023 11:19, Yann Gautier wrote:
> For STM32MP25, we'll need to distinguish how is managed the delay block.
> This is done through a new comptible dedicated for this SoC, as the
> delay block registers are located in SYSCFG peripheral.
>
> Signed-off-by: Yann Gautier <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/arm,pl18x.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> index 1c96da04f0e53..e47b3418b6c77 100644
> --- a/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> +++ b/Documentation/devicetree/bindings/mmc/arm,pl18x.yaml
> @@ -59,6 +59,12 @@ properties:
> - const: st,stm32-sdmmc2
> - const: arm,pl18x
> - const: arm,primecell
> + - description: Entry for STMicroelectronics variant of PL18x for
> + STM32MP25. This dedicated compatible is used by bootloaders.
> + items:
> + - const: st,stm32mp25-sdmmc2
Except what's said, this looks like can be part of previous entry via enum.
Best regards,
Krzysztof
On 6/15/23 20:51, Linus Walleij wrote:
> On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <[email protected]> wrote:
>
>>> - description: Entry for STMicroelectronics variant of PL18x.
>>> This dedicated compatible is used by bootloaders.
> (...)
>>> - description: Entry for STMicroelectronics variant of PL18x for
>>> STM32MP25. This dedicated compatible is used by bootloaders.
> (...)
>>> Should I remove (or adapt) both descriptions?
>>>
>>>
>>
>> At the time the patch was done it was really just used by bootloaders.
>> But as it is now used in the driver for delay block, I should remove the
>> second sentence.
>
> Remove both.
>
> After "This dedicated compatible is used by bootloaders" there is
> an implicit "in the SDK provided by ST Microelectronics", and that
> is of no concern for DT bindings, which are (well, in theory) used by
> e.g. BSD or other operating systems and who knows what they will
> use and not, we don't put Linux specifics in there, neither Boot
> loader specifics nor ST SDK specifics.
>
> At least that is the little bureaucratic ambition we have.
>
> Yours,
> Linus Walleij
Hi,
Thanks for all the reviews.
I'll update this patch in the v2, removing bootloader line and using enum.
Ulf, should I send the new series now, or do you prefer to review the
whole series before?
Yann
On Mon, 19 Jun 2023 at 09:29, Yann Gautier <[email protected]> wrote:
>
> On 6/15/23 20:51, Linus Walleij wrote:
> > On Thu, Jun 15, 2023 at 5:19 PM Yann Gautier <[email protected]> wrote:
> >
> >>> - description: Entry for STMicroelectronics variant of PL18x.
> >>> This dedicated compatible is used by bootloaders.
> > (...)
> >>> - description: Entry for STMicroelectronics variant of PL18x for
> >>> STM32MP25. This dedicated compatible is used by bootloaders.
> > (...)
> >>> Should I remove (or adapt) both descriptions?
> >>>
> >>>
> >>
> >> At the time the patch was done it was really just used by bootloaders.
> >> But as it is now used in the driver for delay block, I should remove the
> >> second sentence.
> >
> > Remove both.
> >
> > After "This dedicated compatible is used by bootloaders" there is
> > an implicit "in the SDK provided by ST Microelectronics", and that
> > is of no concern for DT bindings, which are (well, in theory) used by
> > e.g. BSD or other operating systems and who knows what they will
> > use and not, we don't put Linux specifics in there, neither Boot
> > loader specifics nor ST SDK specifics.
> >
> > At least that is the little bureaucratic ambition we have.
> >
> > Yours,
> > Linus Walleij
>
> Hi,
>
> Thanks for all the reviews.
> I'll update this patch in the v2, removing bootloader line and using enum.
>
> Ulf, should I send the new series now, or do you prefer to review the
> whole series before?
Actually I have already looked through the series and it looks good to
me! Please submit a new version so we can get this queued up for v6.5.
Kind regards
Uffe