Re: [RFC PATCH 1/1] dw_mmc: add quirk about using only one slot

[Jaehoon Chung <jh80.chung@samsung.com>]

Hi Will

Will Newton wrote:
> On Fri, Mar 25, 2011 at 6:00 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> If assume only using one slot, i think that dw_mci_queue_request() need not.
>>
>> Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com
>> ---
>>  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