RE: [PATCH] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Anton,
Thanks a lot for your comments. I've just reviewed those host drivers under drivers/mmc/host who use SDHCI_QUIRK_BROKEN_CARD_DETECTION. And here are the details,

1. sdhci-pxav2/3.c
Actually they set "SDHCI_QUIRK_BROKEN_CARD_DETECTION and MMC_CAP_NONREMOVABLE" together for a permanent card who is always wired to host and with power always on, eg, emmc card. So, they don't set SDHCI_QUIRK_BROKEN_CARD_DETECTION for polling:-)

2. sdhci-s3c.c
If the card detection type is S3C_SDHCI_CD_NONE, they are expecting polling method to detect card. And in the current code logic, sdhci-s3c.c does not set MMC_CAP_NEEDS_POLL itself, but depends on the code which I am preferring to remove. So, you are right this is not so that simple.

3. sdhci-esdhc-imx.c
By default, it sets SDHCI_QUIRK_BROKEN_CARD_DETECTION for all hosts. 
And then if the card detection type is ESDHC_CD_CONTROLLER(indicating we use the host internal card detection), it will clear this flag. No issue.
If the cd_type is ESDHC_CD_PERMANENT, it will still keep the set of the flag. This is just exactly the same case as sdhci-pxav2/3.c. No issue.
If the cd_type is ESDHC_CD_GPIO(using external GPIO for CD), it will keep the set of this flag, and I think this is the right logic since we don't use host internal card detection. But this will eventually causes the enabling of polling method, which is rightly the case mentioned in my patch comment. A +1 for my patch.
If the cd_type is ESDHC_CD_NONE, this is the same case as sdhci-s3c.c. A +1 for your guess/concern:-)

4. sdhci-of-esdhc.c and sdhci-pci.c
Cased are already included in the above 3 samples. So nothing more here.

So, in all, u are right if with my current patch, some host drivers need some improvement to add MMC_CAP_NEEDS_POLL when it is actually needed. But I think this shall be the right way to follow. Or, we might enable polling for some cases in which it is unnecessary, and maybe this is a potential issue-bomb. How do u think?


Best Wishes,

Yong Ding
Operating Systems Engineering,
Application Processor Systems Engineering

> -----Original Message-----
> From: Anton Vorontsov [mailto:anton.vorontsov@xxxxxxxxxx]
> Sent: Tuesday, September 25, 2012 3:04 PM
> To: Yong Ding
> Cc: Chris Ball; linux-mmc@xxxxxxxxxxxxxxx; Daniel Drake; Zhangfei Gao
> Subject: Re: [PATCH] mmc: remove MMC_CAP_NEEDS_POLL setting in
> sdhci_add_host
> 
> On Tue, Sep 25, 2012 at 02:34:07PM +0800, yongd wrote:
> > From: yongd <yongd@xxxxxxxxxxx>
> [...]
> > And the better one to decide whether we use polling or not should be
> > the host driver itself. Actually, some host driver has already been
> > like this. Eg, in drivers/mmc/host/Au1xmmc.c, polling will be enabled
> > only after the board-specific card detection can't be set up
> successfully.
> 
> I guess it's not that simple. If you remove this, you have to add
> appropriate CAP_NEEDS_POLL for these drivers:
> 
>  linux/drivers/mmc/host$ git grep -l SDHCI_QUIRK_BROKEN_CARD_DETECTION
> | xargs grep -L NEEDS_POLL
>  sdhci-esdhc-imx.c
>  sdhci-of-esdhc.c
>  sdhci-pci.c
>  sdhci-pxav2.c
>  sdhci-pxav3.c
>  sdhci-s3c.c
> 
> > Change-Id: I27774488a7b9191d7bc39699fd7d62ee21bbf157
> > Signed-off-by: yongd <yongd@xxxxxxxxxxx>
> > ---
> >  drivers/mmc/host/sdhci.c |    4 ----
> >  1 file changed, 4 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 0e15c79..900d5f4 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -2840,10 +2840,6 @@ int sdhci_add_host(struct sdhci_host *host)
> >  	if (caps[0] & SDHCI_CAN_DO_HISPD)
> >  		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
> >
> > -	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
> > -	    !(host->mmc->caps & MMC_CAP_NONREMOVABLE))
> > -		mmc->caps |= MMC_CAP_NEEDS_POLL;
> > -
> >  	/* If vqmmc regulator and no 1.8V signalling, then there's no UHS
> */
> >  	host->vqmmc = regulator_get(mmc_dev(mmc), "vqmmc");
> >  	if (IS_ERR(host->vqmmc)) {
> > --
> > 1.7.9.5
��.n��������+%������w��{.n�����{��i��)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux