2009-12-28 12:28:31

by Vipin Bhandari

[permalink] [raw]
Subject: [PATCH] davinci: MMC: Adding support for 8bit MMC cards

This patch adds the support for 8bit MMC cards. The controller
data width is configurable depending on the wires setting in the
platform data structure.

MMC 8bit is tested on OMAPL137 and MMC 4bit is tested on OMAPL138 EVM.

Signed-off-by: Vipin Bhandari <[email protected]>
---

This patch has been generated against latest Linus's kernel.

drivers/mmc/host/davinci_mmc.c | 45 +++++++++++++++++++++++++++++++--------
1 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/davinci_mmc.c b/drivers/mmc/host/davinci_mmc.c
index dd45e7c..3bd0ba2 100644
--- a/drivers/mmc/host/davinci_mmc.c
+++ b/drivers/mmc/host/davinci_mmc.c
@@ -73,6 +73,7 @@
/* DAVINCI_MMCCTL definitions */
#define MMCCTL_DATRST (1 << 0)
#define MMCCTL_CMDRST (1 << 1)
+#define MMCCTL_WIDTH_8_BIT (1 << 8)
#define MMCCTL_WIDTH_4_BIT (1 << 2)
#define MMCCTL_DATEG_DISABLED (0 << 6)
#define MMCCTL_DATEG_RISING (1 << 6)
@@ -791,22 +792,42 @@ static void calculate_clk_divider(struct mmc_host *mmc, struct mmc_ios *ios)

