On 2012年11月05日 09:54, Shawn Guo wrote:
On Fri, Nov 02, 2012 at 08:37:41PM +0800, yongd wrote:
I got it. So how do you think of such below partition/reorganization?

It looks fine to me for imx.

Ok. I will update like this way if we have no other issues. Thanks.


Patch-1:
For sdhci-esdhc-imx.c, only add MMC_CAP_NEEDS_POLL for ESDHC_CD_NONE. With that
improper logic of sdhci_add_host(), this is redundant, but shall be better
than something unpleasant you mentioned.

Patch-2:
For sdhci-s3c.c, do exactly the same thing as Patch-1.

Patch-3:
For sdhci.c, remove that improper logic of sdhci_add_host().

Patch-4:
For sdhci-esdhc-imx.c, set SDHCI_QUIRK_BROKEN_CARD_DETECTION for ESDHC_CD_GPIO.

Patch-5:
For sdhci-s3c.c, broaden SDHCI_QUIRK_BROKEN_CARD_DETECTION range for all 
detection
types except S3C_SDHCI_CD_INTERNAL.
<snip>

Yes, not equal as before. But you just remind me of one more improper place in 
our current sdhci.c.
Let's review those lines in sdhci_request() which are added by Anton long long 
ago in commit
68d1fb7e229c6f95be4fbbe3eb46b24e41184924(sdhci: Add support for card-detection 
polling),

        /* If polling, assume that the card is always present. */
        if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
                present = true;
        else
                present = sdhci_readl(host, SDHCI_PRESENT_STATE) &
                                SDHCI_CARD_PRESENT;

Here before sending command, if we can confirm the card dose not exist, we will 
return quickly without
sending this command.This is a good optimization. But if polling, we can't do 
such checking, so we can
only assume the card is always present.
Here is another improper example of using SDHCI_QUIRK_BROKEN_CARD_DETECTION. 
Exactly the same as
sdhci_add_host(), it thinks only polling and host internal card detection 
methods exist.
Maybe we can determine whether and how we can do such card-existence-checking 
optimization based on our
detection type directly rather than an indirect flag which should have its own 
pure meaning. But Let's
do such similar further improvement in future since currently with my patch of 
setting
SDHCI_QUIRK_BROKEN_CARD_DETECTION for ESDHC_CD_GPIO, functionality is OK as 
before. How do u think?

I'm not sure it will function same as before.  When I was testing your
v2 series, I can not see card removal message with removing card, but
can see it show up together with the next card inserting message.

Shawn


Shawn,
From the code logic, without SDHCI_QUIRK_BROKEN_CARD_DETECTION for 
ESDHC_CD_GPIO, when your card (using
GPIO detection) is removed, we can know the card's absence through the fake 
CARD_PRESENT flag in esdhc_readl_le().
As a result, we can quickly return ENOMEDIUM error without command sending. 
Finally mmc_rescan can know the card
is removed.

On the other hand, with SDHCI_QUIRK_BROKEN_CARD_DETECTION for ESDHC_CD_GPIO, we 
will just send MMC_SEND_STATUS
command (cd_irq->sdhci_tasklet_card->mmc_recan->mmc_sd_detect->mmc_sd_alive), 
and we will find error since the card
is already gone. Finally, we shall also know the card is removed.

This is really strange why the removal message shows up together with the next 
card inserting message.
Could u pls tell us more about what actually happened when the card is removed 
with my patches? Did the GPIO interrupt
happen? Was mmc_rescan() called? Did mmc_sd_detect() finally find error when it 
sent MMC_SEND_STATUS? What step did
the card removing procedure arrive at? Really really thanks a lot:-)





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

Reply via email to