Re: [PATCH V2 0/3] mmc: remove MMC_CAP_NEEDS_POLL setting in sdhci_add_host

[yongd <yongd@marvell.com>]

On 2012年10月31日 07:11, Anton Vorontsov wrote:
> On Tue, Oct 30, 2012 at 05:30:00PM +0800, yongd wrote:
>> Sorry for my so late. And eventually these updated patches are here for your
>> review. Thanks in advance.
>> For patch 1, add SDHCI_QUIRK_BROKEN_CARD_DETECTION setting for ESDHC_CD_GPIO type,
>> or the host controller detection interrupts will be redundantly enabled. And its
>> comment is correspondingly updated.
>> For patch 2, broaden SDHCI_QUIRK_BROKEN_CARD_DETECTION setting for all detection
>> types except S3C_SDHCI_CD_INTERNAL. Then also update the comment.
>> For patch 3, no update.
> They all look OK, but note that I did not check drivers' logic, so I hope
> you carefully reviewed all the drivers yourself. :)
>
> Unfortunately, you decided to go the dangerous way, instead of taking my
> 100%-safe approach (i.e.
> http://www.mail-archive.com/linux-mmc@vger.kernel.org/msg16555.html ),
> which, to me, still makes more sense (of course :).
>
> Anyways, I'm not opposed either, so hopefully it works:
>
> 	Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>
>
> Thanks!
> Anton.
Anton,
Thanks for your kindly review:-)

Just as I replied to u in the past (maybe got missed due to my previous 
improper mail setting.
Thanks for Shawn Guo again:-)), I don't think your approach is proper. 
Pls forgive me, ha ha...

Something like SDHCI_QUIRK_NEEDS_POLL means all the other detection 
types are unavailable
except polling, and then if the card is removable, polling will be 
enabled. Based on such meaning,
host driver will need set this flag if the host and card are of the 
case. That is exactly of the detection
type like  ESDHC_CD_NONE or S3C_SDHCI_CD_NONE, etc. So, why not directly 
set the existing
MMC_CAP_NEEDS_POLL in host driver itself (just like Au1xmmc.c, etc)? I 
think it is better than achieving
this indirectly through one more flag and some more logic code. Yes, one 
more line for each host driver,
but it is very clear and save one unnecessary flag, isn't it?

BTW, currently not all SDHCI_QUIRK_BROKEN_CARD_DETECTION flags in other 
host drivers are set to
notify we shall use polling since this flag has its own pure usage. So 
we can not simply convert them
to the new flag u mentioned. Hope I've gotten your idea:-)

So I think my patches are the proper way:-) BTW, I've carefully review 
those patches. Thanks for your
reminding.

Shawn,
Could u pls help review those patches again? Thanks in advance:-)