| Message ID | 20180119133906.11280-3-horms+renesas@verge.net.au (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Simon, > + void (*disable_scc)(struct mmc_host *mmc); Do we really need this callback? I'd think it can be folded into reset_hs400_mode() because it is called only once? > + void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios); Can't we use the host->ops->prepare_hs400_tuning() callback invoked by the core? > + void (*reset_hs400_mode)(struct mmc_host *mmc); Maybe we can get rid of this, too? See later... > + if (host->disable_scc) > + host->disable_scc(mmc); (Here, this can be folded into the next callback) > + > + /* reset HS400 mode */ > + if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode) > + host->reset_hs400_mode(mmc); I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the hs400_mode needs to be reset. Couldn't we as well then disable the mode always after the MMC_TIMING_MMC_HS400 tuning was selected? Just brainstorming here... Regards, Wolfram
On Wed, Feb 07, 2018 at 11:20:12PM +0100, Wolfram Sang wrote: > > Hi Simon, > > > + void (*disable_scc)(struct mmc_host *mmc); > > Do we really need this callback? I'd think it can be folded into > reset_hs400_mode() because it is called only once? > > > + void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios); > > Can't we use the host->ops->prepare_hs400_tuning() callback invoked by > the core? Empirically that does not seem to work. > > + void (*reset_hs400_mode)(struct mmc_host *mmc); > > Maybe we can get rid of this, too? See later... > > > + if (host->disable_scc) > > + host->disable_scc(mmc); > > (Here, this can be folded into the next callback) Yes, agreed. I've folded the callbacks as you suggest. > > + > > + /* reset HS400 mode */ > > + if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode) > > + host->reset_hs400_mode(mmc); > > I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the > hs400_mode needs to be reset. Couldn't we as well then disable the mode > always after the MMC_TIMING_MMC_HS400 tuning was selected? Just > brainstorming here... Perhaps but I'm unsure where we would hook in this change, any ideas?
On Tue, Feb 13, 2018 at 12:33:53PM +0100, Simon Horman wrote: > On Wed, Feb 07, 2018 at 11:20:12PM +0100, Wolfram Sang wrote: > > > > Hi Simon, > > > > > + void (*disable_scc)(struct mmc_host *mmc); > > > > Do we really need this callback? I'd think it can be folded into > > reset_hs400_mode() because it is called only once? > > > > > + void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios); > > > > Can't we use the host->ops->prepare_hs400_tuning() callback invoked by > > the core? > > Empirically that does not seem to work. :( Pity. > > > + void (*reset_hs400_mode)(struct mmc_host *mmc); > > > > Maybe we can get rid of this, too? See later... > > > > > + if (host->disable_scc) > > > + host->disable_scc(mmc); > > > > (Here, this can be folded into the next callback) > > Yes, agreed. I've folded the callbacks as you suggest. Cool, thanks! > > > + > > > + /* reset HS400 mode */ > > > + if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode) > > > + host->reset_hs400_mode(mmc); > > > > I wonder: If for any ios which is != MMC_TIMING_MMC_HS400, the > > hs400_mode needs to be reset. Couldn't we as well then disable the mode > > always after the MMC_TIMING_MMC_HS400 tuning was selected? Just > > brainstorming here... > > Perhaps but I'm unsure where we would hook in this change, any ideas? I think this becomes moot if we can't get the prepare-callback of the core to work. I'd think it makes sense to have both callbacks from here moved to the core or none to keep at least one kind of symmetry.
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h index e7d651352dc9..8f163e7f71e0 100644 --- a/drivers/mmc/host/tmio_mmc.h +++ b/drivers/mmc/host/tmio_mmc.h @@ -46,6 +46,7 @@ #define CTL_DMA_ENABLE 0xd8 #define CTL_RESET_SD 0xe0 #define CTL_VERSION 0xe2 +#define CTL_SDIF_MODE 0xe6 #define CTL_SDIO_REGS 0x100 #define CTL_CLK_AND_WAIT_CTL 0x138 #define CTL_RESET_SDIO 0x1e0 @@ -191,6 +192,11 @@ struct tmio_mmc_host { /* Tuning values: 1 for success, 0 for failure */ DECLARE_BITMAP(taps, BITS_PER_BYTE * sizeof(long)); unsigned int tap_num; + unsigned long tap_set; + + void (*disable_scc)(struct mmc_host *mmc); + void (*prepare_hs400_tuning)(struct mmc_host *mmc, struct mmc_ios *ios); + void (*reset_hs400_mode)(struct mmc_host *mmc); const struct tmio_mmc_dma_ops *dma_ops; }; diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c index 41767d33ef97..2a84da51b08d 100644 --- a/drivers/mmc/host/tmio_mmc_core.c +++ b/drivers/mmc/host/tmio_mmc_core.c @@ -199,6 +199,13 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, tmio_mmc_clk_stop(host); return; } + /* + * Both HS400 and HS200/SD104 set 200MHz, but some devices need to + * set 400MHz to distinguish the CPG settings in HS400. + */ + if (host->mmc->ios.timing == MMC_TIMING_MMC_HS400 && + new_clock == 200000000) + new_clock = 400000000; if (host->clk_update) clock = host->clk_update(host, new_clock) / 512; @@ -209,8 +216,13 @@ static void tmio_mmc_set_clock(struct tmio_mmc_host *host, clock <<= 1; /* 1/1 clock is option */ - if ((host->pdata->flags & TMIO_MMC_CLK_ACTUAL) && ((clk >> 22) & 0x1)) - clk |= 0xff; + if ((host->pdata->flags & TMIO_MMC_CLK_ACTUAL) && + ((clk >> 22) & 0x1)) { + if (!(host->mmc->ios.timing == MMC_TIMING_MMC_HS400)) + clk |= 0xff; + else + clk &= ~0xff; + } if (host->set_clk_div) host->set_clk_div(host->pdev, (clk >> 22) & 1); @@ -1012,6 +1024,13 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) struct device *dev = &host->pdev->dev; unsigned long flags; + if (host->disable_scc) + host->disable_scc(mmc); + + /* reset HS400 mode */ + if (ios->timing != MMC_TIMING_MMC_HS400 && host->reset_hs400_mode) + host->reset_hs400_mode(mmc); + mutex_lock(&host->ios_lock); spin_lock_irqsave(&host->lock, flags); @@ -1062,6 +1081,11 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios) "%s.%d: IOS interrupted: clk %u, mode %u", current->comm, task_pid_nr(current), ios->clock, ios->power_mode); + + /* HS400 Register setting */ + if (ios->timing == MMC_TIMING_MMC_HS400 && host->prepare_hs400_tuning) + host->prepare_hs400_tuning(mmc, ios); + host->mrq = NULL; host->clk_cache = ios->clock;