On Thu, Feb 16, 2006 at 10:27:22AM -0400, Carlos Aguiar wrote:
> + /* Any data transfer means adtc type (but that information is not
> + * in command structure, so we flagged it into host struct.)
> + * However, telling bc, bcr and ac apart based on response is
> + * not foolproof:
> + * CMD0 = bc = resp0 CMD15 = ac = resp0
> + * CMD2 = bcr = resp2 CMD10 = ac = resp2
> + *
> + * Resolve to best guess with some exception testing:
> + * resp0 -> bc, except CMD15 = ac
> + * rest are ac, except if opendrain
> + */
> + if (host->data) {
> + cmdtype = OMAP_MMC_CMDTYPE_ADTC;
> + } else if (resptype == 0 && cmd->opcode != 15) {
> + cmdtype = OMAP_MMC_CMDTYPE_BC;
> + } else if (host->bus_mode == MMC_BUSMODE_OPENDRAIN) {
> + cmdtype = OMAP_MMC_CMDTYPE_BCR;
> + } else {
> + cmdtype = OMAP_MMC_CMDTYPE_AC;
> + }
This is no longer necessary - we now supply the command type via
cmd->flags.
> +static void
> +mmc_omap_cmd_done(struct mmc_omap_host *host, struct mmc_command *cmd)
> +{
> + host->cmd = NULL;
> +
if (cmd->flags & MMC_RSP_PRESENT) {
if (cmd->flags & MMC_RSP_136) {
cmd->resp[3] =
OMAP_MMC_READ(host->base, RSP0) |
(OMAP_MMC_READ(host->base, RSP1) << 16);
cmd->resp[2] =
OMAP_MMC_READ(host->base, RSP2) |
(OMAP_MMC_READ(host->base, RSP3) << 16);
cmd->resp[1] =
OMAP_MMC_READ(host->base, RSP4) |
(OMAP_MMC_READ(host->base, RSP5) << 16);
cmd->resp[0] =
OMAP_MMC_READ(host->base, RSP6) |
(OMAP_MMC_READ(host->base, RSP7) << 16);
} else {
cmd->resp[0] =
OMAP_MMC_READ(host->base, RSP6) |
(OMAP_MMC_READ(host->base, RSP7) << 16);
}
}
would be clearer, and probably more efficient. (gcc isn't really
that good at working out bit patterns from switch statements.)
> + /* 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);
> + }
readsw / writesw would be a far more efficient implementation.
> + host->base = (void __iomem *)pdev->resource[0].start;
This is still very very very very wrong. MMIO resources do not contain
virtual addresses. Casting it to a virtual address like this is insane.
> + mmc_add_host(mmc);
At this point the host becomes live. Is the following initialisation
safe to do after the host has been manipulated to talk to MMC cards?
> +
> + if (host->switch_pin >= 0) {
> + INIT_WORK(&host->switch_work, mmc_omap_switch_handler, host);
> + init_timer(&host->switch_timer);
> + host->switch_timer.function = mmc_omap_switch_timer;
> + host->switch_timer.data = (unsigned long) host;
> + if (omap_request_gpio(host->switch_pin) != 0) {
> + printk(KERN_WARNING "MMC%d: Unable to get GPIO pin for MMC cover switch\n",
> + host->id);
> + host->switch_pin = -1;
> + goto no_switch;
> + }
> +
> + omap_set_gpio_direction(host->switch_pin, 1);
> + set_irq_type(OMAP_GPIO_IRQ(host->switch_pin), IRQT_RISING);
> + ret = request_irq(OMAP_GPIO_IRQ(host->switch_pin),
> + mmc_omap_switch_irq, 0, DRIVER_NAME, host);
Don't use set_irq_type, use request_irq with SA_TRIGGER_RISING please.
> + if (host) {
> + mmc_remove_host(host->mmc);
> + free_irq(host->irq, host);
> +#ifdef CONFIG_I2C
> + mmc_omap_power(host, 0);
> +#endif
Power should be turned off by the MMC layer anyway.
> +#ifdef CONFIG_PM
> +static int mmc_omap_suspend(struct platform_device *pdev, pm_message_t mesg)
> +{
> + int ret = 0;
> + struct mmc_omap_host *host = platform_get_drvdata(pdev);
> +
> + if (host && host->suspended)
> + return 0;
> +
> + if (!irqs_disabled())
> + return -EAGAIN;
IRQs will never be disabled here.
--
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
