lkml.org 
[lkml]   [2006]   [Feb]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [RFC] mmc: add OMAP driver
    On Fri, Feb 24, 2006 at 12:38:24PM -0400, Carlos Aguiar wrote:
    > All suggestions and comments are welcome.

    Okay, further comments. Almost there!

    > +/* PIO only */
    > +static void
    > +mmc_omap_xfer_data(struct mmc_omap_host *host, int write)
    > +{
    > + int n;
    > + void __iomem *reg;
    > + u16 *p;
    > +
    > + if (host->buffer_bytes_left == 0) {
    > + host->sg_idx++;
    > + BUG_ON(host->sg_idx == host->sg_len);
    > + mmc_omap_sg_to_buf(host);
    > + }
    > + n = 64;
    > + if (n > host->buffer_bytes_left)
    > + n = host->buffer_bytes_left;
    > + host->buffer_bytes_left -= n;
    > + host->total_bytes_left -= n;
    > + host->data->bytes_xfered += n;
    > +
    > + /* Optimize the loop a bit by calculating the register only
    > + * once */
    > + reg = host->base + OMAP_MMC_REG_DATA;
    > + p = host->buffer;
    > + n /= 2;
    > + if (write) {
    > + while (n--)
    > + __raw_writew(*p++, reg);
    > + } else {
    > + while (n-- > 0)
    > + *p++ = __raw_readw(reg);
    > + }

    I thought I made a comment about readsw/writesw being more efficient
    versions of these?

    > + host->buffer = p;
    > +}

    > +static irqreturn_t mmc_omap_irq(int irq, void *dev_id, struct pt_regs *regs)
    > +{
    > + struct mmc_omap_host * host = (struct mmc_omap_host *)dev_id;
    > + u16 status;
    > + int end_command;
    > + int end_transfer;
    > + int transfer_error;
    > +
    > + if (host->cmd == NULL && host->data == NULL) {
    > + status = OMAP_MMC_READ(host->base, STAT);
    > + printk(KERN_INFO "MMC%d: Spurious interrupt 0x%04x\n",
    > + host->id, status);

    dev_info(mmc_dev(host->mmc), "spurious irq 0x%04x\n", status);

    There's no need to print host->id - mmc_dev(host->mmc) will print the
    platform device bus_id, which is generated from the platform device
    name and the platform device id.

    > + if (status != 0) {
    > + OMAP_MMC_WRITE(host->base, STAT, status);
    > + OMAP_MMC_WRITE(host->base, IE, 0);
    > + }
    > + return IRQ_HANDLED;
    > + }
    > +
    > + end_command = 0;
    > + end_transfer = 0;
    > + transfer_error = 0;
    > +
    > + while ((status = OMAP_MMC_READ(host->base, STAT)) != 0) {
    > + OMAP_MMC_WRITE(host->base, STAT, status);
    > +#ifdef CONFIG_MMC_DEBUG
    > + printk(KERN_DEBUG "MMC IRQ %04x (CMD %d): ", status,
    > + host->cmd != NULL ? host->cmd->opcode : -1);

    dev_dbg(mmc_dev(host->mmc), "MMC IRQ %04x (CMD %d): ",
    status, host->cmd != NULL ? host->cmd->opcode : -1);

    > + mmc_omap_report_irq(status);
    > + printk("\n");
    > +#endif
    > + if (host->total_bytes_left) {
    > + if ((status & OMAP_MMC_STAT_A_FULL) ||
    > + (status & OMAP_MMC_STAT_END_OF_DATA))
    > + mmc_omap_xfer_data(host, 0);
    > + if (status & OMAP_MMC_STAT_A_EMPTY)
    > + mmc_omap_xfer_data(host, 1);
    > + }
    > +
    > + if (status & OMAP_MMC_STAT_END_OF_DATA) {
    > + end_transfer = 1;
    > + }
    > +
    > + if (status & OMAP_MMC_STAT_DATA_TOUT) {
    > + dev_dbg(mmc_dev(host->mmc), "MMC%d: Data timeout\n",
    > + host->id);

    dev_dbg(mmc_dev(host->mmc), "data timeout\n");

    > + if (host->data) {
    > + host->data->error |= MMC_ERR_TIMEOUT;
    > + transfer_error = 1;
    > + }
    > + }
    > +
    > + if (status & OMAP_MMC_STAT_DATA_CRC) {
    > + if (host->data) {
    > + host->data->error |= MMC_ERR_BADCRC;
    > + dev_dbg(mmc_dev(host->mmc), "MMC%d: Data CRC
    > + error, bytes left %d\n",
    > + host->id,
    > + host->total_bytes_left);

    dev_dbg(mmc_dev(host->mmc),
    "data CRC error, bytes left %d\n",
    host->total_bytes_left);

    > + transfer_error = 1;
    > + } else {
    > + dev_dbg(mmc_dev(host->mmc), "MMC%d: Data CRC
    > + error\n", host->id);

    dev_dbg(mmc_dev(host->mmc), "data CRC error\n");

    > + }
    > + }
    > +
    > + if (status & OMAP_MMC_STAT_CMD_TOUT) {
    > + /* Timeouts are routine with some commands */
    > + if (host->cmd) {
    > + if (host->cmd->opcode != MMC_ALL_SEND_CID &&
    > + host->cmd->opcode !=
    > + MMC_SEND_OP_COND &&
    > + host->cmd->opcode !=
    > + MMC_APP_CMD &&
    > + !mmc_omap_cover_is_open(host))
    > + dev_err(mmc_dev(host->mmc), "MMC%d:
    > + Command timeout,
    > + CMD%d\n", host->id,
    > + host->cmd->opcode);

    dev_err(mmc_dev(host->mmc),
    "command timeout, CMD %d\n",
    host->cmd->opcode);

    > + host->cmd->error = MMC_ERR_TIMEOUT;
    > + end_command = 1;
    > + }
    > + }
    > +
    > + if (status & OMAP_MMC_STAT_CMD_CRC) {
    > + if (host->cmd) {
    > + dev_err(mmc_dev(host->mmc), "MMC%d: Command CRC
    > + error (CMD%d, arg 0x%08x)\n",
    > + host->id, host->cmd->opcode,
    > + host->cmd->arg);

    dev_err(mmc_dev(host->mmc),
    "command CRC error (CMD%d, arg 0x%08x)\n",
    host->cmd->opcode, host->cmd->arg);

    > + host->cmd->error = MMC_ERR_BADCRC;
    > + end_command = 1;
    > + } else
    > + dev_err(mmc_dev(host->mmc), "MMC%d: Command CRC
    > + error without cmd?\n",
    > + host->id);

    dev_err(mmc_dev(host->mmc),
    "command CRC error without cmd?\n");

    > + }
    > +
    > + if (status & OMAP_MMC_STAT_CARD_ERR) {
    > + if (host->cmd && host->cmd->opcode == MMC_STOP_TRANSMISSION) {
    > + u32 response = OMAP_MMC_READ(host->base, RSP6)
    > + | (OMAP_MMC_READ(host->base, RSP7) << 16);
    > + /* STOP sometimes sets must-ignore bits */
    > + if (!(response & (R1_CC_ERROR
    > + | R1_ILLEGAL_COMMAND
    > + | R1_COM_CRC_ERROR))) {
    > + end_command = 1;
    > + continue;
    > + }
    > + }
    > +
    > + dev_dbg(mmc_dev(host->mmc), "MMC%d: Card status error (CMD%d)\n",
    > + host->id, host->cmd->opcode);

    dev_dbg(mmc_dev(host->mmc), "card status error (CMD%d)\n",
    host->cmd->opcode);

    > + if (host->cmd) {
    > + host->cmd->error = MMC_ERR_FAILED;
    > + end_command = 1;
    > + }
    > + if (host->data) {
    > + host->data->error = MMC_ERR_FAILED;
    > + transfer_error = 1;
    > + }
    > + }
    > +
    > + /*
    > + * NOTE: On 1610 the END_OF_CMD may come too early when
    > + * starting a write
    > + */
    > + if ((status & OMAP_MMC_STAT_END_OF_CMD) &&
    > + (!(status & OMAP_MMC_STAT_A_EMPTY))) {
    > + end_command = 1;
    > + }
    > + }
    > +
    > + if (end_command) {
    > + mmc_omap_cmd_done(host, host->cmd);
    > + }
    > + if (transfer_error)
    > + mmc_omap_xfer_done(host, host->data);
    > + else if (end_transfer)
    > + mmc_omap_end_of_data(host, host->data);
    > +
    > + return IRQ_HANDLED;
    > +}

    Same comments for the other dev_* and printk's.

    > +static int __init mmc_omap_probe(struct platform_device *pdev)
    > +{
    > + struct omap_mmc_conf *minfo = pdev->dev.platform_data;
    > + struct mmc_host *mmc;
    > + struct mmc_omap_host *host = NULL;
    > + int ret = 0;
    > +
    > + if (platform_get_resource(pdev, IORESOURCE_MEM, 0) ||
    > + platform_get_irq(pdev, IORESOURCE_IRQ, 0)) {
    > + dev_err(&pdev->dev, "mmc_omap_probe: invalid resource type\n");
    > + return -ENODEV;
    > + }
    > +
    > + if (!request_mem_region(pdev->resource[0].start,
    > + pdev->resource[0].end - pdev->resource[0].start + 1,
    > + pdev->name)) {
    > + dev_dbg(&pdev->dev, "request_mem_region failed\n");
    > + return -EBUSY;
    > + }
    > +
    > + mmc = mmc_alloc_host(sizeof(struct mmc_omap_host), &pdev->dev);
    > + if (!mmc) {
    > + ret = -ENOMEM;
    > + goto out;
    > + }
    > +
    > + host = mmc_priv(mmc);
    > + host->mmc = mmc;
    > +
    > + spin_lock_init(&host->dma_lock);
    > + init_timer(&host->dma_timer);
    > + host->dma_timer.function = mmc_omap_dma_timer;
    > + host->dma_timer.data = (unsigned long) host;
    > +
    > + host->id = pdev->id;
    > +
    > + if (cpu_is_omap24xx()) {
    > + host->iclk = clk_get(&pdev->dev, "mmc_ick");
    > + if (IS_ERR(host->iclk))
    > + goto out;
    > + clk_enable(host->iclk);
    > + }
    > +
    > + if (!cpu_is_omap24xx())
    > + host->fclk = clk_get(&pdev->dev, "mmc_ck");
    > + else
    > + host->fclk = clk_get(&pdev->dev, "mmc_fck");
    > +
    > + if (IS_ERR(host->fclk)) {
    > + ret = PTR_ERR(host->fclk);
    > + goto out;
    > + }
    > +
    > + /* REVISIT:
    > + * Also, use minfo->cover to decide how to manage
    > + * the card detect sensing.
    > + */
    > + host->power_pin = minfo->power_pin;
    > + host->switch_pin = minfo->switch_pin;
    > + host->wp_pin = minfo->wp_pin;
    > + host->use_dma = 1;
    > + host->dma_ch = -1;
    > +
    > + host->irq = pdev->resource[1].start;
    > + host->base = ioremap(pdev->res.start, SZ_4K);

    What if ioremap fails with NULL?

    > +
    > + if (minfo->wire4)
    > + mmc->caps |= MMC_CAP_4_BIT_DATA;
    > +
    > + mmc->ops = &mmc_omap_ops;
    > + mmc->f_min = 400000;
    > + mmc->f_max = 24000000;
    > + mmc->ocr_avail = MMC_VDD_33_34;

    Some cards want at least two bits set - use MMC_VDD_32_33|MMC_VDD_33_34
    here please.


    --
    Russell King
    Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
    maintainer of: 2.6 Serial core
    -
    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: 2006-02-24 21:52    [W:0.042 / U:1.052 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site