On Fri, Apr 8, 2011 at 4:25 PM, John Calixto
<john.calixto@xxxxxxxxxxxxxx> wrote:
> Hi Andrei,
>
> On Fri, 8 Apr 2011, Andrei Warkentin wrote:
>> 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?
>
> Actually, ASSD is a different-but-similarly-named-so-it's-confusing set
> of functionality from that which I am using. Do you have access to
> "Part 3, Security Specification"? There is no "simplified" version of
> this spec.
>
> As for the usage of the force_timeout_ns, yes, it is also used for
> DAT-transfer commands.
>
>>
>> Do you always invoke commands that only involve transfers?
>
> No.
>
>> 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
>
> I'll have a look.
>
>> 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;
>> }
>>
>>
>
> Sure. Like you said, this sounds reasonable while there's still
> variation in timeout setting between the host drivers.
>
>> >
>> > 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
>
> Unfortunately, I really do mean "read". I suspect it's meant to give
> the card enough time to do the crypto <shrug>.
>
>> 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?
>> >>
>
> I can't quote directly from this spec (it would be nice if some other
> SDA member could corroborate here... Part 3, anybody???), but it is
> clear that there is nothing to do but wait. However, speaking to your
> initial question about usleep() vs. udelay(), I'll try testing with
> usleep().
>
> John
I'm not sure what the etiquette for this list is, but you can a
Reviewed-by: Andrei Warkentin <andreiw@xxxxxxxxxxxx>
...if that's alright with the maintainers.
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
