| Message ID | 20240501185331.1189647-1-alexander.sverdlin@siemens.com |
|---|---|
| State | Accepted |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | mmc: sdhci: Correct ADMA_DESC_LEN to 12 | expand |
"A. Sverdlin" <alexander.sverdlin@siemens.com> writes: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Commit 37cb626da25d0d ("mmc: sdhci: Add Support for ADMA2") introduced > ADMA_DESC_LEN == 16 (64 bit case), but it was never used before commit > 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops"). > > "sizeof(struct sdhci_adma_desc)" (== 12 for 64bit case) was used instead. > > Confusion probably originates from Linux commit 685e444bbaa0 > ("mmc: sdhci: Add ADMA2 64-bit addressing support for V4 mode"), but > the latter "V4 mode" was never ported to U-Boot. > > Fixes: 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops") > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > include/sdhci.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/sdhci.h b/include/sdhci.h > index d73a725609be3..810ef56e4be66 100644 > --- a/include/sdhci.h > +++ b/include/sdhci.h > @@ -300,7 +300,7 @@ struct sdhci_ops { > > #define ADMA_MAX_LEN 65532 > #ifdef CONFIG_DMA_ADDR_T_64BIT > -#define ADMA_DESC_LEN 16 > +#define ADMA_DESC_LEN 12 > #else > #define ADMA_DESC_LEN 8 > #endif > -- > 2.44.0 on TI AM64 and AM62p platforms: Tested-by: Jonathan Humphreys <j-humphreys@ti.com>
Thanks for fixing this for me. > Confusion probably originates from Linux commit 685e444bbaa0 > ("mmc: sdhci: Add ADMA2 64-bit addressing support for V4 mode"), but > the latter "V4 mode" was never ported to U-Boot. I have one of the rare platforms that does not support 64-bit ADMA2 V3 so we also submitted 5359cd1135 ("mmc: Support 32-bit only ADMA on 64-bit platforms") to support that, but if we saw hardware that required v4 descriptors or someone insisted on it, we'd need to add another Kconfig for it. Do you think we should port the rest of the v4 support? What about mirroring the kernel's dynamic behavior by checking the combination of capabilities and control registers to figure out which mode to use rather than hardcoding it? -- Greg Malysa Timesys Corporation
Hi Greg, On Wed, 2024-05-01 at 22:38 -0400, Greg Malysa wrote: > Thanks for fixing this for me. > > > Confusion probably originates from Linux commit 685e444bbaa0 > > ("mmc: sdhci: Add ADMA2 64-bit addressing support for V4 mode"), but > > the latter "V4 mode" was never ported to U-Boot. > > I have one of the rare platforms that does not support 64-bit ADMA2 V3 > so we also submitted 5359cd1135 ("mmc: Support 32-bit only ADMA on > 64-bit platforms") to support that, but if we saw hardware that > required v4 descriptors or someone insisted on it, we'd need to add > another Kconfig for it. Do you think we should port the rest of the v4 > support? What about mirroring the kernel's dynamic behavior by > checking the combination of capabilities and control registers to > figure out which mode to use rather than hardcoding it? current u-boot master contains ADMA_DESC_LEN value from 685e444bbaa0 without all the pre-requisites. My patch changes it to 685e444bbaa0^ state. My patch will help to apply 685e444bbaa0^^^...685e444bbaa0, but I'd prefer someone with some experience in this area would do it, I'm personally not so sound in SD/MMC controller spec.
Hi, On 5/1/24 1:53 PM, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Commit 37cb626da25d0d ("mmc: sdhci: Add Support for ADMA2") introduced > ADMA_DESC_LEN == 16 (64 bit case), but it was never used before commit > 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops"). > > "sizeof(struct sdhci_adma_desc)" (== 12 for 64bit case) was used instead. > > Confusion probably originates from Linux commit 685e444bbaa0 > ("mmc: sdhci: Add ADMA2 64-bit addressing support for V4 mode"), but > the latter "V4 mode" was never ported to U-Boot. > > Fixes: 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops") > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> > --- > include/sdhci.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/sdhci.h b/include/sdhci.h > index d73a725609be3..810ef56e4be66 100644 > --- a/include/sdhci.h > +++ b/include/sdhci.h > @@ -300,7 +300,7 @@ struct sdhci_ops { > > #define ADMA_MAX_LEN 65532 > #ifdef CONFIG_DMA_ADDR_T_64BIT > -#define ADMA_DESC_LEN 16 > +#define ADMA_DESC_LEN 12 > #else > #define ADMA_DESC_LEN 8 > #endif on TI AM62 platform: Tested-by: Judith Mendez <jm@ti.com>
On Wed, May 01, 2024 at 08:53:04PM +0200, A. Sverdlin wrote: > From: Alexander Sverdlin <alexander.sverdlin@siemens.com> > > Commit 37cb626da25d0d ("mmc: sdhci: Add Support for ADMA2") introduced > ADMA_DESC_LEN == 16 (64 bit case), but it was never used before commit > 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops"). > > "sizeof(struct sdhci_adma_desc)" (== 12 for 64bit case) was used instead. > > Confusion probably originates from Linux commit 685e444bbaa0 > ("mmc: sdhci: Add ADMA2 64-bit addressing support for V4 mode"), but > the latter "V4 mode" was never ported to U-Boot. > > Fixes: 74755c1fed1b0 ("mmc: sdhci: introduce adma_write_desc() hook to struct sdhci_ops") > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com> Applied to u-boot/master, thanks!
diff --git a/include/sdhci.h b/include/sdhci.h index d73a725609be3..810ef56e4be66 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -300,7 +300,7 @@ struct sdhci_ops { #define ADMA_MAX_LEN 65532 #ifdef CONFIG_DMA_ADDR_T_64BIT -#define ADMA_DESC_LEN 16 +#define ADMA_DESC_LEN 12 #else #define ADMA_DESC_LEN 8 #endif