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

From: yongd
Date: Wed Oct 31 2012 - 06:15:29 EST


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@xxxxxxxxxxxxxxx/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@xxxxxxxxx>

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