| Message ID | 1411502430-25535-6-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 23 September 2014 22:00, Adrian Hunter <adrian.hunter@intel.com> wrote: > Nowhere in the SD Association Specifications does > it state that the stop command has an R1 response > type. It is always R1B. Change accordingly. It depends on how detailed you read the spec. :-) First, R1B is the same as R1, but with optional busy signalling on DAT0. Just reading the table listing CMDS their related response types, confirms what you are saying. CMD12 has an R1B. Though, going into the details of the "Timing" section where this is clearly visualized in diagrams, you realize there are no busy signalling associated with a DATA READ, only for DATA WRITE. It is also indicated in earlier sections of the spec when "DATA READ/WRITE sequences are described", but I think the "Timing" section describes this the best. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 25/09/14 12:20, Ulf Hansson wrote: > On 23 September 2014 22:00, Adrian Hunter <adrian.hunter@intel.com> wrote: >> Nowhere in the SD Association Specifications does >> it state that the stop command has an R1 response >> type. It is always R1B. Change accordingly. > > It depends on how detailed you read the spec. :-) > > First, R1B is the same as R1, but with optional busy signalling on DAT0. Not exactly: "R1b is identical to R1 with an optional busy signal transmitted on the data line. The card may become busy after receiving these commands based on its state prior to the command reception. The Host shall check for busy at the response. Refer to Section 4.12.3 for a detailed description and timing diagrams." Note it says "The Host shall check for busy at the response." It does not say "The Host is not affected" > > Just reading the table listing CMDS their related response types, > confirms what you are saying. CMD12 has an R1B. Which is explicit and definitive. > Though, going into the details of the "Timing" section where this is > clearly visualized in diagrams, you realize there are no busy > signalling associated with a DATA READ, only for DATA WRITE. It is > also indicated in earlier sections of the spec when "DATA READ/WRITE > sequences are described", but I think the "Timing" section describes > this the best. You are looking at it only from the point of view of the card. The host controller can expect that CMD12/R1b is the only valid combination because that is what the specification defines. You can't know what the host controller will do if you tell it to do CMD12/R1 because that is outside the spec. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 September 2014 13:21, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 25/09/14 12:20, Ulf Hansson wrote: >> On 23 September 2014 22:00, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> Nowhere in the SD Association Specifications does >>> it state that the stop command has an R1 response >>> type. It is always R1B. Change accordingly. >> >> It depends on how detailed you read the spec. :-) >> >> First, R1B is the same as R1, but with optional busy signalling on DAT0. > > Not exactly: > > "R1b is identical to R1 with an optional busy signal > transmitted on the data line. The card may become > busy after receiving these commands based on its > state prior to the command reception. The Host shall > check for busy at the response. Refer to Section 4.12.3 > for a detailed description and timing diagrams." > > Note it says "The Host shall check for busy at the response." > It does not say "The Host is not affected" Sorry, I was a bit unclear. I was referring to the format of the response. > >> >> Just reading the table listing CMDS their related response types, >> confirms what you are saying. CMD12 has an R1B. > > Which is explicit and definitive. > >> Though, going into the details of the "Timing" section where this is >> clearly visualized in diagrams, you realize there are no busy >> signalling associated with a DATA READ, only for DATA WRITE. It is >> also indicated in earlier sections of the spec when "DATA READ/WRITE >> sequences are described", but I think the "Timing" section describes >> this the best. > > You are looking at it only from the point of view of the card. The host > controller can expect that CMD12/R1b is the only valid combination > because that is what the specification defines. You can't know what > the host controller will do if you tell it to do CMD12/R1 because that > is outside the spec. > It doesn't matter from what point of view we look at it. It's all about the details of the spec, which tells us there are no busy signalling involved with a READ. HW designers of MMC controllers should know this as well. Unless you really are fixing a regression here, I can't see the need for your patch. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/09/14 15:09, Ulf Hansson wrote: > On 30 September 2014 13:21, Adrian Hunter <adrian.hunter@intel.com> wrote: >> On 25/09/14 12:20, Ulf Hansson wrote: >>> On 23 September 2014 22:00, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>> Nowhere in the SD Association Specifications does >>>> it state that the stop command has an R1 response >>>> type. It is always R1B. Change accordingly. >>> >>> It depends on how detailed you read the spec. :-) >>> >>> First, R1B is the same as R1, but with optional busy signalling on DAT0. >> >> Not exactly: >> >> "R1b is identical to R1 with an optional busy signal >> transmitted on the data line. The card may become >> busy after receiving these commands based on its >> state prior to the command reception. The Host shall >> check for busy at the response. Refer to Section 4.12.3 >> for a detailed description and timing diagrams." >> >> Note it says "The Host shall check for busy at the response." >> It does not say "The Host is not affected" > > Sorry, I was a bit unclear. I was referring to the format of the response. > >> >>> >>> Just reading the table listing CMDS their related response types, >>> confirms what you are saying. CMD12 has an R1B. >> >> Which is explicit and definitive. >> >>> Though, going into the details of the "Timing" section where this is >>> clearly visualized in diagrams, you realize there are no busy >>> signalling associated with a DATA READ, only for DATA WRITE. It is >>> also indicated in earlier sections of the spec when "DATA READ/WRITE >>> sequences are described", but I think the "Timing" section describes >>> this the best. >> >> You are looking at it only from the point of view of the card. The host >> controller can expect that CMD12/R1b is the only valid combination >> because that is what the specification defines. You can't know what >> the host controller will do if you tell it to do CMD12/R1 because that >> is outside the spec. >> > > It doesn't matter from what point of view we look at it. It's all > about the details of the spec, which tells us there are no busy > signalling involved with a READ. HW designers of MMC controllers > should know this as well. HW designers may well choose to follow the spec. > > Unless you really are fixing a regression here, I can't see the need > for your patch. No, I have a host controller that wants to give a TC interrupt on CMD12 which is correct if the response type is R1b but incorrect if the response type is R1. However I am anyway fixing that with a quirk because theoretically MMC is affected too - although not in practice because it uses CMD23 instead of CMD12. That got me thinking that we really ought to follow the spec and use R1b. -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30 September 2014 14:41, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 30/09/14 15:09, Ulf Hansson wrote: >> On 30 September 2014 13:21, Adrian Hunter <adrian.hunter@intel.com> wrote: >>> On 25/09/14 12:20, Ulf Hansson wrote: >>>> On 23 September 2014 22:00, Adrian Hunter <adrian.hunter@intel.com> wrote: >>>>> Nowhere in the SD Association Specifications does >>>>> it state that the stop command has an R1 response >>>>> type. It is always R1B. Change accordingly. >>>> >>>> It depends on how detailed you read the spec. :-) >>>> >>>> First, R1B is the same as R1, but with optional busy signalling on DAT0. >>> >>> Not exactly: >>> >>> "R1b is identical to R1 with an optional busy signal >>> transmitted on the data line. The card may become >>> busy after receiving these commands based on its >>> state prior to the command reception. The Host shall >>> check for busy at the response. Refer to Section 4.12.3 >>> for a detailed description and timing diagrams." >>> >>> Note it says "The Host shall check for busy at the response." >>> It does not say "The Host is not affected" >> >> Sorry, I was a bit unclear. I was referring to the format of the response. >> >>> >>>> >>>> Just reading the table listing CMDS their related response types, >>>> confirms what you are saying. CMD12 has an R1B. >>> >>> Which is explicit and definitive. >>> >>>> Though, going into the details of the "Timing" section where this is >>>> clearly visualized in diagrams, you realize there are no busy >>>> signalling associated with a DATA READ, only for DATA WRITE. It is >>>> also indicated in earlier sections of the spec when "DATA READ/WRITE >>>> sequences are described", but I think the "Timing" section describes >>>> this the best. >>> >>> You are looking at it only from the point of view of the card. The host >>> controller can expect that CMD12/R1b is the only valid combination >>> because that is what the specification defines. You can't know what >>> the host controller will do if you tell it to do CMD12/R1 because that >>> is outside the spec. >>> >> >> It doesn't matter from what point of view we look at it. It's all >> about the details of the spec, which tells us there are no busy >> signalling involved with a READ. HW designers of MMC controllers >> should know this as well. > > HW designers may well choose to follow the spec. > >> >> Unless you really are fixing a regression here, I can't see the need >> for your patch. > > No, I have a host controller that wants to give a TC interrupt on CMD12 > which is correct if the response type is R1b but incorrect if the > response type is R1. However I am anyway fixing that with a quirk because > theoretically MMC is affected too - although not in practice because it uses > CMD23 instead of CMD12. > > That got me thinking that we really ought to follow the spec and use R1b. I will certainly keep this in mind. Likely a similar situation will occur when I fixup mmc erase/discard. In principle host drivers must pay attention to MMC_RSP_R1B and act accordingly both if set and not set. I suppose you will add some extra quirks around that in your driver. Anyway, thanks for the discussion, it was useful! Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c index c3770dd..0736efb 100644 --- a/drivers/mmc/card/block.c +++ b/drivers/mmc/card/block.c @@ -801,6 +801,9 @@ static int send_stop(struct mmc_card *card, unsigned int timeout_ms, if (host->max_busy_timeout && (timeout_ms > host->max_busy_timeout)) use_r1b_resp = false; + if (!mmc_card_mmc(card)) + use_r1b_resp = true; + cmd.opcode = MMC_STOP_TRANSMISSION; if (use_r1b_resp) { cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; @@ -1436,9 +1439,14 @@ static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq, if (rq_data_dir(req) == READ) { brq->cmd.opcode = readcmd; brq->data.flags |= MMC_DATA_READ; - if (brq->mrq.stop) - brq->stop.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | - MMC_CMD_AC; + if (brq->mrq.stop) { + if (mmc_card_mmc(card)) + brq->stop.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | + MMC_CMD_AC; + else + brq->stop.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1B | + MMC_CMD_AC; + } } else { brq->cmd.opcode = writecmd; brq->data.flags |= MMC_DATA_WRITE;
Nowhere in the SD Association Specifications does it state that the stop command has an R1 response type. It is always R1B. Change accordingly. Note that, for SD cards, this puts the situation back to what it was prior to commit bcc3e1726d827c2d6f62f0e0e7bbc99eed7ad925. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- drivers/mmc/card/block.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)