On 11/28/2016 03:58 AM, Jaehoon Chung wrote: > Hi Marek, > > On 11/26/2016 07:32 AM, Marek Vasut wrote: >> From: Paul Burton <[email protected]> >> >> Add driver for the JZ47xx MSC controller. > > There are some checkpatch error and warings. Could you fix them?
Yeah > And i don't know what means MSC? Me neither, probably MMC SD Controller . >> Signed-off-by: Marek Vasut <[email protected]> >> Cc: Daniel Schwierzeck <[email protected]> >> Cc: Paul Burton <[email protected]> >> --- >> drivers/mmc/Kconfig | 6 + >> drivers/mmc/Makefile | 1 + >> drivers/mmc/jz_mmc.c | 443 >> +++++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 450 insertions(+) >> create mode 100644 drivers/mmc/jz_mmc.c >> >> diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig >> index aca438b8..761b886 100644 >> --- a/drivers/mmc/Kconfig >> +++ b/drivers/mmc/Kconfig >> @@ -102,6 +102,12 @@ config MMC_UNIPHIER >> help >> This selects support for the SD/MMC Host Controller on UniPhier SoCs. >> >> +config JZ47XX_MMC >> + bool "Ingenic JZ47xx SD/MMC Host Controller support" >> + depends on ARCH_JZ47XX >> + help >> + This selects support for the SD Card Controller on Ingenic JZ47xx >> SoCs. >> + >> config SANDBOX_MMC >> bool "Sandbox MMC support" >> depends on MMC && SANDBOX >> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile >> index d850758..5f7cca3 100644 >> --- a/drivers/mmc/Makefile >> +++ b/drivers/mmc/Makefile >> @@ -57,6 +57,7 @@ obj-$(CONFIG_TEGRA_MMC) += tegra_mmc.o >> obj-$(CONFIG_MMC_UNIPHIER) += uniphier-sd.o >> obj-$(CONFIG_ZYNQ_SDHCI) += zynq_sdhci.o >> obj-$(CONFIG_ROCKCHIP_SDHCI) += rockchip_sdhci.o >> +obj-$(CONFIG_JZ47XX_MMC) += jz_mmc.o >> >> ifdef CONFIG_SPL_BUILD >> obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o >> diff --git a/drivers/mmc/jz_mmc.c b/drivers/mmc/jz_mmc.c >> new file mode 100644 >> index 0000000..213fe63 >> --- /dev/null >> +++ b/drivers/mmc/jz_mmc.c >> @@ -0,0 +1,443 @@ >> +/* >> + * Ingenic JZ MMC driver >> + * >> + * Copyright (c) 2013 Imagination Technologies >> + * Author: Paul Burton <[email protected]> >> + * >> + * SPDX-License-Identifier: GPL-2.0+ >> + */ >> + >> +#include <common.h> >> +#include <malloc.h> >> +#include <mmc.h> >> +#include <errno.h> >> +#include <asm/io.h> >> +#include <asm/unaligned.h> >> +#include <mach/jz4780.h> >> + >> +/* Registers */ >> +#define MSC_STRPCL 0x000 >> +#define MSC_STAT 0x004 >> +#define MSC_CLKRT 0x008 >> +#define MSC_CMDAT 0x00c >> +#define MSC_RESTO 0x010 >> +#define MSC_RDTO 0x014 >> +#define MSC_BLKLEN 0x018 >> +#define MSC_NOB 0x01c >> +#define MSC_SNOB 0x020 >> +#define MSC_IMASK 0x024 >> +#define MSC_IREG 0x028 >> +#define MSC_CMD 0x02c >> +#define MSC_ARG 0x030 >> +#define MSC_RES 0x034 >> +#define MSC_RXFIFO 0x038 >> +#define MSC_TXFIFO 0x03c >> +#define MSC_LPM 0x040 >> +#define MSC_DMAC 0x044 >> +#define MSC_DMANDA 0x048 >> +#define MSC_DMADA 0x04c >> +#define MSC_DMALEN 0x050 >> +#define MSC_DMACMD 0x054 >> +#define MSC_CTRL2 0x058 >> +#define MSC_RTCNT 0x05c >> +#define MSC_DBG 0x0fc >> + >> +/* MSC Clock and Control Register (MSC_STRPCL) */ >> + >> +#define MSC_STRPCL_EXIT_MULTIPLE BIT(7) >> +#define MSC_STRPCL_EXIT_TRANSFER BIT(6) >> +#define MSC_STRPCL_START_READWAIT BIT(5) >> +#define MSC_STRPCL_STOP_READWAIT BIT(4) >> +#define MSC_STRPCL_RESET BIT(3) >> +#define MSC_STRPCL_START_OP BIT(2) >> +#define MSC_STRPCL_CLOCK_CONTROL_STOP BIT(0) >> +#define MSC_STRPCL_CLOCK_CONTROL_START BIT(1) >> + >> +/* MSC Status Register (MSC_STAT) */ >> + >> +#define MSC_STAT_AUTO_CMD_DONE BIT(31) >> +#define MSC_STAT_IS_RESETTING BIT(15) >> +#define MSC_STAT_SDIO_INT_ACTIVE BIT(14) >> +#define MSC_STAT_PRG_DONE BIT(13) >> +#define MSC_STAT_DATA_TRAN_DONE BIT(12) >> +#define MSC_STAT_END_CMD_RES BIT(11) >> +#define MSC_STAT_DATA_FIFO_AFULL BIT(10) >> +#define MSC_STAT_IS_READWAIT BIT(9) >> +#define MSC_STAT_CLK_EN BIT(8) >> +#define MSC_STAT_DATA_FIFO_FULL BIT(7) >> +#define MSC_STAT_DATA_FIFO_EMPTY BIT(6) >> +#define MSC_STAT_CRC_RES_ERR BIT(5) >> +#define MSC_STAT_CRC_READ_ERROR BIT(4) >> +#define MSC_STAT_CRC_WRITE_ERROR BIT(2) >> +#define MSC_STAT_CRC_WRITE_ERROR_NOSTS BIT(4) >> +#define MSC_STAT_TIME_OUT_RES BIT(1) >> +#define MSC_STAT_TIME_OUT_READ BIT(0) >> + >> +/* MSC Bus Clock Control Register (MSC_CLKRT) */ >> +#define MSC_CLKRT_CLK_RATE_MASK 0x7 >> + >> +/* MSC Command Sequence Control Register (MSC_CMDAT) */ >> + >> +#define MSC_CMDAT_IO_ABORT BIT(11) >> +#define MSC_CMDAT_BUS_WIDTH_1BIT (0x0 << 9) >> +#define MSC_CMDAT_BUS_WIDTH_4BIT (0x2 << 9) >> +#define MSC_CMDAT_DMA_EN BIT(8) >> +#define MSC_CMDAT_INIT BIT(7) >> +#define MSC_CMDAT_BUSY BIT(6) >> +#define MSC_CMDAT_STREAM_BLOCK BIT(5) >> +#define MSC_CMDAT_WRITE BIT(4) >> +#define MSC_CMDAT_DATA_EN BIT(3) >> +#define MSC_CMDAT_RESPONSE_MASK (0x7 << 0) >> +#define MSC_CMDAT_RESPONSE_NONE (0x0 << 0) /* No response */ >> +#define MSC_CMDAT_RESPONSE_R1 (0x1 << 0) /* Format R1 and R1b >> */ >> +#define MSC_CMDAT_RESPONSE_R2 (0x2 << 0) /* Format R2 */ >> +#define MSC_CMDAT_RESPONSE_R3 (0x3 << 0) /* Format R3 */ >> +#define MSC_CMDAT_RESPONSE_R4 (0x4 << 0) /* Format R4 */ >> +#define MSC_CMDAT_RESPONSE_R5 (0x5 << 0) /* Format R5 */ >> +#define MSC_CMDAT_RESPONSE_R6 (0x6 << 0) /* Format R6 */ > > It doesn't need to shift 0. > >> + >> +/* MSC Interrupts Mask Register (MSC_IMASK) */ >> + >> +#define MSC_IMASK_TIME_OUT_RES BIT(9) >> +#define MSC_IMASK_TIME_OUT_READ BIT(8) >> +#define MSC_IMASK_SDIO BIT(7) >> +#define MSC_IMASK_TXFIFO_WR_REQ BIT(6) >> +#define MSC_IMASK_RXFIFO_RD_REQ BIT(5) >> +#define MSC_IMASK_END_CMD_RES BIT(2) >> +#define MSC_IMASK_PRG_DONE BIT(1) >> +#define MSC_IMASK_DATA_TRAN_DONE BIT(0) >> + >> + >> +/* MSC Interrupts Status Register (MSC_IREG) */ >> + >> +#define MSC_IREG_TIME_OUT_RES BIT(9) >> +#define MSC_IREG_TIME_OUT_READ BIT(8) >> +#define MSC_IREG_SDIO BIT(7) >> +#define MSC_IREG_TXFIFO_WR_REQ BIT(6) >> +#define MSC_IREG_RXFIFO_RD_REQ BIT(5) >> +#define MSC_IREG_END_CMD_RES BIT(2) >> +#define MSC_IREG_PRG_DONE BIT(1) >> +#define MSC_IREG_DATA_TRAN_DONE BIT(0) >> + >> +struct jz_mmc_priv { >> + struct mmc_config cfg; >> + void __iomem *regs; >> + u32 flags; /* FIXME */ > > What FIXME? Dunno, removed. >> +/* priv flags */ >> +#define JZ_MMC_BUS_WIDTH_MASK 0x3 >> +#define JZ_MMC_BUS_WIDTH_1 0x0 >> +#define JZ_MMC_BUS_WIDTH_4 0x2 >> +#define JZ_MMC_BUS_WIDTH_8 0x3 >> +#define JZ_MMC_SENT_INIT BIT(2) >> +}; >> + >> +static int jz_mmc_clock_rate(void) >> +{ >> + return 24000000; >> +} > > I don't know this function is really needs.. Ev. this clock rate should come from DT, so this function should be here. But, we don't support DT in SPL due to size concerns (the SPL has to be below 14 kiB, see previous patch), so there will eventually be come more code here I think. >> + >> +static int jz_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, >> + struct mmc_data *data) >> +{ >> + struct jz_mmc_priv *priv = mmc->priv; >> + u32 stat, mask, cmdat = 0; >> + >> + /* stop the clock */ >> + writel(MSC_STRPCL_CLOCK_CONTROL_STOP, priv->regs + MSC_STRPCL); >> + /* FIXME -- wait_bit() */ > > Fixme -- wait bit()? Seems like wait_for_bit now fits, so fixed. >> + while (readl(priv->regs + MSC_STAT) & MSC_STAT_CLK_EN); >> + >> + writel(0, priv->regs + MSC_DMAC); [...] >> + } else if (data && (data->flags & MMC_DATA_READ)) { >> + /* read the data */ >> + int sz = data->blocks * data->blocksize; >> + void *buf = data->dest; >> + >> + do { >> + stat = readl(priv->regs + MSC_STAT); >> + >> + if (stat & MSC_STAT_TIME_OUT_READ) >> + return -ETIMEDOUT; >> + if (stat & MSC_STAT_CRC_READ_ERROR) >> + return -ECOMM; > > Not ECOMM..It's CRC error..then use the "-EILSEQ 84" > >> + if (stat & MSC_STAT_DATA_FIFO_EMPTY) { >> + udelay(10); > > Why put udelay at here? Wait until more data get loaded, the hardware is crap and sensitive to me polling this register too often (sigh). >> + continue; >> + } >> + do { >> + u32 val = readl(priv->regs + MSC_RXFIFO); >> + >> + if (sz == 1) >> + *(u8 *)buf = (u8)val; >> + else if (sz == 2) >> + put_unaligned_le16(val, buf); >> + else if (sz >= 4) >> + put_unaligned_le32(val, buf); >> + buf += 4; >> + sz -= 4; >> + stat = readl(priv->regs + MSC_STAT); >> + } while (!(stat & MSC_STAT_DATA_FIFO_EMPTY)); >> + } while (!(stat & MSC_STAT_DATA_TRAN_DONE)); >> + } >> + >> + return 0; >> +} [...] >> +static int jz_mmc_ofdata_to_platdata(struct udevice *dev) >> +{ >> + struct jz_mmc_priv *priv = dev_get_priv(dev); >> + const void *fdt = gd->fdt_blob; >> + int node = dev->of_offset; >> + struct mmc_config *cfg; >> + int val; >> + >> + priv->regs = map_physmem(dev_get_addr(dev), 0x100, MAP_NOCACHE); >> + cfg = &priv->cfg; >> + >> + cfg->host_caps = MMC_MODE_HS_52MHz | MMC_MODE_HS; >> + val = fdtdec_get_int(fdt, node, "bus-width", -1); > > I think that almost all mmc can support 1bit buswidth.. True >> + if (val < 0) { >> + printf("error: bus-width property missing\n"); >> + return -ENOENT; >> + } >> + >> + switch (val) { >> + case 0x8: >> + cfg->host_caps |= MMC_MODE_8BIT; >> + case 0x4: >> + cfg->host_caps |= MMC_MODE_4BIT; >> + break; >> + default: >> + printf("error: invalid bus-width property\n"); > > Dosen't consider 1bit-buswidth? Oh, good catch. >> + return -ENOENT; >> + } >> + >> + cfg->f_min = 400000; >> + cfg->f_max = fdtdec_get_int(fdt, node, "max-frequency", 52000000); >> + >> + >> + [...] -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
