2021-04-26 17:57:26

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH 1/2] mmc: meson-gx: make replace WARN_ONCE with dev_warn_once about scatterlist offset alignment

Some drivers like ath10k can sometimg give an sg buffer with an offset whose alignment
is not compatible with the Amlogic DMA descriptor engine requirements.

Simply replace with dev_warn_once() to inform user this should be fixed to avoid
degraded performance.

This should be ultimately fixed in ath10k, but since it's only a performance issue
the warning should be removed.

Fixes: 79ed05e329c3 ("mmc: meson-gx: add support for descriptor chain mode")
Reported-by: Christian Hewitt <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/mmc/host/meson-gx-mmc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index b8b771b643cc..1c61f0f24c09 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -258,7 +258,9 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc,
for_each_sg(data->sg, sg, data->sg_len, i) {
/* check for 8 byte alignment */
if (sg->offset % 8) {
- WARN_ONCE(1, "unaligned scatterlist buffer\n");
+ dev_warn_once(mmc_dev(mmc),
+ "unaligned sg offset %u, disabling descriptor DMA for transfer\n",
+ sg->offset);
return;
}
}
--
2.25.1


2021-04-26 17:57:31

by Neil Armstrong

[permalink] [raw]
Subject: [PATCH 2/2] mmc: meson-gx: also check SD_IO_RW_EXTENDED for scatterlist size alignment

The brcmfmac driver can generate a scatterlist from a skb with each packets
not aligned to the block size. This is not supported by the Amlogic Descriptor
dma engine where each descriptor must match a multiple of the block size.

The sg list is valid, since the sum of the sg buffers is a multiple of the
block size, but we must discard those when in SD_IO_RW_EXTENDED mode since
SDIO block mode can be used under the hood even with data->blocks == 1.

Those transfers are very rare, thus can be replaced by a bounce buffer
without real performance loss.

Fixes: 7412dee9f1fd ("mmc: meson-gx: replace WARN_ONCE with dev_warn_once about scatterlist size alignment in block mode")
Reported-by: Christian Hewitt <[email protected]>
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/mmc/host/meson-gx-mmc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
index 1c61f0f24c09..016a6106151a 100644
--- a/drivers/mmc/host/meson-gx-mmc.c
+++ b/drivers/mmc/host/meson-gx-mmc.c
@@ -236,7 +236,8 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc,
if (host->dram_access_quirk)
return;

- if (data->blocks > 1) {
+ /* SD_IO_RW_EXTENDED (CMD53) can also use block mode under the hood */
+ if (data->blocks > 1 || mrq->cmd->opcode == SD_IO_RW_EXTENDED) {
/*
* In block mode DMA descriptor format, "length" field indicates
* number of blocks and there is no way to pass DMA size that
--
2.25.1

2021-04-27 18:38:06

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH 1/2] mmc: meson-gx: make replace WARN_ONCE with dev_warn_once about scatterlist offset alignment

Hi Neil,

On Mon, Apr 26, 2021 at 7:58 PM Neil Armstrong <[email protected]> wrote:
>
> Some drivers like ath10k can sometimg give an sg buffer with an offset whose alignment
sometimg -> sometimes

> is not compatible with the Amlogic DMA descriptor engine requirements.
>
> Simply replace with dev_warn_once() to inform user this should be fixed to avoid
> degraded performance.
>
> This should be ultimately fixed in ath10k, but since it's only a performance issue
> the warning should be removed.
>
> Fixes: 79ed05e329c3 ("mmc: meson-gx: add support for descriptor chain mode")
> Reported-by: Christian Hewitt <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>
Acked-by: Martin Blumenstingl <[email protected]>

2021-05-11 10:57:41

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 1/2] mmc: meson-gx: make replace WARN_ONCE with dev_warn_once about scatterlist offset alignment

On Mon, 26 Apr 2021 at 19:56, Neil Armstrong <[email protected]> wrote:
>
> Some drivers like ath10k can sometimg give an sg buffer with an offset whose alignment
> is not compatible with the Amlogic DMA descriptor engine requirements.
>
> Simply replace with dev_warn_once() to inform user this should be fixed to avoid
> degraded performance.
>
> This should be ultimately fixed in ath10k, but since it's only a performance issue
> the warning should be removed.
>
> Fixes: 79ed05e329c3 ("mmc: meson-gx: add support for descriptor chain mode")
> Reported-by: Christian Hewitt <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
> drivers/mmc/host/meson-gx-mmc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index b8b771b643cc..1c61f0f24c09 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -258,7 +258,9 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc,
> for_each_sg(data->sg, sg, data->sg_len, i) {
> /* check for 8 byte alignment */
> if (sg->offset % 8) {
> - WARN_ONCE(1, "unaligned scatterlist buffer\n");
> + dev_warn_once(mmc_dev(mmc),
> + "unaligned sg offset %u, disabling descriptor DMA for transfer\n",
> + sg->offset);
> return;
> }
> }
> --
> 2.25.1
>

2021-05-11 10:58:51

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH 2/2] mmc: meson-gx: also check SD_IO_RW_EXTENDED for scatterlist size alignment

On Mon, 26 Apr 2021 at 19:56, Neil Armstrong <[email protected]> wrote:
>
> The brcmfmac driver can generate a scatterlist from a skb with each packets
> not aligned to the block size. This is not supported by the Amlogic Descriptor
> dma engine where each descriptor must match a multiple of the block size.
>
> The sg list is valid, since the sum of the sg buffers is a multiple of the
> block size, but we must discard those when in SD_IO_RW_EXTENDED mode since
> SDIO block mode can be used under the hood even with data->blocks == 1.
>
> Those transfers are very rare, thus can be replaced by a bounce buffer
> without real performance loss.
>
> Fixes: 7412dee9f1fd ("mmc: meson-gx: replace WARN_ONCE with dev_warn_once about scatterlist size alignment in block mode")
> Reported-by: Christian Hewitt <[email protected]>
> Signed-off-by: Neil Armstrong <[email protected]>

Applied for fixes and by adding a stable tag, thanks!

Kind regards
Uffe


> ---
> drivers/mmc/host/meson-gx-mmc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/meson-gx-mmc.c b/drivers/mmc/host/meson-gx-mmc.c
> index 1c61f0f24c09..016a6106151a 100644
> --- a/drivers/mmc/host/meson-gx-mmc.c
> +++ b/drivers/mmc/host/meson-gx-mmc.c
> @@ -236,7 +236,8 @@ static void meson_mmc_get_transfer_mode(struct mmc_host *mmc,
> if (host->dram_access_quirk)
> return;
>
> - if (data->blocks > 1) {
> + /* SD_IO_RW_EXTENDED (CMD53) can also use block mode under the hood */
> + if (data->blocks > 1 || mrq->cmd->opcode == SD_IO_RW_EXTENDED) {
> /*
> * In block mode DMA descriptor format, "length" field indicates
> * number of blocks and there is no way to pass DMA size that
> --
> 2.25.1
>