[U-Boot] [U-Boot, 3/5] mmc: omap_hsmmc: Add support for DMA (ADMA2)
Jean-Jacques Hiblot
jjhiblot at ti.com
Fri Jul 28 15:40:48 UTC 2017
On 28/07/2017 14:46, Jaehoon Chung wrote:
> Hi JJ
>
> On 07/12/2017 01:20 AM, Jean-Jacques Hiblot wrote:
>> From: Kishon Vijay Abraham I <kishon at ti.com>
>>
>> The omap hsmmc host controller can have the ADMA2 feature. It brings better
>> read and write throughput.
>> On most SOC, the capability is read from the hl_hwinfo register. On OMAP3,
>> DMA support is compiled out.
>>
>> Signed-off-by: Kishon Vijay Abraham I <kishon at ti.com>
>> Signed-off-by: Jean-Jacques Hiblot <jjhiblot at ti.com>
> Sorry..i added the some comment at below.
>
>> ---
>> arch/arm/include/asm/omap_mmc.h | 12 ++-
>> drivers/mmc/omap_hsmmc.c | 186 +++++++++++++++++++++++++++++++++++++++-
>> 2 files changed, 195 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/omap_mmc.h b/arch/arm/include/asm/omap_mmc.h
>> index 2b489a4..206badb 100644
>> --- a/arch/arm/include/asm/omap_mmc.h
>> +++ b/arch/arm/include/asm/omap_mmc.h
>> @@ -29,7 +29,10 @@
>>
>> struct hsmmc {
>> #ifndef CONFIG_OMAP34XX
>> - unsigned char res0[0x100];
>> + unsigned int hl_rev;
>> + unsigned int hl_hwinfo;
>> + unsigned int hl_sysconfig;
>> + unsigned char res0[0xf4];
>> #endif
>> unsigned char res1[0x10];
>> unsigned int sysconfig; /* 0x10 */
>> @@ -52,6 +55,9 @@ struct hsmmc {
>> unsigned int ie; /* 0x134 */
>> unsigned char res4[0x8];
>> unsigned int capa; /* 0x140 */
>> + unsigned char res5[0x10];
>> + unsigned int admaes; /* 0x154 */
>> + unsigned int admasal; /* 0x158 */
>> };
>>
>> struct omap_hsmmc_plat {
>> @@ -64,6 +70,7 @@ struct omap_hsmmc_plat {
>> /*
>> * OMAP HS MMC Bit definitions
>> */
>> +#define MADMA_EN (0x1 << 0)
>> #define MMC_SOFTRESET (0x1 << 1)
>> #define RESETDONE (0x1 << 0)
>> #define NOOPENDRAIN (0x0 << 0)
>> @@ -80,6 +87,7 @@ struct omap_hsmmc_plat {
>> #define WPP_ACTIVEHIGH (0x0 << 8)
>> #define RESERVED_MASK (0x3 << 9)
>> #define CTPL_MMC_SD (0x0 << 11)
>> +#define DMA_MASTER (0x1 << 20)
>> #define BLEN_512BYTESLEN (0x200 << 0)
>> #define NBLK_STPCNT (0x0 << 16)
>> #define DE_DISABLE (0x0 << 0)
>> @@ -119,6 +127,7 @@ struct omap_hsmmc_plat {
>> #define SDBP_PWRON (0x1 << 8)
>> #define SDVS_1V8 (0x5 << 9)
>> #define SDVS_3V0 (0x6 << 9)
>> +#define DMA_SELECT (0x2 << 3)
>> #define ICE_MASK (0x1 << 0)
>> #define ICE_STOP (0x0 << 0)
>> #define ICS_MASK (0x1 << 1)
>> @@ -148,6 +157,7 @@ struct omap_hsmmc_plat {
>> #define IE_DTO (0x01 << 20)
>> #define IE_DCRC (0x01 << 21)
>> #define IE_DEB (0x01 << 22)
>> +#define IE_ADMAE (0x01 << 25)
>> #define IE_CERR (0x01 << 28)
>> #define IE_BADA (0x01 << 29)
>>
>> diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c
>> index 2c1e429..3419dc5 100644
>> --- a/drivers/mmc/omap_hsmmc.c
>> +++ b/drivers/mmc/omap_hsmmc.c
>> @@ -25,6 +25,7 @@
>> #include <config.h>
>> #include <common.h>
>> #include <malloc.h>
>> +#include <memalign.h>
>> #include <mmc.h>
>> #include <part.h>
>> #include <i2c.h>
>> @@ -71,10 +72,37 @@ struct omap_hsmmc_data {
>> int wp_gpio;
>> #endif
>> #endif
>> + u8 controller_flags;
>> +#ifndef CONFIG_OMAP34XX
>> + struct omap_hsmmc_adma_desc *adma_desc_table;
>> + uint desc_slot;
>> +#endif
>> +};
>> +
>> +#ifndef CONFIG_OMAP34XX
>> +struct omap_hsmmc_adma_desc {
>> + u8 attr;
>> + u8 reserved;
>> + u16 len;
>> + u32 addr;
>> };
>>
>> +#define ADMA_MAX_LEN 63488
>> +
>> +/* Decriptor table defines */
>> +#define ADMA_DESC_ATTR_VALID BIT(0)
>> +#define ADMA_DESC_ATTR_END BIT(1)
>> +#define ADMA_DESC_ATTR_INT BIT(2)
>> +#define ADMA_DESC_ATTR_ACT1 BIT(4)
>> +#define ADMA_DESC_ATTR_ACT2 BIT(5)
>> +
>> +#define ADMA_DESC_TRANSFER_DATA ADMA_DESC_ATTR_ACT2
>> +#define ADMA_DESC_LINK_DESC (ADMA_DESC_ATTR_ACT1 | ADMA_DESC_ATTR_ACT2)
>> +#endif
>> +
>> /* If we fail after 1 second wait, something is really bad */
>> #define MAX_RETRY_MS 1000
>> +#define OMAP_HSMMC_USE_ADMA BIT(2)
>>
>> static int mmc_read_data(struct hsmmc *mmc_base, char *buf, unsigned int size);
>> static int mmc_write_data(struct hsmmc *mmc_base, const char *buf,
>> @@ -242,6 +270,11 @@ static int omap_hsmmc_init_setup(struct mmc *mmc)
>> return -ETIMEDOUT;
>> }
>> }
>> +#ifndef CONFIG_OMAP34XX
>> + reg_val = readl(&mmc_base->hl_hwinfo);
>> + if (reg_val & MADMA_EN)
>> + priv->controller_flags |= OMAP_HSMMC_USE_ADMA;
>> +#endif
>> writel(DTW_1_BITMODE | SDBP_PWROFF | SDVS_3V0, &mmc_base->hctl);
>> writel(readl(&mmc_base->capa) | VS30_3V0SUP | VS18_1V8SUP,
>> &mmc_base->capa);
>> @@ -269,8 +302,8 @@ static int omap_hsmmc_init_setup(struct mmc *mmc)
>> writel(readl(&mmc_base->hctl) | SDBP_PWRON, &mmc_base->hctl);
>>
>> writel(IE_BADA | IE_CERR | IE_DEB | IE_DCRC | IE_DTO | IE_CIE |
>> - IE_CEB | IE_CCRC | IE_CTO | IE_BRR | IE_BWR | IE_TC | IE_CC,
>> - &mmc_base->ie);
>> + IE_CEB | IE_CCRC | IE_ADMAE | IE_CTO | IE_BRR | IE_BWR | IE_TC |
>> + IE_CC, &mmc_base->ie);
>>
>> mmc_init_stream(mmc_base);
>>
>> @@ -322,6 +355,123 @@ static void mmc_reset_controller_fsm(struct hsmmc *mmc_base, u32 bit)
>> }
>> }
>> }
>> +
>> +#ifndef CONFIG_OMAP34XX
>> +static int omap_hsmmc_adma_desc(struct mmc *mmc, char *buf, u16 len, bool end)
> Can be void..?
yes indeed. I'll fix that.
>
>> +{
>> + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc);
>> + struct omap_hsmmc_adma_desc *desc;
>> + u8 attr;
>> +
>> + desc = &priv->adma_desc_table[priv->desc_slot];
>> +
>> + attr = ADMA_DESC_ATTR_VALID | ADMA_DESC_TRANSFER_DATA;
>> + if (!end)
>> + priv->desc_slot++;
>> + else
>> + attr |= ADMA_DESC_ATTR_END;
>> +
>> + desc->len = len;
>> + desc->addr = (u32)buf;
>> + desc->reserved = 0;
>> + desc->attr = attr;
>> +
>> + return 0;
>> +}
>> +
>> +static int omap_hsmmc_prepare_adma_table(struct mmc *mmc, struct mmc_data *data)
> Ditto.
>
>> +{
>> + uint total_len = data->blocksize * data->blocks;
>> + uint desc_count = DIV_ROUND_UP(total_len, ADMA_MAX_LEN);
>> + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc);
>> + int i = desc_count;
>> + char *buf;
>> +
>> + priv->desc_slot = 0;
>> + priv->adma_desc_table = (struct omap_hsmmc_adma_desc *)
>> + memalign(ARCH_DMA_MINALIGN, desc_count *
>> + sizeof(struct omap_hsmmc_adma_desc));
>> +
>> + if (data->flags & MMC_DATA_READ)
>> + buf = data->dest;
>> + else
>> + buf = (char *)data->src;
>> +
>> + while (--i) {
>> + omap_hsmmc_adma_desc(mmc, buf, ADMA_MAX_LEN, false);
>> + buf += ADMA_MAX_LEN;
>> + total_len -= ADMA_MAX_LEN;
>> + }
>> +
>> + omap_hsmmc_adma_desc(mmc, buf, total_len, true);
>> +
>> + flush_dcache_range((long)priv->adma_desc_table,
>> + (long)priv->adma_desc_table +
>> + ROUND(desc_count *
>> + sizeof(struct omap_hsmmc_adma_desc),
>> + ARCH_DMA_MINALIGN));
>> + return 0;
>> +}
>> +
>> +static void omap_hsmmc_prepare_data(struct mmc *mmc, struct mmc_data *data)
>> +{
>> + struct hsmmc *mmc_base;
>> + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc);
>> + u32 val;
>> + char *buf;
>> +
>> + mmc_base = priv->base_addr;
>> + omap_hsmmc_prepare_adma_table(mmc, data);
>> +
>> + if (data->flags & MMC_DATA_READ)
>> + buf = data->dest;
>> + else
>> + buf = (char *)data->src;
>> +
>> + val = readl(&mmc_base->hctl);
>> + val |= DMA_SELECT;
>> + writel(val, &mmc_base->hctl);
>> +
>> + val = readl(&mmc_base->con);
>> + val |= DMA_MASTER;
>> + writel(val, &mmc_base->con);
>> +
>> + writel((u32)priv->adma_desc_table, &mmc_base->admasal);
>> +
>> + /* TODO: This shouldn't be required for read. However I don't seem
>> + * to get valid data without this.
> TODO? not FIXME?
Actually the comment should be removed. It's required for read as well
(at least the cache invalidation part is required).
I'll remove the comment in v2
>
>> + */
>> + flush_dcache_range((u32)buf,
>> + (u32)buf +
>> + ROUND(data->blocksize * data->blocks,
>> + ARCH_DMA_MINALIGN));
>> +}
>> +
>> +static void omap_hsmmc_dma_cleanup(struct mmc *mmc)
>> +{
>> + struct hsmmc *mmc_base;
>> + struct omap_hsmmc_data *priv = omap_hsmmc_get_data(mmc);
>> + u32 val;
>> +
>> + mmc_base = priv->base_addr;
>> +
>> + val = readl(&mmc_base->con);
>> + val &= ~DMA_MASTER;
>> + writel(val, &mmc_base->con);
>> +
>> + val = readl(&mmc_base->hctl);
>> + val &= ~DMA_SELECT;
>> + writel(val, &mmc_base->hctl);
>> +
>> + kfree(priv->adma_desc_table);
>> +}
>> +#else
>> +#define omap_hsmmc_adma_desc
>> +#define omap_hsmmc_prepare_adma_table
>> +#define omap_hsmmc_prepare_data
>> +#define omap_hsmmc_dma_cleanup
>> +#endif
>> +
>> #ifndef CONFIG_DM_MMC
>> static int omap_hsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>> struct mmc_data *data)
>> @@ -332,6 +482,8 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> struct mmc_data *data)
>> {
>> struct omap_hsmmc_data *priv = dev_get_priv(dev);
>> + struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> + struct mmc *mmc = upriv->mmc;
>> #endif
>> struct hsmmc *mmc_base;
>> unsigned int flags, mmc_stat;
>> @@ -405,6 +557,14 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> flags |= (DP_DATA | DDIR_READ);
>> else
>> flags |= (DP_DATA | DDIR_WRITE);
>> +
>> +#ifndef CONFIG_OMAP34XX
>> + if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) &&
>> + !mmc_is_tuning_cmd(cmd->cmdidx)) {
>> + omap_hsmmc_prepare_data(mmc, data);
>> + flags |= DE_ENABLE;
>> + }
>> +#endif
>> }
>>
>> writel(cmd->cmdarg, &mmc_base->arg);
>> @@ -441,6 +601,28 @@ static int omap_hsmmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
>> }
>> }
>>
>> +#ifndef CONFIG_OMAP34XX
>> + if ((priv->controller_flags & OMAP_HSMMC_USE_ADMA) && data &&
>> + !mmc_is_tuning_cmd(cmd->cmdidx)) {
>> + if (mmc_stat & IE_ADMAE) {
>> + omap_hsmmc_dma_cleanup(mmc);
>> + return -1;
> Use the valid error number.
>
>> + }
>> +
>> + do {
>> + mmc_stat = readl(&mmc_base->stat);
>> + if (mmc_stat & TC_MASK) {
>> + writel(readl(&mmc_base->stat) | TC_MASK,
>> + &mmc_base->stat);
>> + break;
>> + }
>> + } while (1);
> Can you add the timeout? There is a potential infinte loop.
ok
Thanks for the review
JJ
>
>> +
>> + omap_hsmmc_dma_cleanup(mmc);
>> + return 0;
>> + }
>> +#endif
>> +
>> if (data && (data->flags & MMC_DATA_READ)) {
>> mmc_read_data(mmc_base, data->dest,
>> data->blocksize * data->blocks);
>>
>
More information about the U-Boot
mailing list