On Thursday 16 April 2009, Bhandari, Vipin wrote: > > Not sending those 70-odd clocks seems, at least, unwise. > > Please refer to function mmc_davinci_set_ios() and inside > the if statement for ios->power_mode == MMC_POWER_UP. > 70 odd clocks are send here as you mentioned in point a.
And so they are. Good point; ignore that comment of mine. :) > > > > - if (!host->do_dma && (host->data_dir == > > > > DAVINCI_MMC_DATADIR_WRITE)) > > > > - davinci_fifo_data_trans(host, 32); > > > > + if (!host->do_dma && (host->data_dir == > > > > DAVINCI_MMC_DATADIR_WRITE) > > && > > > > + ((cmd->opcode == MMC_WRITE_BLOCK) || > > > > + (cmd->opcode == MMC_WRITE_MULTIPLE_BLOCK))) > > > > + davinci_fifo_data_trans(host, > > > > DAVINCI_MMC_FIFO_SIZE_BYTE); > > > > > > > > writel(cmd->arg, host->base + DAVINCI_MMCARGHL); > > > > writel(cmd_reg, host->base + DAVINCI_MMCCMD); > > > > That confuses me, and seems like it goes against the standard "host > > adapters > > should not care about specific commands" policy. Won't it prevent SDIO > > from > > working, for example? Would a simple "host->data != NULL" address the > > same > > issue? Can DATADIR_WRITE even be set to that value for a non-write > > command? > > > > There are non-MMC/non-SD write commands... > > Please refer to the TRM of MMCSD which states as follow > xxxxxxxxxxxxxxxxxxxx > When starting the write transaction, the CPU is responsible for getting > the FIFO ready to start transferring data by filling up the FIFO with > data prior to invoking/posting the write command to the card. Filling > up the FIFO is a requirement since no interrupt/event generates at the > start of the write transfer. > xxxxxxxxxxxxxxxxxxxx > This is required only in case of non dma based transfers and that too > during write operation. Exactly. But there are more write transactions than those two; they just happen to be some of the most common ones. And from what I could see, the existing code already covered all of the cases: all write transactions (i.e. everything that sends a DATA stage to the card, using DAT0..DATx) was filling the fifo. Although there does seem to be a bug there ... it should never fill the fifo with more data than is being sent. > > > > > - /* record how much data we transferred */ > > > > - temp = readl(host->base + DAVINCI_MMCNBLC); > > > > - data->bytes_xfered += (data->blocks - temp) * data->blksz; > > > > - > > > > /* reset command and data state machines */ > > > > temp = readl(host->base + DAVINCI_MMCCTL); > > > > writel(temp | MMCCTL_CMDRST | MMCCTL_DATRST, > > > > This doesn't look right ... doesn't it introduce misbehavior > > in some of the "mmctest" cases? It's at least under-reporting > > the number of bytes transferred (and needlessly so). > > This function is an abort data call which happens during timeout > or crc errors. There is no way to tell how much of the data has > been transferred, as it is not acknowledged. Well, by *observation* that register gives the correct answer. Its value seems to get decremented by the ACK ... as implied by the documentation. > Putting data->bytes_xfered += 0; seems to be right but as it > should have been already 0 initially so not reqd. to update it again. Adding zero is always pointless. ;) - Dave _______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
