Hello,

On 11/19/2012 11:34 PM, Per Förlin wrote:
> On 11/19/2012 03:32 PM, Per Förlin wrote:
>> On 11/19/2012 10:48 AM, Konstantin Dorfman wrote:
>>
> I'm fine with wait_event() (block request transfers) combined with completion (for other transfer), as long as the core.c API is intact. I don't see a reason for a new start_data_req()
>
> mmc_start_req() is only used by the mmc block transfers.
> Making a test case in mmc_test.c is a good way to stress test the feature (e.g. random timing of incoming new requests) and it will show how the API works too.
>
> BR
> Per
>
Right now there are couple of tests in mmc_test module that use async 
request mechanism. After applying my fix (v3 mmc: fix async request 
mechanism for sequential read scenarios), these test were broken.

The patch attached fixes those broken tests and also you can see exactly 
what is API change. It is not in mmc_start_req() signature, it is new 
context_info field that we need to synchronize with NEW_REQUEST 
notification. mmc_test is not uses the notification, but it is very easy 
to implement.


>> My main concern is to avoid adding new API to core.c in order to add the NEW_REQ feature. My patch is an example of changes to be done in core.c without adding new functions.
Now you can review API changes.

>>
>>>
>>> We would like to have a generic capability to handle additional events,
>>> such as HPI/stop flow, in addition to the NEW_REQUEST notification.
>>> Therefore, an event mechanism seems to be a better design than completion.
>>>
>>> I've looked at SDIO code and from what I can understand, right now SDIO
>>> is not using async request mechanism and works from 'wait_for_cmd()`
>>> level. This means that such work as exposing MMC core API's is major
>>> change and definitely should be separate design and implementation
>>> effort, while my current patch right now will fix mmc thread behavior
>>> and give better performance for upper layers.
>> You are right. There is no support in the SDIO implementation for pre/post/async feature. Still I had it in mind design the "struct mmc_async". It's possible to re-use the mmc core parts in order to utilize this support in the SDIO case. I'm not saying we should design for SDIO but at least keep the API in a way that it's potential usable from SDIO too. The API of core.c (start/wait of requests) should make sense without the existence of MMC block driver (block/queue).
>>
>> One way to test the design is to add a test in mmc_test.c for penging NEW_REQ. In mmc_test.c there is not block.c or queue.c.
>> If the test case if simple to write in mmc_test.c it's means that the API is on a good level.
I can simulate single NEW_PACKET notification in the mmc_test, but this 
will check only correctness, without real queue of requests we can't 
see/measure tpt/latency gain of this.

Kind reminder about patch v3, waiting for your review.

Thanks

-- 
Konstantin Dorfman,
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center,
Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation