| Message ID | 20211115082345.802238-1-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci: Fix ADMA for PAGE_SIZE >= 64KiB | expand |
On Mon, 15 Nov 2021 at 09:24, Adrian Hunter <adrian.hunter@intel.com> wrote: > > The block layer forces a minimum segment size of PAGE_SIZE, so a segment > can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing > multiple descriptors, noting that the ADMA table is sized for 4KiB chunks > anyway, so it will be big enough. > > Reported-and-tested-by: Bough Chen <haibo.chen@nxp.com> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Applied for next, thanks! Should we perhaps tag this for stable and/or queue it as a fix for v5.16? Kind regards Uffe > --- > drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++--- > drivers/mmc/host/sdhci.h | 4 +++- > 2 files changed, 21 insertions(+), 4 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index 269c86569402..07c6da1f2f0f 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host, > len -= offset; > } > > - BUG_ON(len > 65536); > + /* > + * The block layer forces a minimum segment size of PAGE_SIZE, > + * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write > + * multiple descriptors, noting that the ADMA table is sized > + * for 4KiB chunks anyway, so it will be big enough. > + */ > + while (len > host->max_adma) { > + int n = 32 * 1024; /* 32KiB*/ > + > + __sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID); > + addr += n; > + len -= n; > + } > > /* tran, valid */ > if (len) > @@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev, > * descriptor for each segment, plus 1 for a nop end descriptor. > */ > host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1; > + host->max_adma = 65536; > > host->max_timeout_count = 0xE; > > @@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host) > * be larger than 64 KiB though. > */ > if (host->flags & SDHCI_USE_ADMA) { > - if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) > + if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) { > + host->max_adma = 65532; /* 32-bit alignment */ > mmc->max_seg_size = 65535; > - else > + } else { > mmc->max_seg_size = 65536; > + } > } else { > mmc->max_seg_size = mmc->max_req_size; > } > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index bb883553d3b4..d7929d725730 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc { > > /* > * Maximum segments assuming a 512KiB maximum requisition size and a minimum > - * 4KiB page size. > + * 4KiB page size. Note this also allows enough for multiple descriptors in > + * case of PAGE_SIZE >= 64KiB. > */ > #define SDHCI_MAX_SEGS 128 > > @@ -543,6 +544,7 @@ struct sdhci_host { > unsigned int blocks; /* remaining PIO blocks */ > > int sg_count; /* Mapped sg entries */ > + int max_adma; /* Max. length in ADMA descriptor */ > > void *adma_table; /* ADMA descriptor table */ > void *align_buffer; /* Bounce buffer */ > -- > 2.25.1 >
On 15/11/2021 16:54, Ulf Hansson wrote: > On Mon, 15 Nov 2021 at 09:24, Adrian Hunter <adrian.hunter@intel.com> wrote: >> >> The block layer forces a minimum segment size of PAGE_SIZE, so a segment >> can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing >> multiple descriptors, noting that the ADMA table is sized for 4KiB chunks >> anyway, so it will be big enough. >> >> Reported-and-tested-by: Bough Chen <haibo.chen@nxp.com> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > Applied for next, thanks! > > Should we perhaps tag this for stable and/or queue it as a fix for v5.16? Could do. I thought I would leave it up to Haibo Chen. > > Kind regards > Uffe > > > >> --- >> drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++--- >> drivers/mmc/host/sdhci.h | 4 +++- >> 2 files changed, 21 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c >> index 269c86569402..07c6da1f2f0f 100644 >> --- a/drivers/mmc/host/sdhci.c >> +++ b/drivers/mmc/host/sdhci.c >> @@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host, >> len -= offset; >> } >> >> - BUG_ON(len > 65536); >> + /* >> + * The block layer forces a minimum segment size of PAGE_SIZE, >> + * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write >> + * multiple descriptors, noting that the ADMA table is sized >> + * for 4KiB chunks anyway, so it will be big enough. >> + */ >> + while (len > host->max_adma) { >> + int n = 32 * 1024; /* 32KiB*/ >> + >> + __sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID); >> + addr += n; >> + len -= n; >> + } >> >> /* tran, valid */ >> if (len) >> @@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev, >> * descriptor for each segment, plus 1 for a nop end descriptor. >> */ >> host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1; >> + host->max_adma = 65536; >> >> host->max_timeout_count = 0xE; >> >> @@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host) >> * be larger than 64 KiB though. >> */ >> if (host->flags & SDHCI_USE_ADMA) { >> - if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) >> + if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) { >> + host->max_adma = 65532; /* 32-bit alignment */ >> mmc->max_seg_size = 65535; >> - else >> + } else { >> mmc->max_seg_size = 65536; >> + } >> } else { >> mmc->max_seg_size = mmc->max_req_size; >> } >> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h >> index bb883553d3b4..d7929d725730 100644 >> --- a/drivers/mmc/host/sdhci.h >> +++ b/drivers/mmc/host/sdhci.h >> @@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc { >> >> /* >> * Maximum segments assuming a 512KiB maximum requisition size and a minimum >> - * 4KiB page size. >> + * 4KiB page size. Note this also allows enough for multiple descriptors in >> + * case of PAGE_SIZE >= 64KiB. >> */ >> #define SDHCI_MAX_SEGS 128 >> >> @@ -543,6 +544,7 @@ struct sdhci_host { >> unsigned int blocks; /* remaining PIO blocks */ >> >> int sg_count; /* Mapped sg entries */ >> + int max_adma; /* Max. length in ADMA descriptor */ >> >> void *adma_table; /* ADMA descriptor table */ >> void *align_buffer; /* Bounce buffer */ >> -- >> 2.25.1 >>
On Tue, 16 Nov 2021 at 09:14, Adrian Hunter <adrian.hunter@intel.com> wrote: > > On 15/11/2021 16:54, Ulf Hansson wrote: > > On Mon, 15 Nov 2021 at 09:24, Adrian Hunter <adrian.hunter@intel.com> wrote: > >> > >> The block layer forces a minimum segment size of PAGE_SIZE, so a segment > >> can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing > >> multiple descriptors, noting that the ADMA table is sized for 4KiB chunks > >> anyway, so it will be big enough. > >> > >> Reported-and-tested-by: Bough Chen <haibo.chen@nxp.com> > >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > > > Applied for next, thanks! > > > > Should we perhaps tag this for stable and/or queue it as a fix for v5.16? > > Could do. I thought I would leave it up to Haibo Chen. I have moved it to the fixes branch and added a stable tag. Haibo, please tell me if you think that isn't needed. [...] Kind regards Uffe
> -----Original Message----- > From: Ulf Hansson [mailto:ulf.hansson@linaro.org] > Sent: 2021年11月17日 17:38 > To: Adrian Hunter <adrian.hunter@intel.com> > Cc: Bough Chen <haibo.chen@nxp.com>; linux-mmc > <linux-mmc@vger.kernel.org> > Subject: Re: [PATCH] mmc: sdhci: Fix ADMA for PAGE_SIZE >= 64KiB > > On Tue, 16 Nov 2021 at 09:14, Adrian Hunter <adrian.hunter@intel.com> > wrote: > > > > On 15/11/2021 16:54, Ulf Hansson wrote: > > > On Mon, 15 Nov 2021 at 09:24, Adrian Hunter <adrian.hunter@intel.com> > wrote: > > >> > > >> The block layer forces a minimum segment size of PAGE_SIZE, so a > > >> segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. > > >> Fix by writing multiple descriptors, noting that the ADMA table is > > >> sized for 4KiB chunks anyway, so it will be big enough. > > >> > > >> Reported-and-tested-by: Bough Chen <haibo.chen@nxp.com> > > >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > > > > > > Applied for next, thanks! > > > > > > Should we perhaps tag this for stable and/or queue it as a fix for v5.16? > > > > Could do. I thought I would leave it up to Haibo Chen. > > I have moved it to the fixes branch and added a stable tag. Haibo, please tell > me if you think that isn't needed. > I'm okay with this. Best Regards Haibo Chen > [...] > > Kind regards > Uffe
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index 269c86569402..07c6da1f2f0f 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host, len -= offset; } - BUG_ON(len > 65536); + /* + * The block layer forces a minimum segment size of PAGE_SIZE, + * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write + * multiple descriptors, noting that the ADMA table is sized + * for 4KiB chunks anyway, so it will be big enough. + */ + while (len > host->max_adma) { + int n = 32 * 1024; /* 32KiB*/ + + __sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID); + addr += n; + len -= n; + } /* tran, valid */ if (len) @@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev, * descriptor for each segment, plus 1 for a nop end descriptor. */ host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1; + host->max_adma = 65536; host->max_timeout_count = 0xE; @@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host) * be larger than 64 KiB though. */ if (host->flags & SDHCI_USE_ADMA) { - if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) + if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) { + host->max_adma = 65532; /* 32-bit alignment */ mmc->max_seg_size = 65535; - else + } else { mmc->max_seg_size = 65536; + } } else { mmc->max_seg_size = mmc->max_req_size; } diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h index bb883553d3b4..d7929d725730 100644 --- a/drivers/mmc/host/sdhci.h +++ b/drivers/mmc/host/sdhci.h @@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc { /* * Maximum segments assuming a 512KiB maximum requisition size and a minimum - * 4KiB page size. + * 4KiB page size. Note this also allows enough for multiple descriptors in + * case of PAGE_SIZE >= 64KiB. */ #define SDHCI_MAX_SEGS 128 @@ -543,6 +544,7 @@ struct sdhci_host { unsigned int blocks; /* remaining PIO blocks */ int sg_count; /* Mapped sg entries */ + int max_adma; /* Max. length in ADMA descriptor */ void *adma_table; /* ADMA descriptor table */ void *align_buffer; /* Bounce buffer */
The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough. Reported-and-tested-by: Bough Chen <haibo.chen@nxp.com> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++--- drivers/mmc/host/sdhci.h | 4 +++- 2 files changed, 21 insertions(+), 4 deletions(-)