linux-kernel - Re: [PATCH V2 1/3] mmc: esdhc: enable polling to detect card by itself

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 5 Nov 2012 09:54:22 +0800
From:	Shawn Guo <shawn.guo@...aro.org>
To:	yongd <yongd@...vell.com>
CC:	Chris Ball <cjb@...top.org>,
	Anton Vorontsov <anton.vorontsov@...aro.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Wolfram Sang <w.sang@...gutronix.de>,
	Daniel Drake <dsd@...top.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Wilson Callan <wilson.callan@...antsystems.com>,
	Ben Dooks <ben-linux@...ff.org>,
	Zhangfei Gao <zgao6@...vell.com>,
	Kevin Liu <kliu5@...vell.com>,
	"linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2 1/3] mmc: esdhc: enable polling to detect card by
 itself

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.

> 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/