[PATCH V2 1/2] mmc: add OpenPiton mmc support

Tianrui Wei tianrui-wei at outlook.com
Thu Apr 22 12:34:46 CEST 2021


Dear Jaehoon,

Many thanks again for reviewing our code. I’ll reformat and refactor the patch as you have kindly suggested. We really appreciate your enthusiasm in this matter.

Best Regards,
Tianrui

________________________________________
From: Jaehoon Chung <jh80.chung at samsung.com>
Sent: Thursday, April 22, 2021 2:55 PM
To: Tianrui Wei; u-boot at lists.denx.de
Cc: ycliang at andestech.com; rick at andestech.com; peng.fan at nxp.com; jbalkind at ucsb.edu
Subject: Re: [PATCH V2 1/2] mmc: add OpenPiton mmc support

Dear Tianrui,

On 4/22/21 3:19 PM, Tianrui Wei wrote:
> This patch adds mmc support for OpenPiton.
>
> Signed-off-by: Tianrui Wei <tianrui-wei at outlook.com>
> Signed-off-by: Jonathan Balkind <jbalkind at ucsb.edu>
>
> ---
>
>  drivers/mmc/Kconfig                     |   6 +
>  drivers/mmc/Makefile                    |   1 +
>  drivers/mmc/piton_mmc.c                 | 177 +++++
>  3 files changed, 184 insertions(+)
>  create mode 100644 drivers/mmc/piton_mmc.c
>
> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> index 14d79139..41822c39 100644
> --- a/drivers/mmc/Kconfig
> +++ b/drivers/mmc/Kconfig
> @@ -707,6 +707,12 @@ config MMC_SUNXI_HAS_MODE_SWITCH
>       bool
>       depends on MMC_SUNXI
>
> +config MMC_PITON
> +     bool "MMC support for openpiton SoC"
> +  depends on DM_MMC && BLK
> +     help
> +    This driver enables sd card support in U-Boot port for openpiton
> +
>  config GENERIC_ATMEL_MCI
>       bool "Atmel Multimedia Card Interface support"
>       depends on DM_MMC && BLK && ARCH_AT91
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 1c849cba..698dfe05 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -71,6 +71,7 @@ obj-$(CONFIG_MMC_SDHCI_XENON)               += xenon_sdhci.o
>  obj-$(CONFIG_MMC_SDHCI_ZYNQ)         += zynq_sdhci.o
>
>  obj-$(CONFIG_MMC_SUNXI)                      += sunxi_mmc.o
> +obj-$(CONFIG_MMC_PITON)      += piton_mmc.o
>  obj-$(CONFIG_MMC_UNIPHIER)           += tmio-common.o uniphier-sd.o
>  obj-$(CONFIG_RENESAS_SDHI)           += tmio-common.o renesas-sdhi.o
>  obj-$(CONFIG_MMC_BCM2835)            += bcm2835_sdhost.o
> diff --git a/drivers/mmc/piton_mmc.c b/drivers/mmc/piton_mmc.c
> new file mode 100644
> index 00000000..f2ceb87f
> --- /dev/null
> +++ b/drivers/mmc/piton_mmc.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2009 SAMSUNG Electronics
> + * Minkyu Kang <mk7.kang at samsung.com>
> + * Jaehoon Chung <jh80.chung at samsung.com>
> + * Portions Copyright 2011-2019 NVIDIA Corporation
> + * Portions Copyright 2021 Tianrui Wei
> + * Tianrui Wei <tianrui-wei at outlook.com>
> + */
> +
> +#include <asm/gpio.h>
> +#include <asm/io.h>
> +#include <common.h>
> +#include <dm.h>
> +#include <errno.h>
> +#include <linux/bitops.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <log.h>
> +#include <div64.h>
> +#include <mmc.h>
> +
> +struct piton_mmc_plat {
> +     struct mmc_config cfg;
> +     struct mmc mmc;
> +};
> +
> +struct piton_mmc_priv {
> +     u64 piton_mmc_base_addr; /* peripheral id */
> +};
> +
> +/*
> + * see mmc_read_blocks to see how it is used.
> + * start block is hidden at cmd->arg
> + * also, initialize the block size at init
> + */
> +static int piton_mmc_send_cmd(struct udevice *dev, struct mmc_cmd *cmd,
> +                                                                                             struct mmc_data *data)
> +{
> +     // check first if this is a pure command

Remove all "//" comment.
If you want to add some comment, add comment with "/* *"

e.g)
/* Check first if this is a pure command */

> +     if (data == NULL) {
> +             return 0;
> +     }
> +
> +     // byte count counts all the bytes required for this command

Not need. Remove, plz.

> +     u64 byte_cnt = data->blocks * data->blocksize;
> +     // get which block in mmc card to start from

Ditto

> +     u64 start_block = cmd->cmdarg;
> +     // buff points to the address we store the data stored at mmc card

Ditto.

> +     unsigned *buff = (unsigned int *) data->dest;
> +
> +     struct piton_mmc_priv *priv = dev_get_priv(dev);
> +     // start address denotes the absolute address where the transmission start

Ditto.

