* [PATCH RFC 1/4] mmc: host: renesas_sdhi_sys_dmac: Use dma_buswidth instead of hardcoded value
2019-11-22 6:13 [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Yoshihiro Shimoda
@ 2019-11-22 6:13 ` Yoshihiro Shimoda
2019-11-26 10:45 ` Ulrich Hecht
2019-11-22 6:13 ` [PATCH RFC 2/4] mmc: host: renesas_sdhi_sys_dmac: Do not fall back to PIO Yoshihiro Shimoda
` (3 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-11-22 6:13 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Yoshihiro Shimoda
This patch uses dma_buswidth instread of hardcoded value of
TMIO_MMC_MIN_DMA_LEN (8) for increasing the buswidth in the future.
Note that, since the dma_buswidth is 4 and the align is 2,
when the sg_tmp->length is 6, it cannot transfer correcly.
So, this patch changes the conditions.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 13ff023..30f34a3 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -23,8 +23,6 @@
#include "renesas_sdhi.h"
#include "tmio_mmc.h"
-#define TMIO_MMC_MIN_DMA_LEN 8
-
static const struct renesas_sdhi_of_data of_default_cfg = {
.tmio_flags = TMIO_MMC_HAS_IDLE_WAIT,
};
@@ -159,11 +157,12 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
int ret, i;
bool aligned = true, multiple = true;
unsigned int align = (1 << host->pdata->alignment_shift) - 1;
+ unsigned int min_len = priv->dma_priv.dma_buswidth ? : align + 1;
for_each_sg(sg, sg_tmp, host->sg_len, i) {
if (sg_tmp->offset & align)
aligned = false;
- if (sg_tmp->length & align) {
+ if (sg_tmp->length % min_len) {
multiple = false;
break;
}
@@ -175,7 +174,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
goto pio;
}
- if (sg->length < TMIO_MMC_MIN_DMA_LEN)
+ if (sg->length < min_len)
return;
/* The only sg element can be unaligned, use our bounce buffer then */
@@ -231,11 +230,12 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
int ret, i;
bool aligned = true, multiple = true;
unsigned int align = (1 << host->pdata->alignment_shift) - 1;
+ unsigned int min_len = priv->dma_priv.dma_buswidth ? : align + 1;
for_each_sg(sg, sg_tmp, host->sg_len, i) {
if (sg_tmp->offset & align)
aligned = false;
- if (sg_tmp->length & align) {
+ if (sg_tmp->length % min_len) {
multiple = false;
break;
}
@@ -247,7 +247,7 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
goto pio;
}
- if (sg->length < TMIO_MMC_MIN_DMA_LEN)
+ if (sg->length < min_len)
return;
/* The only sg element can be unaligned, use our bounce buffer then */
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 1/4] mmc: host: renesas_sdhi_sys_dmac: Use dma_buswidth instead of hardcoded value
2019-11-22 6:13 ` [PATCH RFC 1/4] mmc: host: renesas_sdhi_sys_dmac: Use dma_buswidth instead of hardcoded value Yoshihiro Shimoda
@ 2019-11-26 10:45 ` Ulrich Hecht
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Hecht @ 2019-11-26 10:45 UTC (permalink / raw)
To: Yoshihiro Shimoda, linux-renesas-soc
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
>
>
> This patch uses dma_buswidth instread of hardcoded value of
> TMIO_MMC_MIN_DMA_LEN (8) for increasing the buswidth in the future.
> Note that, since the dma_buswidth is 4 and the align is 2,
> when the sg_tmp->length is 6, it cannot transfer correcly.
> So, this patch changes the conditions.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> index 13ff023..30f34a3 100644
> --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> @@ -23,8 +23,6 @@
> #include "renesas_sdhi.h"
> #include "tmio_mmc.h"
>
> -#define TMIO_MMC_MIN_DMA_LEN 8
> -
> static const struct renesas_sdhi_of_data of_default_cfg = {
> .tmio_flags = TMIO_MMC_HAS_IDLE_WAIT,
> };
> @@ -159,11 +157,12 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
> int ret, i;
> bool aligned = true, multiple = true;
> unsigned int align = (1 << host->pdata->alignment_shift) - 1;
> + unsigned int min_len = priv->dma_priv.dma_buswidth ? : align + 1;
>
> for_each_sg(sg, sg_tmp, host->sg_len, i) {
> if (sg_tmp->offset & align)
> aligned = false;
> - if (sg_tmp->length & align) {
> + if (sg_tmp->length % min_len) {
> multiple = false;
> break;
> }
> @@ -175,7 +174,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
> goto pio;
> }
>
> - if (sg->length < TMIO_MMC_MIN_DMA_LEN)
> + if (sg->length < min_len)
> return;
>
> /* The only sg element can be unaligned, use our bounce buffer then */
> @@ -231,11 +230,12 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
> int ret, i;
> bool aligned = true, multiple = true;
> unsigned int align = (1 << host->pdata->alignment_shift) - 1;
> + unsigned int min_len = priv->dma_priv.dma_buswidth ? : align + 1;
>
> for_each_sg(sg, sg_tmp, host->sg_len, i) {
> if (sg_tmp->offset & align)
> aligned = false;
> - if (sg_tmp->length & align) {
> + if (sg_tmp->length % min_len) {
> multiple = false;
> break;
> }
> @@ -247,7 +247,7 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
> goto pio;
> }
>
> - if (sg->length < TMIO_MMC_MIN_DMA_LEN)
> + if (sg->length < min_len)
> return;
>
> /* The only sg element can be unaligned, use our bounce buffer then */
> --
> 2.7.4
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 2/4] mmc: host: renesas_sdhi_sys_dmac: Do not fall back to PIO
2019-11-22 6:13 [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Yoshihiro Shimoda
2019-11-22 6:13 ` [PATCH RFC 1/4] mmc: host: renesas_sdhi_sys_dmac: Use dma_buswidth instead of hardcoded value Yoshihiro Shimoda
@ 2019-11-22 6:13 ` Yoshihiro Shimoda
2019-11-26 10:45 ` Ulrich Hecht
2019-11-22 6:13 ` [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting Yoshihiro Shimoda
` (2 subsequent siblings)
4 siblings, 1 reply; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-11-22 6:13 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Yoshihiro Shimoda
Falling back to PIO forever is not convenience if a buffer condition
is not match with the hardware once. So, this patch removes it.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++++++-------------------------
1 file changed, 7 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 30f34a3..09137cc 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -175,7 +175,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
}
if (sg->length < min_len)
- return;
+ goto pio;
/* The only sg element can be unaligned, use our bounce buffer then */
if (!aligned) {
@@ -200,23 +200,13 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
ret = cookie;
}
host->dma_on = true;
+ renesas_sdhi_sys_dmac_enable_dma(host, true);
}
pio:
if (!desc) {
- /* DMA failed, fall back to PIO */
renesas_sdhi_sys_dmac_enable_dma(host, false);
- if (ret >= 0)
- ret = -EIO;
- host->chan_rx = NULL;
- dma_release_channel(chan);
- /* Free the Tx channel too */
- chan = host->chan_tx;
- if (chan) {
- host->chan_tx = NULL;
- dma_release_channel(chan);
- }
- dev_warn(&host->pdev->dev,
- "DMA failed: %d, falling back to PIO\n", ret);
+ host->dma_on = false;
+ dev_dbg(&host->pdev->dev, "DMA failed: %d\n", ret);
}
}
@@ -248,7 +238,7 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
}
if (sg->length < min_len)
- return;
+ goto pio;
/* The only sg element can be unaligned, use our bounce buffer then */
if (!aligned) {
@@ -281,20 +271,9 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
}
pio:
if (!desc) {
- /* DMA failed, fall back to PIO */
renesas_sdhi_sys_dmac_enable_dma(host, false);
- if (ret >= 0)
- ret = -EIO;
- host->chan_tx = NULL;
- dma_release_channel(chan);
- /* Free the Rx channel too */
- chan = host->chan_rx;
- if (chan) {
- host->chan_rx = NULL;
- dma_release_channel(chan);
- }
- dev_warn(&host->pdev->dev,
- "DMA failed: %d, falling back to PIO\n", ret);
+ host->dma_on = false;
+ dev_dbg(&host->pdev->dev, "DMA failed: %d\n", ret);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 2/4] mmc: host: renesas_sdhi_sys_dmac: Do not fall back to PIO
2019-11-22 6:13 ` [PATCH RFC 2/4] mmc: host: renesas_sdhi_sys_dmac: Do not fall back to PIO Yoshihiro Shimoda
@ 2019-11-26 10:45 ` Ulrich Hecht
0 siblings, 0 replies; 15+ messages in thread
From: Ulrich Hecht @ 2019-11-26 10:45 UTC (permalink / raw)
To: Yoshihiro Shimoda, linux-renesas-soc
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
>
>
> Falling back to PIO forever is not convenience if a buffer condition
> is not match with the hardware once. So, this patch removes it.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 35 +++++++-------------------------
> 1 file changed, 7 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> index 30f34a3..09137cc 100644
> --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> @@ -175,7 +175,7 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
> }
>
> if (sg->length < min_len)
> - return;
> + goto pio;
>
> /* The only sg element can be unaligned, use our bounce buffer then */
> if (!aligned) {
> @@ -200,23 +200,13 @@ static void renesas_sdhi_sys_dmac_start_dma_rx(struct tmio_mmc_host *host)
> ret = cookie;
> }
> host->dma_on = true;
> + renesas_sdhi_sys_dmac_enable_dma(host, true);
> }
> pio:
> if (!desc) {
> - /* DMA failed, fall back to PIO */
> renesas_sdhi_sys_dmac_enable_dma(host, false);
> - if (ret >= 0)
> - ret = -EIO;
> - host->chan_rx = NULL;
> - dma_release_channel(chan);
> - /* Free the Tx channel too */
> - chan = host->chan_tx;
> - if (chan) {
> - host->chan_tx = NULL;
> - dma_release_channel(chan);
> - }
> - dev_warn(&host->pdev->dev,
> - "DMA failed: %d, falling back to PIO\n", ret);
> + host->dma_on = false;
> + dev_dbg(&host->pdev->dev, "DMA failed: %d\n", ret);
> }
> }
>
> @@ -248,7 +238,7 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
> }
>
> if (sg->length < min_len)
> - return;
> + goto pio;
>
> /* The only sg element can be unaligned, use our bounce buffer then */
> if (!aligned) {
> @@ -281,20 +271,9 @@ static void renesas_sdhi_sys_dmac_start_dma_tx(struct tmio_mmc_host *host)
> }
> pio:
> if (!desc) {
> - /* DMA failed, fall back to PIO */
> renesas_sdhi_sys_dmac_enable_dma(host, false);
> - if (ret >= 0)
> - ret = -EIO;
> - host->chan_tx = NULL;
> - dma_release_channel(chan);
> - /* Free the Rx channel too */
> - chan = host->chan_rx;
> - if (chan) {
> - host->chan_rx = NULL;
> - dma_release_channel(chan);
> - }
> - dev_warn(&host->pdev->dev,
> - "DMA failed: %d, falling back to PIO\n", ret);
> + host->dma_on = false;
> + dev_dbg(&host->pdev->dev, "DMA failed: %d\n", ret);
> }
> }
>
> --
> 2.7.4
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting
2019-11-22 6:13 [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Yoshihiro Shimoda
2019-11-22 6:13 ` [PATCH RFC 1/4] mmc: host: renesas_sdhi_sys_dmac: Use dma_buswidth instead of hardcoded value Yoshihiro Shimoda
2019-11-22 6:13 ` [PATCH RFC 2/4] mmc: host: renesas_sdhi_sys_dmac: Do not fall back to PIO Yoshihiro Shimoda
@ 2019-11-22 6:13 ` Yoshihiro Shimoda
2019-11-26 10:46 ` Ulrich Hecht
2019-11-22 6:13 ` [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte Yoshihiro Shimoda
2019-11-26 10:45 ` [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Ulrich Hecht
4 siblings, 1 reply; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-11-22 6:13 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Yoshihiro Shimoda
This patch adds DMACR setting which needs to use the 32 bytes
transfer mode of SYS-DMAC.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/mmc/host/renesas_sdhi_core.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 234551a..d9a69f6 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/clk.h>
+#include <linux/dmaengine.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/of_device.h>
@@ -46,6 +47,12 @@
#define SDHI_VER_GEN3_SD 0xcc10
#define SDHI_VER_GEN3_SDMMC 0xcd10
+#define DMACR_SDR104 0x192
+#define DMACR_SDR104_32BYTE 0x0004
+#define DMACR_SDR50 0xe4
+#define DMACR_SDR50_32BYTE 0x000a
+#define DMACR_2_OR_4BYTE 0x0000
+
struct renesas_sdhi_quirks {
bool hs400_disabled;
bool hs400_4taps;
@@ -604,6 +611,32 @@ static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
return blk_size;
}
+static void renesas_sdhi_set_dmacr(struct tmio_mmc_host *host)
+{
+ struct renesas_sdhi *priv = host_to_priv(host);
+ u16 val = DMACR_2_OR_4BYTE;
+ u16 reg;
+ enum dma_slave_buswidth width = priv->dma_priv.dma_buswidth;
+
+ switch (sd_ctrl_read16(host, CTL_VERSION)) {
+ case SDHI_VER_GEN2_SDR50:
+ if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
+ val = DMACR_SDR50_32BYTE;
+ reg = DMACR_SDR50;
+ break;
+ case SDHI_VER_GEN2_SDR104:
+ if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
+ val = DMACR_SDR104_32BYTE;
+ reg = DMACR_SDR104;
+ break;
+ default:
+ /* nothing to do */
+ return;
+ }
+
+ sd_ctrl_write16(host, reg, val);
+}
+
static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
{
/* Iff regs are 8 byte apart, sdbuf is 64 bit. Otherwise always 32. */
@@ -611,6 +644,8 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? DMA_ENABLE_DMASDRW : 0);
renesas_sdhi_sdbuf_width(host, enable ? width : 16);
+
+ renesas_sdhi_set_dmacr(host);
}
static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400 = {
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting
2019-11-22 6:13 ` [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting Yoshihiro Shimoda
@ 2019-11-26 10:46 ` Ulrich Hecht
2019-12-02 8:19 ` Yoshihiro Shimoda
0 siblings, 1 reply; 15+ messages in thread
From: Ulrich Hecht @ 2019-11-26 10:46 UTC (permalink / raw)
To: Yoshihiro Shimoda, linux-renesas-soc
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
>
>
> This patch adds DMACR setting which needs to use the 32 bytes
> transfer mode of SYS-DMAC.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/mmc/host/renesas_sdhi_core.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 234551a..d9a69f6 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -20,6 +20,7 @@
>
> #include <linux/kernel.h>
> #include <linux/clk.h>
> +#include <linux/dmaengine.h>
> #include <linux/slab.h>
> #include <linux/module.h>
> #include <linux/of_device.h>
> @@ -46,6 +47,12 @@
> #define SDHI_VER_GEN3_SD 0xcc10
> #define SDHI_VER_GEN3_SDMMC 0xcd10
>
> +#define DMACR_SDR104 0x192
> +#define DMACR_SDR104_32BYTE 0x0004
> +#define DMACR_SDR50 0xe4
> +#define DMACR_SDR50_32BYTE 0x000a
> +#define DMACR_2_OR_4BYTE 0x0000
> +
Could you give me a pointer to where these magic numbers are documented? In my (rather old) SDHI docs the register addresses don't match.
> struct renesas_sdhi_quirks {
> bool hs400_disabled;
> bool hs400_4taps;
> @@ -604,6 +611,32 @@ static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
> return blk_size;
> }
>
> +static void renesas_sdhi_set_dmacr(struct tmio_mmc_host *host)
> +{
> + struct renesas_sdhi *priv = host_to_priv(host);
> + u16 val = DMACR_2_OR_4BYTE;
> + u16 reg;
> + enum dma_slave_buswidth width = priv->dma_priv.dma_buswidth;
> +
> + switch (sd_ctrl_read16(host, CTL_VERSION)) {
> + case SDHI_VER_GEN2_SDR50:
> + if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> + val = DMACR_SDR50_32BYTE;
> + reg = DMACR_SDR50;
> + break;
> + case SDHI_VER_GEN2_SDR104:
> + if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> + val = DMACR_SDR104_32BYTE;
> + reg = DMACR_SDR104;
> + break;
> + default:
> + /* nothing to do */
> + return;
> + }
> +
> + sd_ctrl_write16(host, reg, val);
> +}
> +
> static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
> {
> /* Iff regs are 8 byte apart, sdbuf is 64 bit. Otherwise always 32. */
> @@ -611,6 +644,8 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
>
> sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? DMA_ENABLE_DMASDRW : 0);
> renesas_sdhi_sdbuf_width(host, enable ? width : 16);
> +
> + renesas_sdhi_set_dmacr(host);
> }
>
> static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400 = {
> --
> 2.7.4
>
Assuming that the register addresses are correct,
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting
2019-11-26 10:46 ` Ulrich Hecht
@ 2019-12-02 8:19 ` Yoshihiro Shimoda
0 siblings, 0 replies; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-12-02 8:19 UTC (permalink / raw)
To: Ulrich Hecht; +Cc: linux-renesas-soc
Hi Ulrich-san,
Thank you for your review!
> From: Ulrich Hecht, Sent: Tuesday, November 26, 2019 7:46 PM
<snip>
> > +#define DMACR_SDR104 0x192
> > +#define DMACR_SDR104_32BYTE 0x0004
> > +#define DMACR_SDR50 0xe4
> > +#define DMACR_SDR50_32BYTE 0x000a
> > +#define DMACR_2_OR_4BYTE 0x0000
> > +
>
> Could you give me a pointer to where these magic numbers are documented? In my (rather old) SDHI docs the register addresses
> don't match.
R-Car Gen2 has 2 types of SDHI controller from renesas_sdhi_core.c:
---
#define SDHI_VER_GEN2_SDR50 0x490c
<snip>
#define SDHI_VER_GEN2_SDR104 0xcb0d
---
- SDR50/104 supported controller's bus_shift will be 1, so that DMACR_SDR104 is 0x192 (actual address is 0x324).
- SDR50 supported controller's bus_shift will be 0, so that DMACR_SDR50 is 0xe6 (actual address is 0xe6).
So, I should fix the "#define DMACR_SDR50" value to 0xe6, not 0xe4 :)
Best regards,
Yoshihiro Shimoda
> > struct renesas_sdhi_quirks {
> > bool hs400_disabled;
> > bool hs400_4taps;
> > @@ -604,6 +611,32 @@ static int renesas_sdhi_multi_io_quirk(struct mmc_card *card,
> > return blk_size;
> > }
> >
> > +static void renesas_sdhi_set_dmacr(struct tmio_mmc_host *host)
> > +{
> > + struct renesas_sdhi *priv = host_to_priv(host);
> > + u16 val = DMACR_2_OR_4BYTE;
> > + u16 reg;
> > + enum dma_slave_buswidth width = priv->dma_priv.dma_buswidth;
> > +
> > + switch (sd_ctrl_read16(host, CTL_VERSION)) {
> > + case SDHI_VER_GEN2_SDR50:
> > + if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> > + val = DMACR_SDR50_32BYTE;
> > + reg = DMACR_SDR50;
> > + break;
> > + case SDHI_VER_GEN2_SDR104:
> > + if (width == DMA_SLAVE_BUSWIDTH_32_BYTES)
> > + val = DMACR_SDR104_32BYTE;
> > + reg = DMACR_SDR104;
> > + break;
> > + default:
> > + /* nothing to do */
> > + return;
> > + }
> > +
> > + sd_ctrl_write16(host, reg, val);
> > +}
> > +
> > static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
> > {
> > /* Iff regs are 8 byte apart, sdbuf is 64 bit. Otherwise always 32. */
> > @@ -611,6 +644,8 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
> >
> > sd_ctrl_write16(host, CTL_DMA_ENABLE, enable ? DMA_ENABLE_DMASDRW : 0);
> > renesas_sdhi_sdbuf_width(host, enable ? width : 16);
> > +
> > + renesas_sdhi_set_dmacr(host);
> > }
> >
> > static const struct renesas_sdhi_quirks sdhi_quirks_4tap_nohs400 = {
> > --
> > 2.7.4
> >
>
> Assuming that the register addresses are correct,
> Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
>
> CU
> Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte
2019-11-22 6:13 [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Yoshihiro Shimoda
` (2 preceding siblings ...)
2019-11-22 6:13 ` [PATCH RFC 3/4] mmc: host: renesas_sdhi_sys_dmac: add DMACR setting Yoshihiro Shimoda
@ 2019-11-22 6:13 ` Yoshihiro Shimoda
2019-11-26 10:46 ` Ulrich Hecht
2019-11-28 21:07 ` Wolfram Sang
2019-11-26 10:45 ` [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Ulrich Hecht
4 siblings, 2 replies; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-11-22 6:13 UTC (permalink / raw)
To: linux-renesas-soc; +Cc: Yoshihiro Shimoda
To improve performance, this patch sets dma_buswidth value to 32
when transfer size is multiples of 32. In other words, a sd card
transfer's size if not multiples of 32, this driver uses PIO
and then the performance will be down.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
index 09137cc..65e71b6 100644
--- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
@@ -58,7 +58,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen2_compatible = {
.capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
MMC_CAP_CMD23,
.capabilities2 = MMC_CAP2_NO_WRITE_PROTECT,
- .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES,
+ .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES,
.dma_rx_offset = 0x2000,
.scc_offset = 0x0300,
.taps = rcar_gen2_scc_taps,
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte
2019-11-22 6:13 ` [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte Yoshihiro Shimoda
@ 2019-11-26 10:46 ` Ulrich Hecht
2019-11-28 21:07 ` Wolfram Sang
1 sibling, 0 replies; 15+ messages in thread
From: Ulrich Hecht @ 2019-11-26 10:46 UTC (permalink / raw)
To: Yoshihiro Shimoda, linux-renesas-soc
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
>
>
> To improve performance, this patch sets dma_buswidth value to 32
> when transfer size is multiples of 32. In other words, a sd card
> transfer's size if not multiples of 32, this driver uses PIO
> and then the performance will be down.
>
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
> drivers/mmc/host/renesas_sdhi_sys_dmac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/renesas_sdhi_sys_dmac.c b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> index 09137cc..65e71b6 100644
> --- a/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_sys_dmac.c
> @@ -58,7 +58,7 @@ static const struct renesas_sdhi_of_data of_rcar_gen2_compatible = {
> .capabilities = MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
> MMC_CAP_CMD23,
> .capabilities2 = MMC_CAP2_NO_WRITE_PROTECT,
> - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES,
> + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES,
> .dma_rx_offset = 0x2000,
> .scc_offset = 0x0300,
> .taps = rcar_gen2_scc_taps,
> --
> 2.7.4
>
Reviewed-by: Ulrich Hecht <uli+renesas@fpond.eu>
CU
Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte
2019-11-22 6:13 ` [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte Yoshihiro Shimoda
2019-11-26 10:46 ` Ulrich Hecht
@ 2019-11-28 21:07 ` Wolfram Sang
2019-12-02 8:38 ` Yoshihiro Shimoda
1 sibling, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2019-11-28 21:07 UTC (permalink / raw)
To: Yoshihiro Shimoda; +Cc: linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
Hi Shimoda-san,
Interesting series!
> - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES,
> + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES,
Two very high level questions:
1) can't we set dma_priv->dma_buswidth at runtime when we know what the
card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or
DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO.
AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even
remove it from of_data entirely?
2) Just by grepping in mmc/hosts, I see that no driver uses
DMA_SLAVE_BUSWIDTH_32_BYTES. Do you have an idea why? Because it is the
convenient setting which works for all cards?
Thanks and kind regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte
2019-11-28 21:07 ` Wolfram Sang
@ 2019-12-02 8:38 ` Yoshihiro Shimoda
2019-12-02 8:54 ` Wolfram Sang
0 siblings, 1 reply; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-12-02 8:38 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-renesas-soc
Hi Wolfram-san,
Thank you for your comments!
> From: Wolfram Sang, Sent: Friday, November 29, 2019 6:07 AM
>
> Hi Shimoda-san,
>
> Interesting series!
>
> > - .dma_buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES,
> > + .dma_buswidth = DMA_SLAVE_BUSWIDTH_32_BYTES,
>
> Two very high level questions:
>
> 1) can't we set dma_priv->dma_buswidth at runtime when we know what the
> card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or
> DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO.
> AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even
> remove it from of_data entirely?
As I replied to Ulrich-san on other email thread, for now, rcar-dmac has a limitation
on dmaengine_slave_config(), we should not call it at runtime. But, I don't think
any sd card have such a limitation. In other words, if rcar-dmac doesn't have
the limitation, I think we can change the buswidth at runtime and then we can
remove the .dma_buswidth from of_data.
> 2) Just by grepping in mmc/hosts, I see that no driver uses
> DMA_SLAVE_BUSWIDTH_32_BYTES. Do you have an idea why? Because it is the
> convenient setting which works for all cards?
I also grepped in drivers/dma, and all dmaengine drivers except Renesas related
SoCs don't support DMA_SLAVE_BUSWIDTH_32_BYTES. So, I think no driver uses
the 32 bytes on mmc/hosts :)
Best regards,
Yoshihiro Shimoda
> Thanks and kind regards,
>
> Wolfram
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte
2019-12-02 8:38 ` Yoshihiro Shimoda
@ 2019-12-02 8:54 ` Wolfram Sang
0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2019-12-02 8:54 UTC (permalink / raw)
To: Yoshihiro Shimoda; +Cc: linux-renesas-soc
[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]
Hi Shimoda-san,
> > 1) can't we set dma_priv->dma_buswidth at runtime when we know what the
> > card is capable of? Either DMA_SLAVE_BUSWIDTH_32_BYTES or
> > DMA_SLAVE_BUSWIDTH_4_BYTES? Then we don't need to fallback to PIO.
> > AFAIS, we only Gen2 sets .dma_buswidth in of_data, so we could even
> > remove it from of_data entirely?
>
> As I replied to Ulrich-san on other email thread, for now, rcar-dmac has a limitation
> on dmaengine_slave_config(), we should not call it at runtime. But, I don't think
> any sd card have such a limitation. In other words, if rcar-dmac doesn't have
> the limitation, I think we can change the buswidth at runtime and then we can
> remove the .dma_buswidth from of_data.
So, that I understand correctly: The DMAC limitation is because of the
driver and not because of the HW? If so, is it hard/planned to be fixed?
> I also grepped in drivers/dma, and all dmaengine drivers except Renesas related
> SoCs don't support DMA_SLAVE_BUSWIDTH_32_BYTES. So, I think no driver uses
> the 32 bytes on mmc/hosts :)
Wow, we are bleeding edge with this? :)
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth
2019-11-22 6:13 [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Yoshihiro Shimoda
` (3 preceding siblings ...)
2019-11-22 6:13 ` [PATCH RFC 4/4] mmc: host: renesas_sdhi_sys_dmac: Set dma_buswidth value to 32 byte Yoshihiro Shimoda
@ 2019-11-26 10:45 ` Ulrich Hecht
2019-12-02 8:09 ` Yoshihiro Shimoda
4 siblings, 1 reply; 15+ messages in thread
From: Ulrich Hecht @ 2019-11-26 10:45 UTC (permalink / raw)
To: Yoshihiro Shimoda, linux-renesas-soc
> On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
>
>
> This patch can improve performance when a sd card transfer size
> is multiples of 32 like a sd memory card. However, this may
> disimprove performance when a sd card transfer size is not
> multiples of 32 because this patch uses PIO instead of SYS-DMAC.
I have logged the DMA transfer sizes of a Marvell SD8897 WiFi/BT chip, and there transfers that are not multiples of 32 are infrequent (every second or so) and small (184 bytes); all large transfers are multiples of 32.
Would it be practical to switch the DMA bus width down if odd sizes are encountered, and back up again otherwise?
CU
Uli
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth
2019-11-26 10:45 ` [PATCH RFC 0/4] mmc: host: renesas_sdhi_sys_dmac: change dma_buswidth Ulrich Hecht
@ 2019-12-02 8:09 ` Yoshihiro Shimoda
0 siblings, 0 replies; 15+ messages in thread
From: Yoshihiro Shimoda @ 2019-12-02 8:09 UTC (permalink / raw)
To: Ulrich Hecht; +Cc: linux-renesas-soc
Hi Ulrich-san,
Thank you for your comment!
> From: Ulrich Hecht, Sent: Tuesday, November 26, 2019 7:45 PM
>
> > On November 22, 2019 at 7:13 AM Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> wrote:
> >
> >
> > This patch can improve performance when a sd card transfer size
> > is multiples of 32 like a sd memory card. However, this may
> > disimprove performance when a sd card transfer size is not
> > multiples of 32 because this patch uses PIO instead of SYS-DMAC.
>
> I have logged the DMA transfer sizes of a Marvell SD8897 WiFi/BT chip, and there transfers that are not multiples of 32
> are infrequent (every second or so) and small (184 bytes); all large transfers are multiples of 32.
>
> Would it be practical to switch the DMA bus width down if odd sizes are encountered, and back up again otherwise?
I don't know why, but according to drivers/dma/sh/rcar-dmac.c, we should not configure the channel while using it.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/dma/sh/rcar-dmac.c?h=v5.4#n1245
The comment has been described on the first commit. But, since other drivers don't mention such a limitation,
I think we can remove the comments somehow.
Best regards,
Yoshihiro Shimoda
> CU
> Uli
^ permalink raw reply [flat|nested] 15+ messages in thread