static void mmc_davinci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
{
- unsigned int mmc_pclk = 0;
struct mmc_davinci_host *host = mmc_priv(mmc);

- mmc_pclk = host->mmc_input_clk;
dev_dbg(mmc_dev(host->mmc),
"clock %dHz busmode %d powermode %d Vdd %04x\n",
ios->clock, ios->bus_mode, ios->power_mode,
ios->vdd);
- if (ios->bus_width == MMC_BUS_WIDTH_4) {
- dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
- writel(readl(host->base + DAVINCI_MMCCTL) | MMCCTL_WIDTH_4_BIT,
- host->base + DAVINCI_MMCCTL);
- } else {
- dev_dbg(mmc_dev(host->mmc), "Disabling 4 bit mode\n");
- writel(readl(host->base + DAVINCI_MMCCTL) & ~MMCCTL_WIDTH_4_BIT,
+
+ switch (ios->bus_width) {
+ case MMC_BUS_WIDTH_8:
+ dev_dbg(mmc_dev(host->mmc), "Enabling 8 bit mode\n");
+ writel((readl(host->base + DAVINCI_MMCCTL) &
+ ~MMCCTL_WIDTH_4_BIT) | MMCCTL_WIDTH_8_BIT,
host->base + DAVINCI_MMCCTL);
+ break;
+ case MMC_BUS_WIDTH_4:
+ dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
+ if (host->version == MMC_CTLR_VERSION_2)
+ writel((readl(host->base + DAVINCI_MMCCTL) &
+ ~MMCCTL_WIDTH_8_BIT) | MMCCTL_WIDTH_4_BIT,
+ host->base + DAVINCI_MMCCTL);
+ else
+ writel(readl(host->base + DAVINCI_MMCCTL) |
+ MMCCTL_WIDTH_4_BIT,
+ host->base + DAVINCI_MMCCTL);
+ break;
+ case MMC_BUS_WIDTH_1:
+ dev_dbg(mmc_dev(host->mmc), "Enabling 1 bit mode\n");
+ if (host->version == MMC_CTLR_VERSION_2)
+ writel(readl(host->base + DAVINCI_MMCCTL) &
+ ~(MMCCTL_WIDTH_8_BIT | MMCCTL_WIDTH_4_BIT),
+ host->base + DAVINCI_MMCCTL);
+ else
+ writel(readl(host->base + DAVINCI_MMCCTL) &
+ ~MMCCTL_WIDTH_4_BIT,
+ host->base + DAVINCI_MMCCTL);
+ break;
}

calculate_clk_divider(mmc, ios);
@@ -1189,10 +1210,14 @@ static int __init davinci_mmcsd_probe(struct platform_device *pdev)

/* REVISIT: someday, support IRQ-driven card detection. */
mmc->caps |= MMC_CAP_NEEDS_POLL;
+ mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;

- if (!pdata || pdata->wires == 4 || pdata->wires == 0)
+ if (pdata && (pdata->wires == 4 || pdata->wires == 0))
mmc->caps |= MMC_CAP_4_BIT_DATA;

+ if (pdata && (pdata->wires == 8))
+ mmc->caps |= (MMC_CAP_4_BIT_DATA | MMC_CAP_8_BIT_DATA);
+
host->version = pdata->version;

mmc->ops = &mmc_davinci_ops;
--
1.5.6


2009-12-30 11:00:25

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] davinci: MMC: Adding support for 8bit MMC cards

Hello.

Vipin Bhandari wrote:

[Re-replying to all, as I only replied to Vipin the first time.]

> This patch adds the support for 8bit MMC cards. The controller
> data width is configurable depending on the wires setting in the
> platform data structure.
>
> MMC 8bit is tested on OMAPL137 and MMC 4bit is tested on OMAPL138 EVM.
>
> Signed-off-by: Vipin Bhandari <[email protected]>
>

This has been done by MV in the internal tree but as you code is
significantly differenet from that one, I'm not asking you about the
missing MV signoffs...

> diff --git a/drivers/mmc/host/davinci_mmc.c
> b/drivers/mmc/host/davinci_mmc.c
> index dd45e7c..3bd0ba2 100644
> --- a/drivers/mmc/host/davinci_mmc.c
> +++ b/drivers/mmc/host/davinci_mmc.c
> @@ -73,6 +73,7 @@
> /* DAVINCI_MMCCTL definitions */
> #define MMCCTL_DATRST (1 << 0)
> #define MMCCTL_CMDRST (1 << 1)
> +#define MMCCTL_WIDTH_8_BIT (1 << 8)
> #define MMCCTL_WIDTH_4_BIT (1 << 2)
> #define MMCCTL_DATEG_DISABLED (0 << 6)
> #define MMCCTL_DATEG_RISING (1 << 6)
> @@ -791,22 +792,42 @@ static void calculate_clk_divider(struct
> mmc_host *mmc, struct mmc_ios *ios)
>
> static void mmc_davinci_set_ios(struct mmc_host *mmc, struct mmc_ios
> *ios)
> {
> - unsigned int mmc_pclk = 0;
> struct mmc_davinci_host *host = mmc_priv(mmc);
>
> - mmc_pclk = host->mmc_input_clk;
> dev_dbg(mmc_dev(host->mmc),
> "clock %dHz busmode %d powermode %d Vdd %04x\n",
> ios->clock, ios->bus_mode, ios->power_mode,
> ios->vdd);
> - if (ios->bus_width == MMC_BUS_WIDTH_4) {
> - dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
> - writel(readl(host->base + DAVINCI_MMCCTL) | MMCCTL_WIDTH_4_BIT,
> - host->base + DAVINCI_MMCCTL);
> - } else {
> - dev_dbg(mmc_dev(host->mmc), "Disabling 4 bit mode\n");
> - writel(readl(host->base + DAVINCI_MMCCTL) & ~MMCCTL_WIDTH_4_BIT,
> +
> + switch (ios->bus_width) {
> + case MMC_BUS_WIDTH_8:
> + dev_dbg(mmc_dev(host->mmc), "Enabling 8 bit mode\n");
> + writel((readl(host->base + DAVINCI_MMCCTL) &
> + ~MMCCTL_WIDTH_4_BIT) | MMCCTL_WIDTH_8_BIT,
> host->base + DAVINCI_MMCCTL);
> + break;
> + case MMC_BUS_WIDTH_4:
> + dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
> + if (host->version == MMC_CTLR_VERSION_2)
> + writel((readl(host->base + DAVINCI_MMCCTL) &
> + ~MMCCTL_WIDTH_8_BIT) | MMCCTL_WIDTH_4_BIT,
> + host->base + DAVINCI_MMCCTL);
> + else
> + writel(readl(host->base + DAVINCI_MMCCTL) |
> + MMCCTL_WIDTH_4_BIT,
> + host->base + DAVINCI_MMCCTL);
>

I don't think it makes sense to check for host->version just to not
clear the bit which is reserved for original DaVinci. There's nothing
criminal in clearing a reserved bit, so I'm suggesting that you remove
the check.

> + break;
> + case MMC_BUS_WIDTH_1:
> + dev_dbg(mmc_dev(host->mmc), "Enabling 1 bit mode\n");
> + if (host->version == MMC_CTLR_VERSION_2)
> + writel(readl(host->base + DAVINCI_MMCCTL) &
> + ~(MMCCTL_WIDTH_8_BIT | MMCCTL_WIDTH_4_BIT),
> + host->base + DAVINCI_MMCCTL);
> + else
> + writel(readl(host->base + DAVINCI_MMCCTL) &
> + ~MMCCTL_WIDTH_4_BIT,
> + host->base + DAVINCI_MMCCTL);
>

Same comment here...

> + break;
>

It'll result less code if you read/write the register only once --
before/after the *switch* statement respectively.

> }
>
> calculate_clk_divider(mmc, ios);
> @@ -1189,10 +1210,14 @@ static int __init davinci_mmcsd_probe(struct
> platform_device *pdev)
>
> /* REVISIT: someday, support IRQ-driven card detection. */
> mmc->caps |= MMC_CAP_NEEDS_POLL;
> + mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
>

Does this flag have to do with the bus width at all? :-/

WBR, Sergei

2010-01-06 00:12:13

by Kevin Hilman

[permalink] [raw]
Subject: Re: [PATCH] davinci: MMC: Adding support for 8bit MMC cards

Sergei Shtylyov <[email protected]> writes:

> Hello.
>
> Vipin Bhandari wrote:
>
> [Re-replying to all, as I only replied to Vipin the first time.]
>
>> This patch adds the support for 8bit MMC cards. The controller
>> data width is configurable depending on the wires setting in the
>> platform data structure.
>>
>> MMC 8bit is tested on OMAPL137 and MMC 4bit is tested on OMAPL138 EVM.
>>
>> Signed-off-by: Vipin Bhandari <[email protected]>
>>
>
> This has been done by MV in the internal tree but as you code is
> significantly differenet from that one, I'm not asking you about the
> missing MV signoffs...
>
>> diff --git a/drivers/mmc/host/davinci_mmc.c
>> b/drivers/mmc/host/davinci_mmc.c
>> index dd45e7c..3bd0ba2 100644
>> --- a/drivers/mmc/host/davinci_mmc.c
>> +++ b/drivers/mmc/host/davinci_mmc.c
>> @@ -73,6 +73,7 @@
>> /* DAVINCI_MMCCTL definitions */
>> #define MMCCTL_DATRST (1 << 0)
>> #define MMCCTL_CMDRST (1 << 1)
>> +#define MMCCTL_WIDTH_8_BIT (1 << 8)
>> #define MMCCTL_WIDTH_4_BIT (1 << 2)
>> #define MMCCTL_DATEG_DISABLED (0 << 6)
>> #define MMCCTL_DATEG_RISING (1 << 6)
>> @@ -791,22 +792,42 @@ static void calculate_clk_divider(struct
>> mmc_host *mmc, struct mmc_ios *ios)
>> static void mmc_davinci_set_ios(struct mmc_host *mmc, struct
>> mmc_ios *ios)
>> {
>> - unsigned int mmc_pclk = 0;
>> struct mmc_davinci_host *host = mmc_priv(mmc);
>> - mmc_pclk = host->mmc_input_clk;
>> dev_dbg(mmc_dev(host->mmc),
>> "clock %dHz busmode %d powermode %d Vdd %04x\n",
>> ios->clock, ios->bus_mode, ios->power_mode,
>> ios->vdd);
>> - if (ios->bus_width == MMC_BUS_WIDTH_4) {
>> - dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
>> - writel(readl(host->base + DAVINCI_MMCCTL) | MMCCTL_WIDTH_4_BIT,
>> - host->base + DAVINCI_MMCCTL);
>> - } else {
>> - dev_dbg(mmc_dev(host->mmc), "Disabling 4 bit mode\n");
>> - writel(readl(host->base + DAVINCI_MMCCTL) & ~MMCCTL_WIDTH_4_BIT,
>> +
>> + switch (ios->bus_width) {
>> + case MMC_BUS_WIDTH_8:
>> + dev_dbg(mmc_dev(host->mmc), "Enabling 8 bit mode\n");
>> + writel((readl(host->base + DAVINCI_MMCCTL) &
>> + ~MMCCTL_WIDTH_4_BIT) | MMCCTL_WIDTH_8_BIT,
>> host->base + DAVINCI_MMCCTL);
>> + break;
>> + case MMC_BUS_WIDTH_4:
>> + dev_dbg(mmc_dev(host->mmc), "Enabling 4 bit mode\n");
>> + if (host->version == MMC_CTLR_VERSION_2)
>> + writel((readl(host->base + DAVINCI_MMCCTL) &
>> + ~MMCCTL_WIDTH_8_BIT) | MMCCTL_WIDTH_4_BIT,
>> + host->base + DAVINCI_MMCCTL);
>> + else
>> + writel(readl(host->base + DAVINCI_MMCCTL) |
>> + MMCCTL_WIDTH_4_BIT,
>> + host->base + DAVINCI_MMCCTL);
>>
>
> I don't think it makes sense to check for host->version just to not
> clear the bit which is reserved for original DaVinci. There's nothing
> criminal in clearing a reserved bit, so I'm suggesting that you remove
> the check.

There's nothing criminal... yet... until some new version of the
controller uses that bit for something else. I think it's always best
to not touch reserved bits unless there is a good, well tested, reason
not to, such as performance etc.

Kevin


>> + break;
>> + case MMC_BUS_WIDTH_1:
>> + dev_dbg(mmc_dev(host->mmc), "Enabling 1 bit mode\n");
>> + if (host->version == MMC_CTLR_VERSION_2)
>> + writel(readl(host->base + DAVINCI_MMCCTL) &
>> + ~(MMCCTL_WIDTH_8_BIT | MMCCTL_WIDTH_4_BIT),
>> + host->base + DAVINCI_MMCCTL);
>> + else
>> + writel(readl(host->base + DAVINCI_MMCCTL) &
>> + ~MMCCTL_WIDTH_4_BIT,
>> + host->base + DAVINCI_MMCCTL);
>>
>
> Same comment here...
>
>> + break;
>>
>
> It'll result less code if you read/write the register only once --
> before/after the *switch* statement respectively.
>
>> }
>> calculate_clk_divider(mmc, ios);
>> @@ -1189,10 +1210,14 @@ static int __init davinci_mmcsd_probe(struct
>> platform_device *pdev)
>> /* REVISIT: someday, support IRQ-driven card detection. */
>> mmc->caps |= MMC_CAP_NEEDS_POLL;
>> + mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
>>
>
> Does this flag have to do with the bus width at all? :-/
>
> WBR, Sergei
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/