Re: [PATCH v4 4/7] ARM: sunxi: Add driver for SD/MMC hosts found on Allwinner sunxi SoCs

Ühel kenal päeval, R, 07.02.2014 kell 22:33, kirjutas David Lanzendörfer:

This is based on the driver Allwinner ships in their Android kernel 

sources.

Initial porting to upstream kernels done by David Lanzendörfer, additional
fixes and cleanups by Hans de Goede.

It uses dma in bus-master mode using a built-in designware idmac 

controller,

which is identical to the one found in the mmc-dw hosts.
The rest of the host is not identical to mmc-dw.

Signed-off-by: David Lanzendörfer 

[off-list ref]

Signed-off-by: Hans de Goede 

[off-list ref]

---
 drivers/mmc/host/Kconfig     |    7 
 drivers/mmc/host/Makefile    |    2 
 drivers/mmc/host/sunxi-mci.c |  871 

++++++++++++++++++++++++++++++++++++++++++

[...]

+
+static void sunxi_mmc_dump_errinfo(struct sunxi_mmc_host *smc_host)
+{
+    struct mmc_command *cmd = smc_host->mrq->cmd;
+    struct mmc_data *data = smc_host->mrq->data;
+
+    /* For some cmds timeout is normal with sd/mmc cards */
+    if ((smc_host->int_sum & SDXC_IntErrBit) == SDXC_RespTimeout &&
+            (cmd->opcode == 5 || cmd->opcode == 52))

Aren't these the ones defined in linux/mmc/sdio.h:
5  - SD_IO_SEND_OP_COND
52 - SD_IO_RW_DIRECT

+        return;
+
+    dev_err(mmc_dev(smc_host->mmc),
+        "smc %d err, cmd %d,%s%s%s%s%s%s%s%s%s%s !!\n",
+        smc_host->mmc->index, cmd->opcode,
+        data ? (data->flags & MMC_DATA_WRITE ? " WR" : " RD") : "",
+        smc_host->int_sum & SDXC_RespErr     ? " RE"     : "",
+        smc_host->int_sum & SDXC_RespCRCErr  ? " RCE"    : "",
+        smc_host->int_sum & SDXC_DataCRCErr  ? " DCE"    : "",
+        smc_host->int_sum & SDXC_RespTimeout ? " RTO"    : "",
+        smc_host->int_sum & SDXC_DataTimeout ? " DTO"    : "",
+        smc_host->int_sum & SDXC_FIFORunErr  ? " FE"     : "",
+        smc_host->int_sum & SDXC_HardWLocked ? " HL"     : "",
+        smc_host->int_sum & SDXC_StartBitErr ? " SBE"    : "",
+        smc_host->int_sum & SDXC_EndBitErr   ? " EBE"    : ""
+        );
+}

[...]

+static void sunxi_mmc_set_clk_dly(struct sunxi_mmc_host *smc_host,
+                  u32 oclk_dly, u32 sclk_dly)
+{
+    unsigned long iflags;
+    struct clk_hw *hw = __clk_get_hw(smc_host->clk_mod);
+
+    spin_lock_irqsave(&smc_host->lock, iflags);
+    clk_sunxi_mmc_phase_control(hw, sclk_dly, oclk_dly);
+    spin_unlock_irqrestore(&smc_host->lock, iflags);
+}
+
+struct sunxi_mmc_clk_dly mmc_clk_dly[MMC_CLK_MOD_NUM] = {
+    { MMC_CLK_400K, 0, 7 },
+    { MMC_CLK_25M, 0, 5 },
+    { MMC_CLK_50M, 3, 5 },
+    { MMC_CLK_50MDDR, 2, 4 },
+    { MMC_CLK_50MDDR_8BIT, 2, 4 },
+    { MMC_CLK_100M, 1, 4 },
+    { MMC_CLK_200M, 1, 4 },

From the looks of it, it should be static const, but apparently this was

raising a warning when David did it.. Hints anyone?

+};
+
+static void sunxi_mmc_clk_set_rate(struct sunxi_mmc_host *smc_host,
+                   unsigned int rate)
+{
+    u32 newrate;
+    u32 src_clk;
+    u32 oclk_dly;
+    u32 sclk_dly;
+    u32 temp;
+    struct sunxi_mmc_clk_dly *dly = NULL;
+
+    newrate = clk_round_rate(smc_host->clk_mod, rate);
+    if (smc_host->clk_mod_rate == newrate) {
+        dev_dbg(mmc_dev(smc_host->mmc), "clk already %d, rounded %d\n",
+            rate, newrate);
+        return;
+    }
+
+    dev_dbg(mmc_dev(smc_host->mmc), "setting clk to %d, rounded %d\n",
+        rate, newrate);
+
+    /* setting clock rate */
+    clk_disable(smc_host->clk_mod);
+    clk_set_rate(smc_host->clk_mod, newrate);
+    clk_enable(smc_host->clk_mod);
+    smc_host->clk_mod_rate = newrate = clk_get_rate(smc_host->clk_mod);
+    dev_dbg(mmc_dev(smc_host->mmc), "clk is now %d\n", newrate);
+
+    sunxi_mmc_oclk_onoff(smc_host, 0);
+    /* clear internal divider */
+    temp = mci_readl(smc_host, REG_CLKCR);
+    temp &= ~0xff;
+    mci_writel(smc_host, REG_CLKCR, temp);
+
+    /* determine delays */
+    if (rate <= 400000) {
+        dly = &mmc_clk_dly[MMC_CLK_400K];
+    } else if (rate <= 25000000) {
+        dly = &mmc_clk_dly[MMC_CLK_25M];
+    } else if (rate <= 50000000) {
+        if (smc_host->ddr) {
+            if (smc_host->bus_width == 8)
+                dly = &mmc_clk_dly[MMC_CLK_50MDDR_8BIT];
+            else
+                dly = &mmc_clk_dly[MMC_CLK_50MDDR];
+        } else {
+            dly = &mmc_clk_dly[MMC_CLK_50M];
+        }
+    } else if (rate <= 104000000) {
+        dly = &mmc_clk_dly[MMC_CLK_100M];
+    } else if (rate <= 208000000) {
+        dly = &mmc_clk_dly[MMC_CLK_200M];
+    } else {
+        dly = &mmc_clk_dly[MMC_CLK_50M];
+    }
+
+    oclk_dly = dly->oclk_dly;
+    sclk_dly = dly->sclk_dly;
+
+    src_clk = clk_get_rate(clk_get_parent(smc_host->clk_mod));
+
+    if (src_clk >= 300000000 && src_clk <= 400000000) {
+        if (oclk_dly)
+            oclk_dly--;
+        if (sclk_dly)
+            sclk_dly--;
+    }
+
+    sunxi_mmc_set_clk_dly(smc_host, oclk_dly, sclk_dly);
+    sunxi_mmc_oclk_onoff(smc_host, 1);
+
+    /* oclk_onoff sets various irq status bits, clear these */
+    mci_writel(smc_host, REG_RINTR,
+           mci_readl(smc_host, REG_RINTR) & ~SDXC_SDIOInt);
+}
+

[...]

[off-list ref]

+ * (C) Copyright 2014-2015 O2S GmbH <www.o2s.ch>
+ * (C) Copyright 2014-2015 David Lanzendörfer 

[off-list ref]

+ * (C) Copyright 2014-2015 Hans de Goede 

[off-list ref]

Had to take a look at the calendar...

+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ */
+
+#ifndef __SUNXI_MCI_H__
+#define __SUNXI_MCI_H__
+

[...]

+
+struct sunxi_idma_des {
+    u32    config;
+#define SDXC_IDMAC_DES0_DIC    BIT(1)  /* disable interrupt on 

completion */

+#define SDXC_IDMAC_DES0_LD    BIT(2)  /* last descriptor */
+#define SDXC_IDMAC_DES0_FD    BIT(3)  /* first descriptor */
+#define SDXC_IDMAC_DES0_CH    BIT(4)  /* chain mode */
+#define SDXC_IDMAC_DES0_ER    BIT(5)  /* end of ring */
+#define SDXC_IDMAC_DES0_CES    BIT(30) /* card error summary */
+#define SDXC_IDMAC_DES0_OWN    BIT(31) /* 1-idma owns it, 0-host owns it 

*/

This #define placement inside the struct looks really weird.

+
+    /*
+     * If the idma-des-size-bits of property is ie 13, bufsize bits are:
+     *  Bits  0-12: buf1 size
+     *  Bits 13-25: buf2 size
+     *  Bits 26-31: not used
+     * Since we only ever set buf1 size, we can simply store it directly.
+     */
+    u32    buf_size;
+    u32    buf_addr_ptr1;
+    u32    buf_addr_ptr2;
+};
+

[...]

+
+#define MMC_CLK_400K            0
+#define MMC_CLK_25M             1
+#define MMC_CLK_50M             2
+#define MMC_CLK_50MDDR          3
+#define MMC_CLK_50MDDR_8BIT     4
+#define MMC_CLK_100M            5
+#define MMC_CLK_200M            6
+#define MMC_CLK_MOD_NUM         7
+
+struct sunxi_mmc_clk_dly {
+    u32 mode;
+    u32 oclk_dly;
+    u32 sclk_dly;

Do these members have to be u32? They all seem to be smaller than 10.


-- 
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/groups/opt_out.

Hi

quoted

+            (cmd->opcode == 5 || cmd->opcode == 52))

Aren't these the ones defined in linux/mmc/sdio.h:
5  - SD_IO_SEND_OP_COND
52 - SD_IO_RW_DIRECT

Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

quoted

+struct sunxi_mmc_clk_dly {
+    u32 mode;
+    u32 oclk_dly;
+    u32 sclk_dly;

Do these members have to be u32? They all seem to be smaller than 10.

Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david

Hi

quoted

+            (cmd->opcode == 5 || cmd->opcode == 52))

Aren't these the ones defined in linux/mmc/sdio.h:
5  - SD_IO_SEND_OP_COND
52 - SD_IO_RW_DIRECT

Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

quoted

+struct sunxi_mmc_clk_dly {
+    u32 mode;
+    u32 oclk_dly;
+    u32 sclk_dly;

Do these members have to be u32? They all seem to be smaller than 10.

Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david

Hi

quoted

+            (cmd->opcode == 5 || cmd->opcode == 52))

Aren't these the ones defined in linux/mmc/sdio.h:
5  - SD_IO_SEND_OP_COND
52 - SD_IO_RW_DIRECT

Yes. They are...
Changed that.
Also I removed the camel cases and the defines within the struct definition.

quoted

+struct sunxi_mmc_clk_dly {
+    u32 mode;
+    u32 oclk_dly;
+    u32 sclk_dly;

Do these members have to be u32? They all seem to be smaller than 10.

Yes. Because of situations where it gets used in bit operations, and shorter
types mess things up and prevent the driver from working ;-)

cheer
david