On Wed, Dec 11, 2013 at 01:03:43PM +0800, Dong Aisheng wrote: > On Wed, Dec 11, 2013 at 11:48 AM, Shawn Guo <shawn.guo@xxxxxxxxxx> wrote: > > On Wed, Dec 11, 2013 at 11:35:17AM +0800, Dong Aisheng wrote: > >> This actually is the register value, not the max timeout counter value. > >> Then you may want define the API as: > >> unsigned int (*get_max_timeout_val)(struct sdhci_host *host); > > > > Yes, something like that. > > > >> But i don't think it's necessary to do the max timeout setting in two steps. > >> First, deinfe a API to get the max timeout counter val, > >> Second, write this val into register. > >> Why not simply implement .set_timeout and handle the details in > >> platform specific > >> host driver respectively? > > > > Well, that's how sdhci host driver is structured. Doing so leaves the > > least details to platform driver, and calling sdhci_writeb() to access > > SDHCI_TIMEOUT_CONTROL in sdhci-esdhc-imx seems a layer violation to me. > > > > The current sdhci-esdhci-imx already does something like that. > You can search SDHCI_* in the code. > It just reuses the register offset definition in sdhci, > It's not the layer violation. I do not understand why we have to do this SDHCI_TIMEOUT_CONTROL setup in our sdhci-esdhc-imx driver, when sdhci driver is already structured to do it in its way. All we need is a hook to give the correct max_timeout_val. Shawn -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html