Hi John,
On Fri, Apr 8, 2011 at 2:20 PM, John Calixto
<john.calixto@xxxxxxxxxxxxxx> wrote:
> Hi Andrei,
>
> On Fri, 8 Apr 2011, Andrei Warkentin wrote:
>
>> Hi John,
>>
>> On Thu, Apr 7, 2011 at 8:18 PM, John Calixto
>> <john.calixto@xxxxxxxxxxxxxx> wrote:
>> > + /* data.flags must already be set before doing this. */
>> > + mmc_set_data_timeout(&data, card);
>> > + /* Allow overriding the timeout_ns for empirical tuning. */
>> > + if (idata->ic.force_timeout_ns)
>> > + data.timeout_ns = idata->ic.force_timeout_ns;
>> > +
>>
>> Are there any R1B ACMDs?
>>
>
> Yes, there are. Do you think this timeout is unnecessary? We found it
> useful to adjust this to be more accepting of some cheaper cards that
> exceeded the timeout in the spec (250 ms).
Is the timeout fixed or variable (I've seen all timeouts in the
simplified ASSD to be specified in units of 250ms in fields of
ASSD-PR)? Are you using this force_timeout_ns for DAT-transfer
commands also, or just for R1B commands?
Do you always invoke commands that only involve transfers? If not, I
can see how your code handles R1B command timeouts correctly (always
pretends it's a data transfer, even if 0-byte sized - thus host driver
sets timeout correctly). In my boot partition patch set, there are two
changes that add cmd_timeout (for DAT-busy commands) and enable proper
handling of it for SDHCI. Obviously, none of the other hosts are
fixed, so I guess what you are doing is okay for now. Could you add a
"cmd_timeout" field to your ioctl, however, so in the future this is
not a breaking change? Then right now you could add something like -
if (blksz * blocks == 0) {
/*
* We're doing a R1B command and relying on the host driver to
compute timeout as if
* we were doing a data transfer. When all host drivers are fixed
to handle R1B right, we won't have to rely
* on this behavior, and can make mrq.data NULL in this case, and
pass the timeout
* through cmd.cmd_timeout.
*/
data.timeout_ns = idata->cmd_timeout;
}
and in the future it can be changed to -
if (blksz * blocks == 0) {
mrq.data = NULL;
cmd.cmd_timeout = idata->cmd_timeout;
}
>
> In practice, 100 us. Borderline? Note that this sleep is separate from
> the timeout above. This one is specifically for read operations where
> there is no such thing as "busy".
>
You mean 'write'? I don't know. Nasty, really, in my opinion. If
you're doing ASSD operations then you know if the card is busy or not
(via ASSD-state) . As in something like this paragraph from the
simplified ASSD?
"The card shall not assert the BUSY signal for time periods
longer then the WR_SEC_BUS_BUSY as
defined in the card properties register. The only indication
for completion of a secure Token execution is
the ASSD_STATE field in the card status register. The host
shall validate that the card is in the correct
state before attempting any ASSD command."
If so, maybe it's worthwhile to just make your user code use SEND_PSI
to get the ASSD_STATE registers and verify that previous secure
operation completed before sending another one.
Anyone else have an opinion on this?
If this is not ASSD but something else, maybe you have a status
register to query someplace?
>> I'm not sure I understand why blksz is passed. If you needed to set
>> the block size (via CMD16) prior to some command (like MMC password
>> protection) I could kind of get it, but then all I see the blk size
>> getting used for is calculating the data amount in bytes, so you're
>> not doing that. So why not just infer it from the card queue?
>>
>
> The block size for some commands is strictly specified, and does
> change according to the command. In these cases, you do not call CMD16
> to change it. You just call the command knowing that its block size is
> fixed according to spec.
>
Ah okay.
A
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