> +     u64 start_addr = priv->piton_mmc_base_addr + (start_block);
> +
> +     /* if there is a read */
> +     if (data->flags & MMC_DATA_READ) {
> +             for (u64 i = 0; i < byte_cnt; i += 4) {
> +                     *(buff) = readl((void *)(start_addr + i));
> +                     buff++;
> +             }
> +     } else {
> +             /* else there is a write
> +              * we don't handle write, so error right away
> +              */
> +             return -ENODEV;
> +     }
> +
> +     return 0;
> +}
> +
> +static int piton_mmc_ofdata_to_platdata(struct udevice *dev)
> +{
> +     struct piton_mmc_priv *priv = dev_get_priv(dev);
> +     struct piton_mmc_plat *plat = dev_get_platdata(dev);
> +     struct mmc_config *cfg;
> +     struct mmc *mmc;
> +     /* fill in device description */
> +     struct blk_desc *bdesc;
> +
> +     priv->piton_mmc_base_addr = dev_read_addr(dev);
> +     cfg = &plat->cfg;
> +     cfg->name = "PITON MMC";
> +     cfg->host_caps = MMC_MODE_8BIT;

If you can use mmc_of_parse() with dt, it can be removed.

> +     cfg->f_max = 100000;
> +     cfg->f_min = 400000;
> +     cfg->voltages = MMC_VDD_21_22;
> +
> +     mmc = &plat->mmc;
> +     mmc->read_bl_len = MMC_MAX_BLOCK_LEN;
> +     mmc->capacity_user = 0x100000000;
> +     mmc->capacity_user *= mmc->read_bl_len;
> +     mmc->capacity_boot = 0;
> +     mmc->capacity_rpmb = 0;
> +     for (int i = 0; i < 4; i++)
> +             mmc->capacity_gp[i] = 0;
> +     mmc->capacity = 0x2000000000ULL;
> +     mmc->has_init = 1;
> +
> +     bdesc = mmc_get_blk_desc(mmc);
> +     bdesc->lun = 0;
> +     bdesc->hwpart = 0;
> +     bdesc->type = 0;
> +     bdesc->blksz = mmc->read_bl_len;
> +     bdesc->log2blksz = LOG2(bdesc->blksz);
> +     bdesc->lba = lldiv(mmc->capacity, mmc->read_bl_len);
> +
> +     return 0;
> +}
> +
> +/*
> + * currently, this is ignored. we only use fixed speed
> + */
> +static int piton_mmc_set_ios(struct udevice *dev) { return 0; }

Remove this, plz. If you nened to add some specific things for OpenPiton, can be added at that time.

> +
> +/* test if piton has the micro mmc card present
> + * always return 1, which means present
> + */
> +static int piton_mmc_getcd(struct udevice *dev) { return 1; }

static int piton_mmc_getcd(struct udevice *dev) {
        /*
         * Always return 1,...
         */
        return 1;
}

Or use non-removable in device-tree.


> +
> +/* dummy function, piton_mmc don't need initialization in hw*/
> +static const struct dm_mmc_ops piton_mmc_ops = {
> +     // send a command to mmc device

Remove, plz. It's generic callback, not need to describe what it is.

> +     .send_cmd = piton_mmc_send_cmd,
> +     // set iospeed

Ditto.

> +     .set_ios = piton_mmc_set_ios,
> +     // detect if card is present

Ditto.

> +     .get_cd = piton_mmc_getcd,
> +};
> +
> +static int piton_mmc_probe(struct udevice *dev)
> +{
> +     struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +     struct piton_mmc_plat *plat = dev_get_platdata(dev);
> +     struct mmc_config *cfg = &plat->cfg;
> +
> +     cfg->name = dev->name;
> +     upriv->mmc = &plat->mmc;
> +     upriv->mmc->has_init = 1;
> +     upriv->mmc->capacity = 0x2000000000ULL;
> +     upriv->mmc->read_bl_len = MMC_MAX_BLOCK_LEN;
> +
> +     return 0;
> +}
> +
> +static int piton_mmc_bind(struct udevice *dev)
> +{
> +     struct piton_mmc_plat *plat = dev_get_platdata(dev);
> +     struct mmc_config *cfg = &plat->cfg;
> +
> +     cfg->name = dev->name;
> +     cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS | MMC_MODE_8BIT;
> +     cfg->voltages = MMC_VDD_165_195 | MMC_VDD_32_33 | MMC_VDD_33_34;
> +     cfg->f_min = 1000000;
> +     cfg->f_max = 52000000;
> +     cfg->b_max = U32_MAX;
> +
> +     return mmc_bind(dev, &plat->mmc, cfg);
> +}
> +
> +static const struct udevice_id piton_mmc_ids[] = {
> +             {.compatible = "openpiton,piton-mmc"},
> +             { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(piton_mmc_drv) = {
> +     .name = "piton_mmc",
> +     .id = UCLASS_MMC,
> +     .of_match = piton_mmc_ids,
> +     .ofdata_to_platdata = piton_mmc_ofdata_to_platdata,
> +     .bind = piton_mmc_bind,
> +     .probe = piton_mmc_probe,
> +     .ops = &piton_mmc_ops,
> +     .platdata_auto_alloc_size = sizeof(struct piton_mmc_plat),
> +     .priv_auto_alloc_size = sizeof(struct piton_mmc_priv),
> +};
>



More information about the U-Boot mailing list