lkml.org 
[lkml]   [2010]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] davinci: MMC: Adding support for 8bit MMC cards
From
Date
Sergei Shtylyov <sshtylyov@ru.mvista.com> 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 <vipin.bhandari@ti.com>
>>
>
> 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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/


\
 
 \ /
  Last update: 2010-01-06 01:15    [W:0.065 / U:0.220 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site