Skip to contentSkip to conversation

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

RFC Jaehoon Chung 2 participants 7 messages Feb 25, 2011 – Mar 2, 2011 MMC, SD and SDIO

lore
Share & tools
lore
#0 lore raw

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);
}
#1 lore raw
Quoted 5 lines · 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)?

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.

#2 lore raw

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

Quoted 15 lines · Will Newton
Quoted 5 lines · 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)?

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#3 lore raw
Quoted 4 lines · 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.

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.

#4 lore raw
Quoted 4 lines · Will Newton
Quoted 3 lines · Jaehoon Chung

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

Quoted 8 lines
Quoted 1 line

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?

Quoted 5 lines

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

#5 lore raw
Quoted 14 lines · Jaehoon Chung
Quoted 4 lines · Will Newton
Quoted 3 lines · Jaehoon Chung

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

Quoted 8 lines
Quoted 1 line

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.

Quoted 3 lines

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?

#6 lore raw
Quoted 13 lines · Will Newton
Quoted 12 lines · Jaehoon Chung
Quoted 11 lines · Will Newton
Quoted 4 lines · 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.

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.

Quoted 8 lines
Quoted 3 lines

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)

Quoted 5 lines

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