[PATCH V4 1/2] mmc: add OpenPiton mmc support
Sean Anderson
seanga2 at gmail.com
Fri May 7 04:59:06 CEST 2021
On 5/6/21 10:57 PM, Tianrui Wei wrote:
> Hi Sean,
>
>
> Thanks for reviewing the patches, I'll fix the initialization thing and the tegra_mmc credit :P
Please also add a changelog to your next revision. It helps reviewers
know what you have fixed.
--Sean
>
> On 5/7/2021 10:55 AM, Sean Anderson wrote:
>> On 5/6/21 10:50 PM, Tianrui Wei wrote:
>>> Hi Sean,
>>>
>>> Many thanks for taking your valuable time to review our patch and give
>>> suggestions. I'll improve both our patches accordingly. Just for the
>>> record, what would be the exact command you'd recommend to run
>>> checkpatch.pl? Because it only showed one warning about me not putting
>>> my name on the MAINTAINERS file on this patch.
>>
>> Huh. I would have expected checkpatch to catch the variable
>> initialization thing, but I guess it didn't.
>>
>>>
>>>
>>> Many thanks,
>>>
>>> Tianrui
>>>
>>> On 5/7/2021 10:43 AM, Sean Anderson wrote:
>>>> On 5/5/21 11:40 PM, Tianrui Wei wrote:
>>>>> From: Tianrui Wei <tianrui-wei at outlook.com>
>>>>> Date: Thu, 6 May 2021 11:30:20 +0800
>>>>> Subject: [PATCH V4 1/2] mmc: add OpenPiton mmc support
>>>>>
>>>>> This patch adds mmc support for OpenPiton.
>>>>> Specifically, some dts bindings were not used because
>>>>> our mmc controller doens't have those configuration
>>>>> options, it only exposes a dummy mmap interface for
>>>>> CPU.
>>>>
>>>> Please wrap your commit messages to 75 characterss, not 56 ;)
>>>>
>>>>>
>>>>> Signed-off-by: Tianrui Wei <tianrui-wei at outlook.com>
>>>>> Signed-off-by: Jonathan Balkind <jbalkind at ucsb.edu>
>>>>> ---
>>>>>
>>>>> drivers/mmc/Kconfig | 7 +
>>>>> drivers/mmc/Makefile | 1 +
>>>>> drivers/mmc/piton_mmc.c | 172 +++++
>>>>> 3 files changed, 180 insertions(+)
>>>>> create mode 100644 drivers/mmc/piton_mmc.c
>>>>>
>>>>> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
>>>>> index 14d79139..1038800f 100644
>>>>> --- a/drivers/mmc/Kconfig
>>>>> +++ b/drivers/mmc/Kconfig
>>>>> @@ -707,6 +707,13 @@ 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 SoC
>>>>> +
>>>>> 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
>>>>
>>>> Fix indentation. And please place this object in alphabetical order.
>>>>
>>>>> 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..266e26d8
>>>>> --- /dev/null
>>>>> +++ b/drivers/mmc/piton_mmc.c
>>>>> @@ -0,0 +1,172 @@
>>>>> +// 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>
>>>>
>>>> This is quite the list of authors. Where does this file come from?
>>>
>>>
>>> This file was adapted from a Samsung mmc file, so I kept the original authors.
>>
>> Can you add a note along the lines of "Adapted from tegra_mmc.c" for the
>> curiosity of future readers :)
>>
>> --Sean
>>
>>>
>>>>
>>>>> + */
>>>>> +
>>>>> +#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;
>>>>> +};
>>>>
>>>> Is platdata used? I don't see it in 2/2.
>>>>
>>>>> +
>>>>> +struct piton_mmc_priv {
>>>>> + u64 piton_mmc_base_addr; /* peripheral id */
>>>>
>>>> This should be void __iomem *.
>>>>
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * 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 */
>>>>> + if (!data)
>>>>> + return 0;
>>>>
>>>> Please place definitions before code (and please run checkpatch).
>>>>
>>>>> +
>>>>> + u64 byte_cnt = data->blocks * data->blocksize;
>>>>> + u64 start_block = cmd->cmdarg;
>>>>> + unsigned int *buff = (unsigned int *)data->dest;
>>>>
>>>> Why is buff an unsigned int? If you are using readl, shouldn't it be a
>>>> long?
>>>>
>>>>> +
>>>>> + struct piton_mmc_priv *priv = dev_get_priv(dev);
>>>>> + 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++;
>>>>> + }
>>>>
>>>> Can you use memcpy_fromio here?
>>>>
>>>>> + } else {
>>>>> + /* else there is a write
>>>>> + * we don't handle write, so error right away
>>>>> + */
>>>>> + return -ENODEV;
>>>>
>>>> Please use -ENOSYS.
>>>>
>>>>> + }
>>>>> +
>>>>> + 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;
>>>>> + 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;
>>>>> +}
>>>>> +
>>>>> +/* test if piton has the micro mmc card present
>>>>> + * always return 1, which means present
>>>>> + */
>>>>> +static int piton_mmc_getcd(struct udevice *dev)
>>>>> +{
>>>>> + /*
>>>>> + * always return 1
>>>>> + */
>>>>> + return 1;
>>>>> +}
>>>>> +
>>>>> +/* dummy function, piton_mmc don't need initialization
>>>>> + * in hw
>>>>> + */
>>>>> +static const struct dm_mmc_ops piton_mmc_ops = {
>>>>> + .send_cmd = piton_mmc_send_cmd,
>>>>> + .set_ios = piton_mmc_set_ios,
>>>>> + .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),
>>>>> +};
>>>>>
>>>>
>>>> --Sean
>>
More information about the U-Boot
mailing list