Shawn, Thanks. Oh, sorry I really have missed the fact u mentioned. U are right in the current code, the bit will also be cleared for ESDHC_CD_GPIO. But I think this is improper since for GPIO detection type, we don't use the host controller internal card detection(ESDHC_CD_CONTROLLER), but with SDHCI_QUIRK_BROKEN_CARD_DETECTION cleared, we'll still enable/disable relevant INT bits (in sdhci_set_card_detection in sdhci.c). This is my biggest concern. And I think the SDHCI_QUIRK_BROKEN_CARD_DETECTION shall be purely used to notify whether the host controller detection method is used or not. So even for the ESDHC_CD_GPIO type, we should still set this flag. How do u think?
-----Original Message----- From: Shawn Guo [mailto:shawn....@linaro.org] Sent: 2012年10月18日 13:51 To: Yong Ding Cc: Chris Ball; Anton Vorontsov; Marek Szyprowski; Wolfram Sang; Daniel Drake; Sascha Hauer; Wilson Callan; Ben Dooks; Zhangfei Gao; Kevin Liu; Jialing Fu; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/3] mmc: esdhc: enable polling to detect card by itself On Tue, Oct 16, 2012 at 09:01:40PM -0700, Yong Ding wrote: > Shawn, > Thanks for your comment. And sorry for my so late due to illness:-) > SDHCI_QUIRK_BROKEN_CARD_DETECTION is used for notifying we don't use the host > internal card detection method so that we don't need enable/disable those > relevant interrupt bits of host(sdhci_set_card_detection in sdhci.c). > And as I double-checked the latest kernel code, actually sdhci-esdhc-imx sets > this flag by default, and then will clear it only when the detection type is > ESDHC_CD_CONTROLLER. So this aligns with my understanding. What I was saying is the bit will be cleared when the detection type is ESDHC_CD_CONTROLLE or ESDHC_CD_GPIO. You may have missed the fact that there is no "break" in case ESDHC_CD_GPIO but a "fall through" comment. switch (boarddata->cd_type) { case ESDHC_CD_GPIO: err = gpio_request_one(boarddata->cd_gpio, GPIOF_IN, "ESDHC_CD"); if (err) { dev_err(mmc_dev(host->mmc), "no card-detect pin available!\n"); goto no_card_detect_pin; } err = request_irq(gpio_to_irq(boarddata->cd_gpio), cd_irq, IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING, mmc_hostname(host->mmc), host); if (err) { dev_err(mmc_dev(host->mmc), "request irq error\n"); goto no_card_detect_irq; } /* fall through */ case ESDHC_CD_CONTROLLER: /* we have a working card_detect back */ host->quirks &= ~SDHCI_QUIRK_BROKEN_CARD_DETECTION; break; case ESDHC_CD_PERMANENT: host->mmc->caps = MMC_CAP_NONREMOVABLE; break; case ESDHC_CD_NONE: break; } Shawn > What I want to do is that 1st we shall set MMC_CAP_NEEDS_POLL by our host > driver itself and 2nd remove the improper logic in sdhci_add_host() . How do > u think? Thanks. >
