| Message ID | 1635302841-18997-1-git-send-email-haibo.chen@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: sdhci: disable ADMA when PAGE SIZE is 64K | expand |
On 27/10/2021 05:47, haibo.chen@nxp.com wrote: > From: Haibo Chen <haibo.chen@nxp.com> > > For ADMA with quirks SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, it limit > the max segment size as 64K-1. Currently, linux kernel support > 4K/16K/64K PAGE_SIZE. If choose 64K PAGE_SIZE, sdhci in ADMA mode > with the upper quirks will meet issue. > > Though mmc driver give block layer the max segment size through > blk_queue_max_segment_size(), but in this function, it will compare > the argument with PAGE_SIZE, and will choose PAGE_SIZE if the argument > is smaller than PAGE_SIZE. When PAGE_SIZE is 64K, finally the mmc > request queue will get 64K as the max segment size. Once the sg data > length=64K, ADMA will meet issue. For this case, need to disable ADMA > mode. Sorry, I didn't look closely enough at this. For the 64K PAGE_SIZE case, did you try blk_queue_virt_boundary() with a 32K mask instead of blk_queue_max_segment_size() ? e.g. a hack for testing purposes like this: diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index b15c034b42fb..4c3a9d999500 100644 --- a/drivers/mmc/core/queue.c +++ b/drivers/mmc/core/queue.c @@ -376,8 +376,7 @@ static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card) * both blk_queue_max_segment_size(). */ if (!host->can_dma_map_merge) - blk_queue_max_segment_size(mq->queue, - round_down(host->max_seg_size, block_size)); + blk_queue_virt_boundary(mq->queue, 0x7fff); dma_set_max_seg_size(mmc_dev(host), queue_max_segment_size(mq->queue)); > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > --- > drivers/mmc/host/sdhci.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index dbe87995596c..47e0c0156b94 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -4187,6 +4187,12 @@ int sdhci_setup_host(struct sdhci_host *host) > host->flags &= ~SDHCI_USE_ADMA; > } > > + if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) && > + PAGE_SIZE >= 65536 && (host->flags & SDHCI_USE_ADMA)) { > + DBG("Disabling ADMA as it don't support >= 64K PAGE_SIZE\n"); > + host->flags &= ~SDHCI_USE_ADMA; > + } > + > if (sdhci_can_64bit_dma(host)) > host->flags |= SDHCI_USE_64_BIT_DMA; > >
> -----Original Message----- > From: Adrian Hunter [mailto:adrian.hunter@intel.com] > Sent: 2021年10月27日 13:50 > To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org > Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K > > On 27/10/2021 05:47, haibo.chen@nxp.com wrote: > > From: Haibo Chen <haibo.chen@nxp.com> > > > > For ADMA with quirks SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, it > limit > > the max segment size as 64K-1. Currently, linux kernel support > > 4K/16K/64K PAGE_SIZE. If choose 64K PAGE_SIZE, sdhci in ADMA mode with > > the upper quirks will meet issue. > > > > Though mmc driver give block layer the max segment size through > > blk_queue_max_segment_size(), but in this function, it will compare > > the argument with PAGE_SIZE, and will choose PAGE_SIZE if the argument > > is smaller than PAGE_SIZE. When PAGE_SIZE is 64K, finally the mmc > > request queue will get 64K as the max segment size. Once the sg data > > length=64K, ADMA will meet issue. For this case, need to disable ADMA > > mode. > > Sorry, I didn't look closely enough at this. > > For the 64K PAGE_SIZE case, did you try blk_queue_virt_boundary() with a 32K > mask instead of blk_queue_max_segment_size() ? > e.g. a hack for testing purposes like this: I test on my board, this do not work, I still can see sg data_length = 65536. Best Regards Haibo Chen > > diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index > b15c034b42fb..4c3a9d999500 100644 > --- a/drivers/mmc/core/queue.c > +++ b/drivers/mmc/core/queue.c > @@ -376,8 +376,7 @@ static void mmc_setup_queue(struct mmc_queue *mq, > struct mmc_card *card) > * both blk_queue_max_segment_size(). > */ > if (!host->can_dma_map_merge) > - blk_queue_max_segment_size(mq->queue, > - round_down(host->max_seg_size, block_size)); > + blk_queue_virt_boundary(mq->queue, 0x7fff); > > dma_set_max_seg_size(mmc_dev(host), > queue_max_segment_size(mq->queue)); > > > > > > > > Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > > --- > > drivers/mmc/host/sdhci.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > > dbe87995596c..47e0c0156b94 100644 > > --- a/drivers/mmc/host/sdhci.c > > +++ b/drivers/mmc/host/sdhci.c > > @@ -4187,6 +4187,12 @@ int sdhci_setup_host(struct sdhci_host *host) > > host->flags &= ~SDHCI_USE_ADMA; > > } > > > > + if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) && > > + PAGE_SIZE >= 65536 && (host->flags & SDHCI_USE_ADMA)) { > > + DBG("Disabling ADMA as it don't support >= 64K PAGE_SIZE\n"); > > + host->flags &= ~SDHCI_USE_ADMA; > > + } > > + > > if (sdhci_can_64bit_dma(host)) > > host->flags |= SDHCI_USE_64_BIT_DMA; > > > >
On 29/10/2021 14:40, Bough Chen wrote: >> -----Original Message----- >> From: Adrian Hunter [mailto:adrian.hunter@intel.com] >> Sent: 2021年10月27日 13:50 >> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org >> Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> >> Subject: Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K >> >> On 27/10/2021 05:47, haibo.chen@nxp.com wrote: >>> From: Haibo Chen <haibo.chen@nxp.com> >>> >>> For ADMA with quirks SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, it >> limit >>> the max segment size as 64K-1. Currently, linux kernel support >>> 4K/16K/64K PAGE_SIZE. If choose 64K PAGE_SIZE, sdhci in ADMA mode with >>> the upper quirks will meet issue. >>> >>> Though mmc driver give block layer the max segment size through >>> blk_queue_max_segment_size(), but in this function, it will compare >>> the argument with PAGE_SIZE, and will choose PAGE_SIZE if the argument >>> is smaller than PAGE_SIZE. When PAGE_SIZE is 64K, finally the mmc >>> request queue will get 64K as the max segment size. Once the sg data >>> length=64K, ADMA will meet issue. For this case, need to disable ADMA >>> mode. >> >> Sorry, I didn't look closely enough at this. >> >> For the 64K PAGE_SIZE case, did you try blk_queue_virt_boundary() with a 32K >> mask instead of blk_queue_max_segment_size() ? >> e.g. a hack for testing purposes like this: > > I test on my board, this do not work, I still can see sg data_length = 65536. What about tweaking the logic to write ADMA descriptors? Something like below for instance? 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 */
> -----Original Message----- > From: Adrian Hunter [mailto:adrian.hunter@intel.com] > Sent: 2021年11月9日 16:42 > To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org > Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > Subject: Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K > > On 29/10/2021 14:40, Bough Chen wrote: > >> -----Original Message----- > >> From: Adrian Hunter [mailto:adrian.hunter@intel.com] > >> Sent: 2021年10月27日 13:50 > >> To: Bough Chen <haibo.chen@nxp.com>; ulf.hansson@linaro.org > >> Cc: linux-mmc@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com> > >> Subject: Re: [PATCH] mmc: sdhci: disable ADMA when PAGE SIZE is 64K > >> > >> On 27/10/2021 05:47, haibo.chen@nxp.com wrote: > >>> From: Haibo Chen <haibo.chen@nxp.com> > >>> > >>> For ADMA with quirks SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC, it > >> limit > >>> the max segment size as 64K-1. Currently, linux kernel support > >>> 4K/16K/64K PAGE_SIZE. If choose 64K PAGE_SIZE, sdhci in ADMA mode > >>> with the upper quirks will meet issue. > >>> > >>> Though mmc driver give block layer the max segment size through > >>> blk_queue_max_segment_size(), but in this function, it will compare > >>> the argument with PAGE_SIZE, and will choose PAGE_SIZE if the > >>> argument is smaller than PAGE_SIZE. When PAGE_SIZE is 64K, finally > >>> the mmc request queue will get 64K as the max segment size. Once the > >>> sg data length=64K, ADMA will meet issue. For this case, need to > >>> disable ADMA mode. > >> > >> Sorry, I didn't look closely enough at this. > >> > >> For the 64K PAGE_SIZE case, did you try blk_queue_virt_boundary() > >> with a 32K mask instead of blk_queue_max_segment_size() ? > >> e.g. a hack for testing purposes like this: > > > > I test on my board, this do not work, I still can see sg data_length = 65536. > > What about tweaking the logic to write ADMA descriptors? > Something like below for instance? Hi Adrian, I prefer this solution. I test this on my board, it works fine. I think you can send this patch out with my Tested-by. Best Regards Haibo Chen > > > > 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 > > > > > > > > Best Regards > > Haibo Chen > >> > >> diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c index > >> b15c034b42fb..4c3a9d999500 100644 > >> --- a/drivers/mmc/core/queue.c > >> +++ b/drivers/mmc/core/queue.c > >> @@ -376,8 +376,7 @@ static void mmc_setup_queue(struct mmc_queue > *mq, > >> struct mmc_card *card) > >> * both blk_queue_max_segment_size(). > >> */ > >> if (!host->can_dma_map_merge) > >> - blk_queue_max_segment_size(mq->queue, > >> - round_down(host->max_seg_size, block_size)); > >> + blk_queue_virt_boundary(mq->queue, 0x7fff); > >> > >> dma_set_max_seg_size(mmc_dev(host), > >> queue_max_segment_size(mq->queue)); > >> > >> > >> > >> > >>> > >>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com> > >>> --- > >>> drivers/mmc/host/sdhci.c | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index > >>> dbe87995596c..47e0c0156b94 100644 > >>> --- a/drivers/mmc/host/sdhci.c > >>> +++ b/drivers/mmc/host/sdhci.c > >>> @@ -4187,6 +4187,12 @@ int sdhci_setup_host(struct sdhci_host *host) > >>> host->flags &= ~SDHCI_USE_ADMA; > >>> } > >>> > >>> + if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) > && > >>> + PAGE_SIZE >= 65536 && (host->flags & SDHCI_USE_ADMA)) { > >>> + DBG("Disabling ADMA as it don't support >= 64K > PAGE_SIZE\n"); > >>> + host->flags &= ~SDHCI_USE_ADMA; > >>> + } > >>> + > >>> if (sdhci_can_64bit_dma(host)) > >>> host->flags |= SDHCI_USE_64_BIT_DMA; > >>> > >>> > >
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c index dbe87995596c..47e0c0156b94 100644 --- a/drivers/mmc/host/sdhci.c +++ b/drivers/mmc/host/sdhci.c @@ -4187,6 +4187,12 @@ int sdhci_setup_host(struct sdhci_host *host) host->flags &= ~SDHCI_USE_ADMA; } + if ((host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) && + PAGE_SIZE >= 65536 && (host->flags & SDHCI_USE_ADMA)) { + DBG("Disabling ADMA as it don't support >= 64K PAGE_SIZE\n"); + host->flags &= ~SDHCI_USE_ADMA; + } + if (sdhci_can_64bit_dma(host)) host->flags |= SDHCI_USE_64_BIT_DMA;