Hi Will
Will Newton wrote:
> On Fri, Mar 25, 2011 at 6:00 AM, Jaehoon Chung <jh80.chung@xxxxxxxxxxx> wrote:
>> If assume only using one slot, i think that dw_mci_queue_request() need not.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@xxxxxxxxxxx>
>> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx
>> ---
>> drivers/mmc/host/dw_mmc.c | 9 +++++++--
>> include/linux/mmc/dw_mmc.h | 3 ++-
>> 2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 882d004..e3f26ea 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -673,8 +673,13 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
>> return;
>> }
>>
>> - /* We don't support multiple blocks of weird lengths. */
>> - dw_mci_queue_request(host, slot, mrq);
>> + if (host->quirks & DW_MCI_QUIRK_FORCE_ONE_SLOT) {
>
> Do we really need a quirk for this? Why not use host->num_slots
Yes, you'r right. if using this patch, can use host->num_slots.
>
>> + slot->mrq = mrq;
>> + host->state = STATE_SENDING_CMD;
>
> I don't think it is safe to manipulate these structures without taking
> the host->lock. If we are to do this then I think I would like to know
> why (e.g. do we have performance numbers to support this change) and
> some analysis of what is protected by host->lock and which functions
> need the lock to be held. For example I do not think it is safe to
> call dw_mci_start_request without taking the lock.
>
I know this patch didn't increase the performance.
BUt If we want to know some analysis..I should analysis what protected by host->lock.
Did you analysis what protect by host->lock?
>> + dw_mci_start_request(host, slot);
>> + } else
>> + /* We don't support multiple blocks of weird lengths. */
>
> This comment is obsolete I think and can be removed.
Anyway, Thanks for your comment.
Regards,
Jaehoon Chung
--
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
