[RFC] dw_mmc: didn't support multiple blocks of weird length?

[RFC] dw_mmc: didn't support multiple blocks of weird length?

[Jaehoon Chung]

Hi..

I didn't understand this point..
Plz let me explain this code..
why need this function (dw_mci_queue_request(host, slot, mrq)?


static void dw_mci_queue_request(struct dw_mci *host, struct dw_mci_slot *slot,
				 struct mmc_request *mrq)
{
	dev_vdbg(&slot->mmc->class_dev, "queue request: state=%d\n",
		 host->state);

	spin_lock_bh(&host->lock);
	slot->mrq = mrq;

	if (host->state == STATE_IDLE) {
		host->state = STATE_SENDING_CMD;
		dw_mci_start_request(host, slot);
	} else {
		list_add_tail(&slot->queue_node, &host->queue);
	}

	spin_unlock_bh(&host->lock);
}

static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)
{
	struct dw_mci_slot *slot = mmc_priv(mmc);
	struct dw_mci *host = slot->host;

	WARN_ON(slot->mrq);

	if (!test_bit(DW_MMC_CARD_PRESENT, &slot->flags)) {
		mrq->cmd->error = -ENOMEDIUM;
		mmc_request_done(mmc, mrq);
		return;
	}

	/* We don't support multiple blocks of weird lengths. */
	dw_mci_queue_request(host, slot, mrq);
}

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Will Newton]

On Fri, Feb 25, 2011 at 7:35 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi..
>
> I didn't understand this point..
> Plz let me explain this code..
> why need this function (dw_mci_queue_request(host, slot, mrq)?

I don't think there would be any problem merging those two functions.
Also I would be happy to see the comment deleted, it doesn't really
make sense.

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Jaehoon Chung]

Right...maybe not problem merging those two functions.
But think not use list_add_tail(&slot->queue_node, &host->queue)..

I want to know when use list_add_tail functions..in this code.

Regards,
Jaehoon Chung


Will Newton wrote:
> On Fri, Feb 25, 2011 at 7:35 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi..
>>
>> I didn't understand this point..
>> Plz let me explain this code..
>> why need this function (dw_mci_queue_request(host, slot, mrq)?
> 
> I don't think there would be any problem merging those two functions.
> Also I would be happy to see the comment deleted, it doesn't really
> make sense.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Will Newton]

On Mon, Feb 28, 2011 at 2:44 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Right...maybe not problem merging those two functions.
> But think not use list_add_tail(&slot->queue_node, &host->queue)..
>
> I want to know when use list_add_tail functions..in this code.

I think that code is from the original NXP driver and appears to be
used to support multiple slots attached to the same block (so you can
be asked to process a request for slot A while the block is busy
processing an earlier request for slot B). I don't have any hardware
setup like this, everything I have has only a single slot so I don't
believe I have ever seen that branch of the conditional execute.

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Jaehoon Chung]

Will Newton wrote:
> On Mon, Feb 28, 2011 at 2:44 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Right...maybe not problem merging those two functions.
>> But think not use list_add_tail(&slot->queue_node, &host->queue)..
>>
>> I want to know when use list_add_tail functions..in this code.
> 
> I think that code is from the original NXP driver and appears to be
> used to support multiple slots attached to the same block (so you can
> be asked to process a request for slot A while the block is busy
> processing an earlier request for slot B). I don't have any hardware
> setup like this, everything I have has only a single slot so I don't
> believe I have ever seen that branch of the conditional execute.

Oh..i also used only single slot..i want to remove spinlock_bh()..
because if we used a only single slot, i think that spinlock_bh() not need..
how think about this?

> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Will Newton]

On Mon, Feb 28, 2011 at 10:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Will Newton wrote:
>> On Mon, Feb 28, 2011 at 2:44 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Right...maybe not problem merging those two functions.
>>> But think not use list_add_tail(&slot->queue_node, &host->queue)..
>>>
>>> I want to know when use list_add_tail functions..in this code.
>>
>> I think that code is from the original NXP driver and appears to be
>> used to support multiple slots attached to the same block (so you can
>> be asked to process a request for slot A while the block is busy
>> processing an earlier request for slot B). I don't have any hardware
>> setup like this, everything I have has only a single slot so I don't
>> believe I have ever seen that branch of the conditional execute.
>
> Oh..i also used only single slot..i want to remove spinlock_bh()..
> because if we used a only single slot, i think that spinlock_bh() not need..
> how think about this?

I'm not convinced that removing the multiple slot functionality will
allow you to remove the spin_lock_bh, it looks like the lock may be
protecting more than just the queue. Why do you want to remove the
spinlock?

Re: [RFC] dw_mmc: didn't support multiple blocks of weird length?

[Jaehoon Chung]

Will Newton wrote:
> On Mon, Feb 28, 2011 at 10:55 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Will Newton wrote:
>>> On Mon, Feb 28, 2011 at 2:44 AM, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> Right...maybe not problem merging those two functions.
>>>> But think not use list_add_tail(&slot->queue_node, &host->queue)..
>>>>
>>>> I want to know when use list_add_tail functions..in this code.
>>> I think that code is from the original NXP driver and appears to be
>>> used to support multiple slots attached to the same block (so you can
>>> be asked to process a request for slot A while the block is busy
>>> processing an earlier request for slot B). I don't have any hardware
>>> setup like this, everything I have has only a single slot so I don't
>>> believe I have ever seen that branch of the conditional execute.
>> Oh..i also used only single slot..i want to remove spinlock_bh()..
>> because if we used a only single slot, i think that spinlock_bh() not need..
>> how think about this?
> 
> I'm not convinced that removing the multiple slot functionality will
> allow you to remove the spin_lock_bh, it looks like the lock may be
> protecting more than just the queue. Why do you want to remove the
> spinlock?

I should not remove the spin_lock_bh in using multiple slot.
If i use only one slot, i asked that need spin_lock_bh()?
I think if we assume using one slot,need not them..(using quirks instead of removing them)

> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>