Re: [PATCH 3/4] atmel-mci: support multiple mmc slots

[Pierre Ossman]

On Mon, 22 Sep 2008 18:38:16 +0200
Haavard Skinnemoen <haavard.skinnemoen@xxxxxxxxx> wrote:

> The Atmel MCI controller can drive multiple cards through separate sets
> of pins, but only one at a time. This patch adds support for
> multiplexing access to the controller so that multiple card slots can be
> used as if they were hooked up to separate mmc controllers.
> 

This multiplexing shenanigans seems to be all the rage these days...

>  
>  static void atmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  {
> -	struct atmel_mci	*host = mmc_priv(mmc);
> +	struct atmel_mci_slot	*slot = mmc_priv(mmc);
> +	struct atmel_mci	*host = slot->host;
>  
>  	if (ios->clock) {
>  		u32 clkdiv;

You forgot the most important part; how to handle the clock given two
different requests.

(This will also get a bit more painful when/if the core starts
disabling the clock when a card is idle)

> +
> +	if (gpio_is_valid(slot->detect_pin)) {
> +		int ret;
> +
> +		setup_timer(&slot->detect_timer, atmci_detect_change,
> +				(unsigned long)slot);
> +
> +		ret = request_irq(gpio_to_irq(slot->detect_pin),
> +				atmci_detect_interrupt,
> +				IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
> +				"mmc-detect", slot);
> +		if (ret) {
> +			dev_dbg(&mmc->class_dev,
> +				"could not request IRQ %d for detect pin\n",
> +				gpio_to_irq(slot->detect_pin));
> +			gpio_free(slot->detect_pin);
> +			slot->detect_pin = -EBUSY;
> +		}
> +	}

Fall back to polling?

> +	/* We need at least one slot to succeed */
> +	ret = -1;
> +	if (pdata->slot[0].bus_width)
> +		ret &= atmci_init_slot(host, &pdata->slot[0],
> +				MCI_SDCSEL_SLOT_A);
> +	if (pdata->slot[1].bus_width)
> +		ret &= atmci_init_slot(host, &pdata->slot[1],
> +				MCI_SDCSEL_SLOT_B);
> +	if (ret) {
> +		ret = -ENODEV;
> +		goto err_init_slot;
>  	}

Memory/resource leak.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

Attachment: signature.asc
Description: PGP signature