[lkml]   [2009]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [Patch 1/1 v5] via-sdmmc: VIA MSP card reader driver support
Dear Joseph, Pierre and others.

Since the driver has still not made any progress in recent days, I took the
liberty of "adopting" it. It can now be found in my linux-2.6-via.git
repository under the via-vx855-sdmmc branch (gitweb at;a=shortlog;h=refs/heads/via-vx855-sdmmc)

As you can see, there are five incremental patches compared to the last v5
release that Joseph posted in April.

On Wed, Apr 22, 2009 at 01:35:29AM +0800, wrote:

> >> +nodata:
> >> + if (cmd->opcode == MMC_STOP_TRANSMISSION)
> >> + cmdctrl |= VIA_CRDR_SDCTRL_STOP;
> > >+
> >I didn't check this part properly the last time. I'm assuming you're telling
> >the hardware that this is a command that terminates a data transmission? If
> >so, you cannot check the opcode like that. There might be other stop
> >commands.
> >What you need to do is to either pass another parameter to your function
> >when you're calling it with data->stop, or simply have a test in there if
> >cmd == data->stop (or mrq->stop, whichever is most convenient).
> OK, we will change to
> if(cmd == host->mrq->stop)
> cmdctrl |= VIA_CRDR_SDCTRL_STOP;


> >> + else if (ios->clock >= 5000000)
> >> + clock = PCI_CLK_8M;
> >>+ else
> >>+ clock = PCI_CLK_375K;
> >>+
> > This looks a bit strange. Why are you not comparing with the clock
> > frequency you'll be setting?
> Do you mean that we should check the current clock frequency before setting?
> But our engineer thinks that the following lines can do that work normally
> too: Any comment?

Well, if the "PCI_CLK" line and the corresponding register actually 1:1
correspond to the actual SD card clock rate, then

> else if (ios->clock >= 5000000)
> clock = PCI_CLK_8M;

would mean we use 8MHz even at the time the MMC core stack asks us to only
use 5MHz, whcih is a bug!

I have fixed this in my git tree now.

> >> + mmc->caps = MMC_CAP_4_BIT_DATA;
> > You also need to set the bits stating if the controller supports the
> > high-speed SD and/or MMC mode, otherwise the core won't go over 25/20 MHz.
> OK, we will add the following line in the via_init_mmc_host function:

Since I cannot see any indication from the VX800 / VX855 data sheet that HS-MMC
is actually supported, I only use SD_HIGHSPEED for now.

Joseph: Can you find out if the controller actually supports HS-MMC, not only
SD highspeed?

> org_data = readl(addrbase + VIA_CRDR_SDEXTCTRL);
> if ((ios->timing == MMC_TIMING_SD_HS) || (ios->timing == MMC_TIMING_MMC_HS))
> else
> writel(org_data, addrbase + VIA_CRDR_SDEXTCTRL);

tried that, unfortunately this is buggy, since org_data refers to SDBUSMODE rather than SDEXTCTRL. I've fixed it in this patch:;a=commitdiff;h=bb1f9f2d9d51bbf294ae036d02e2546f2d8b36c3

The resulting driver runs so far fine on my VX855ES based board, but I'll give
it some more testing before re-submitting the patch.

There is also one case in the code where due to a DMA controller bug, we force
the clock to 8MHz despite the MMC core requesting 375kHz. I think the proper
fix here is to fall back on PIO (non-DMA) mode for 375kHz, rather than
overclocking the poor card by more than 21 times.

After that has been fixed, plus some more testing, I'll re-submit.

- Harald Welte <>
VIA Open Source Liaison

 \ /
  Last update: 2009-05-25 05:13    [from the cache]
©2003-2011 Jasper Spaans