linux-kernel - Re: [PATCH 05/18] MMC: OMAP: Introduce new multislot structure and change driver to use it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20080207183744.1491c995@poseidon.drzeus.cx>
Date:	Thu, 7 Feb 2008 18:37:44 +0100
From:	Pierre Ossman <drzeus-list@...eus.cx>
To:	Carlos Aguiar <carlos.aguiar@...t.org.br>
Cc:	Tony Lindgren <tony@...mide.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/18] MMC: OMAP: Introduce new multislot structure and
 change driver to use it

On Mon, 28 Jan 2008 15:07:23 -0400
Carlos Aguiar <carlos.aguiar@...t.org.br> wrote:

> From: Juha Yrjola <juha.yrjola@...idboot.com>
> 
> Introduce new MMC multislot structure and change driver to use it.
> 
> Note that MMC clocking is now enabled in mmc_omap_select_slot()
> and disabled in mmc_omap_release_slot().
> 
> Signed-off-by: Juha Yrjola <juha.yrjola@...idboot.com>
> Signed-off-by: Jarkko Lavinen <jarkko.lavinen@...ia.com>
> Signed-off-by: Carlos Eduardo Aguiar <carlos.aguiar@...t.org.br>
> Signed-off-by: Tony Lindgren <tony@...mide.com>
> ---

I still think this muxed mmc host thing is a bad idea, but it's your nightmare...

> +
> +/* Access to the R/O switch is required for production testing
> + * purposes. */
> +static ssize_t
> +mmc_omap_show_ro(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> +	struct mmc_host *mmc = container_of(dev, struct mmc_host, class_dev);
> +	struct mmc_omap_slot *slot = mmc_priv(mmc);
> +
> +	return sprintf(buf, "%d\n", slot->pdata->get_ro(mmc_dev(mmc),
> +							slot->id));
> +}
> +
> +static DEVICE_ATTR(ro, S_IRUGO, mmc_omap_show_ro, NULL);
> +

This is unrelated to the slot stuff and should be in its own patch. Also, it should probably be in the core, not a driver.

> +
> +	mmc->caps = MMC_CAP_MULTIWRITE | MMC_CAP_MMC_HIGHSPEED |
> +		    MMC_CAP_SD_HIGHSPEED;

This is also unrelated. From what I've seen, the OMAP is a SD controller and does not support high speed MMC. The fact that you also conditionally set the max frequency later also suggests that this code is entirely incorrect.

> +
> +	r = mmc_add_host(mmc);
> +	if (r < 0)
> +		return r;
> +
> +	if (slot->pdata->name != NULL) {
> +		r = device_create_file(&mmc->class_dev,
> +					&dev_attr_slot_name);
> +		if (r < 0)
> +			goto err_remove_host;
> +	}
> +
> +	if (slot->pdata->get_ro != NULL) {
> +		r = device_create_file(&mmc->class_dev,
> +					&dev_attr_ro);
> +	}
> +

You have a bit of a race here with userspace in case you use the uevent to trigger things.

-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/