From: Pierre Ossman <drzeus-mmc-p3sGCRWkH8CeZLLa646FqQ@public.gmane.org>
To: David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
Cc: Hans-Peter Nilsson
<hans-peter.nilsson-VrBV9hrLPhE@public.gmane.org>,
Mikael Starvik <mikael.starvik-VrBV9hrLPhE@public.gmane.org>,
Mike Lavender
<mike-UTnDXsALFwNjMdQLN6DIHgC/G2K4zDHf@public.gmane.org>,
spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: Re: [patch 2.6.22-git5 3/4] MMC core learns about SPI
Date: Thu, 26 Jul 2007 19:18:24 +0200 [thread overview]
Message-ID: <20070726191824.569542fd@poseidon.drzeus.cx> (raw)
In-Reply-To: <200707141507.17484.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
On Sat, 14 Jul 2007 15:07:16 -0700
David Brownell <david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org> wrote:
> @@ -541,7 +548,9 @@ void mmc_rescan(struct work_struct *work
>
> err = mmc_send_app_op_cond(host, 0, &ocr);
> if (err == MMC_ERR_NONE) {
> - if (mmc_attach_sd(host, ocr))
> + if (mmc_host_is_spi(host))
> + err = mmc_spi_read_ocr(host, &ocr);
> + if (err != MMC_ERR_NONE || mmc_attach_sd(host, ocr))
> mmc_power_off(host);
> } else {
> /*
Getting the OCR is not vital to determine if it's SD or MMC, so we can move that into mmc.c and sd.c, reducing the code in here.
> @@ -550,7 +559,10 @@ void mmc_rescan(struct work_struct *work
> */
> err = mmc_send_op_cond(host, 0, &ocr);
> if (err == MMC_ERR_NONE) {
> - if (mmc_attach_mmc(host, ocr))
> + if (mmc_host_is_spi(host))
> + err = mmc_spi_read_ocr(host, &ocr);
> + if (err != MMC_ERR_NONE
> + || mmc_attach_mmc(host, ocr))
> mmc_power_off(host);
> } else {
> mmc_power_off(host);
This will fail to initialise a high capacity MMC card. From the MMC spec:
"Without the CMD58 with bits [30:29] set as "10b" in prior to the CMD1 a higher than 2GB of density of memory will remain in Idle state forever."
> @@ -95,14 +106,18 @@ int mmc_send_op_cond(struct mmc_host *ho
>
> cmd.opcode = MMC_SEND_OP_COND;
> cmd.arg = ocr;
Argument is "None", not "Ignored", so this should be 0 for SPI.
>
> - mmc_set_data_timeout(&data, card, 0);
> + /* Note that for MMC_SEND_EXT_CSD we could set the timeout; but
> + * not for the other requests. But host->card isn't set yet!
> + */
>
This will break on native hosts as a timeout of 0 means exactly that.
> +
> +int mmc_spi_send_cid(struct mmc_host *host, u32 *cid)
> +{
> + if (!mmc_host_is_spi(host))
> + return mmc_send_cxd_native(host, 0, cid, MMC_SEND_CID);
> +
> + return mmc_send_cxd_data(host, MMC_SEND_CID, cid, 16);
> +}
> +
As this is not SPI specific (except for its current use), why not mmc_send_cid() ?
And the argument for it is the RCA (in native mode).
> +
> +/* Enabling software CRCs can be a significant (30%) performance cost,
> + * and for other reasons isn't always desired; so it can be disabled.
> + */
> +static int use_spi_crc = 1;
> +module_param(use_spi_crc, bool, 0);
> +
The *_ops.c only contain function wrappers of protocol commands, not policy. So I think this is better placed where mmc_spi_set_crc() is called.
> --- g26.orig/drivers/mmc/core/mmc.c 2007-07-14 14:47:12.000000000 -0700
> +++ g26/drivers/mmc/core/mmc.c 2007-07-14 14:47:54.000000000 -0700
> @@ -264,7 +264,13 @@ static int mmc_init_card(struct mmc_host
> /*
> * Fetch CID from card.
> */
> - err = mmc_all_send_cid(host, cid);
> + if (mmc_host_is_spi(host)) {
> + err = mmc_spi_set_crc(host);
> + if (err != MMC_ERR_NONE)
> + goto err;
> + err = mmc_spi_send_cid(host, cid);
> + } else
> + err = mmc_all_send_cid(host, cid);
> if (err != MMC_ERR_NONE)
> goto err;
>
A matter of taste, but wouldn't it be clearer if you separated out the crc bit and did that earlier, before the above comment?
> --- g26.orig/drivers/mmc/core/sd.c 2007-07-14 14:47:12.000000000 -0700
> +++ g26/drivers/mmc/core/sd.c 2007-07-14 14:47:54.000000000 -0700
> @@ -321,7 +321,13 @@ static int mmc_sd_init_card(struct mmc_h
> /*
> * Fetch CID from card.
> */
> - err = mmc_all_send_cid(host, cid);
> + if (mmc_host_is_spi(host)) {
> + err = mmc_spi_set_crc(host);
> + if (err != MMC_ERR_NONE)
> + goto err;
> + err = mmc_spi_send_cid(host, cid);
> + } else
> + err = mmc_all_send_cid(host, cid);
> if (err != MMC_ERR_NONE)
> goto err;
>
Ditto.
> --- g26.orig/drivers/mmc/core/sd_ops.c 2007-07-14 14:47:12.000000000 -0700
> +++ g26/drivers/mmc/core/sd_ops.c 2007-07-14 14:47:54.000000000 -0700
> @@ -70,6 +70,12 @@ int mmc_wait_for_app_cmd(struct mmc_host
> err = cmd->error;
> if (cmd->error == MMC_ERR_NONE)
> break;
> +
> + /* no point in retrying illegal commands! */
> + if (mmc_host_is_spi(host)) {
> + if (cmd->resp[0] & R1_SPI_ILLEGAL_COMMAND)
> + break;
> + }
> }
>
> return err;
"Illegal command" refers to the previous command sent (and failed). So this can give false negatives.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
next prev parent reply other threads:[~2007-07-26 17:18 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-07-14 22:04 [patch 2.6.22-git5 0/4] MMC-over-SPI David Brownell
[not found] ` <200707141504.51950.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-14 22:05 ` [patch 2.6.22-git5 1/4] MMC headers learn about SPI David Brownell
[not found] ` <200707141506.00262.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 16:31 ` Pierre Ossman
[not found] ` <20070726183150.024930aa-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 16:55 ` David Brownell
[not found] ` <200707260955.22967.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 18:05 ` Pierre Ossman
[not found] ` <20070726200525.6a5b7d72-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 20:08 ` David Brownell
2007-07-14 22:06 ` [patch 2.6.22-git5 2/4] MMC block learns " David Brownell
[not found] ` <200707141506.42880.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 16:33 ` Pierre Ossman
[not found] ` <20070726183339.6631243f-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 17:00 ` David Brownell
[not found] ` <200707261000.17339.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 18:06 ` Pierre Ossman
[not found] ` <20070726200639.06242858-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 20:15 ` David Brownell
2007-07-14 22:07 ` [patch 2.6.22-git5 3/4] MMC core " David Brownell
[not found] ` <200707141507.17484.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 17:18 ` Pierre Ossman [this message]
[not found] ` <20070726191824.569542fd-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 21:58 ` David Brownell
[not found] ` <200707261458.55558.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 22:22 ` David Brownell
2007-08-01 15:02 ` Pierre Ossman
[not found] ` <20070801170220.3c5ccff6-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-08-01 17:02 ` David Brownell
[not found] ` <200708011002.28801.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-08-02 12:54 ` Pierre Ossman
[not found] ` <20070802145445.1118d1e7-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-08-02 19:23 ` David Brownell
2007-07-14 22:08 ` [patch 2.6.22-git5 4/4] mmc_spi host driver David Brownell
[not found] ` <200707141508.07555.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-26 18:02 ` Pierre Ossman
[not found] ` <20070726200202.5e5dcf62-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-26 23:32 ` David Brownell
[not found] ` <200707261632.37011.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-08-01 15:12 ` Pierre Ossman
[not found] ` <20070801171217.1478267b-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-08-01 18:17 ` David Brownell
[not found] ` <200708011117.24664.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-08-02 13:06 ` Pierre Ossman
[not found] ` <20070802150615.36e073c6-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-08-02 20:34 ` David Brownell
[not found] ` <200708021334.50670.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-08-04 13:14 ` Pierre Ossman
[not found] ` <20070804151452.0efaa5a9-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-08-04 17:32 ` David Brownell
[not found] ` <200708041032.10001.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-08-04 21:32 ` Pierre Ossman
2007-08-07 12:35 ` Pierre Ossman
2007-08-01 18:40 ` David Brownell
2007-07-16 16:48 ` [patch 2.6.22-git5 0/4] MMC-over-SPI Anton Vorontsov
[not found] ` <20070716164837.GA18707-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2007-07-16 18:54 ` David Brownell
[not found] ` <200707161154.54728.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-17 15:13 ` Anton Vorontsov
[not found] ` <20070717151350.GA3752-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2007-07-17 16:11 ` David Brownell
[not found] ` <200707170911.16715.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-18 7:35 ` Jan Nikitenko
[not found] ` <469DC2BC.5070305-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2007-07-18 17:06 ` David Brownell
2007-07-18 10:00 ` Anton Vorontsov
[not found] ` <20070718100047.GA9544-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2007-07-18 14:05 ` Anton Vorontsov
2007-07-18 14:44 ` Pierre Ossman
[not found] ` <20070718164409.56ca0019-mgABNEgzgxm+PRNnhPf8W5YgPPQkE1Si@public.gmane.org>
2007-07-18 17:27 ` David Brownell
[not found] ` <200707181027.17819.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-19 16:15 ` Anton Vorontsov
[not found] ` <20070719161553.GA15756-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2007-07-19 20:28 ` David Brownell
[not found] ` <200707191328.18846.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-23 14:29 ` Anton Vorontsov
[not found] ` <20070723142923.GA28979-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2007-07-23 17:33 ` David Brownell
[not found] ` <200707231033.31717.david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org>
2007-07-24 10:23 ` Anton Vorontsov
2007-08-07 3:21 ` David Brownell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20070726191824.569542fd@poseidon.drzeus.cx \
--to=drzeus-mmc-p3sgcrwkh8cezlla646fqq@public.gmane.org \
--cc=david-b-yBeKhBN/0LDR7s880joybQ@public.gmane.org \
--cc=hans-peter.nilsson-VrBV9hrLPhE@public.gmane.org \
--cc=mikael.starvik-VrBV9hrLPhE@public.gmane.org \
--cc=mike-UTnDXsALFwNjMdQLN6DIHgC/G2K4zDHf@public.gmane.org \
--cc=spi-devel-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).