| Message ID | 20200129203709.30493-7-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mmc: tmio: move TAP handling to SDHI driver | expand |
Hi Wolfram, Thanks for your patch. On 2020-01-29 21:37:09 +0100, Wolfram Sang wrote: > After various refactoring, we can populate the mmc_ops callbacks > directly and don't need to have wrappers for them anymore. > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > drivers/mmc/host/renesas_sdhi_core.c | 18 ++++++++++------ > drivers/mmc/host/tmio_mmc_core.c | 32 +--------------------------- > 2 files changed, 12 insertions(+), 38 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 22eaabe513d0..0f07cc1aee34 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -321,8 +321,9 @@ static unsigned int renesas_sdhi_init_tuning(struct tmio_mmc_host *host) > SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK; > } > > -static void renesas_sdhi_hs400_complete(struct tmio_mmc_host *host) > +static void renesas_sdhi_hs400_complete(struct mmc_host *mmc) > { > + struct tmio_mmc_host *host = mmc_priv(mmc); > struct renesas_sdhi *priv = host_to_priv(host); > > sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN & > @@ -376,8 +377,9 @@ static void renesas_sdhi_reset_scc(struct tmio_mmc_host *host, > SH_MOBILE_SDHI_SCC_CKSEL)); > } > > -static void renesas_sdhi_disable_scc(struct tmio_mmc_host *host) > +static void renesas_sdhi_disable_scc(struct mmc_host *mmc) > { > + struct tmio_mmc_host *host = mmc_priv(mmc); > struct renesas_sdhi *priv = host_to_priv(host); > > renesas_sdhi_reset_scc(host, priv); > @@ -412,9 +414,12 @@ static void renesas_sdhi_reset_hs400_mode(struct tmio_mmc_host *host, > sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL)); > } > > -static void renesas_sdhi_prepare_hs400_tuning(struct tmio_mmc_host *host) > +static int renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios) > { > + struct tmio_mmc_host *host = mmc_priv(mmc); > + > renesas_sdhi_reset_hs400_mode(host, host_to_priv(host)); > + return 0; > } > > #define SH_MOBILE_SDHI_MAX_TAP 3 > @@ -899,10 +904,9 @@ int renesas_sdhi_probe(struct platform_device *pdev, > > host->execute_tuning = renesas_sdhi_execute_tuning; > host->check_retune = renesas_sdhi_check_scc_error; > - host->prepare_hs400_tuning = > - renesas_sdhi_prepare_hs400_tuning; > - host->hs400_downgrade = renesas_sdhi_disable_scc; > - host->hs400_complete = renesas_sdhi_hs400_complete; > + host->ops.prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning; > + host->ops.hs400_downgrade = renesas_sdhi_disable_scc; Would it make sens to rename renesas_sdhi_disable_scc() to renesas_sdhi_hs400_downgrade() to fit the pattern that it's called from the mmc_ops? With or without this nit, Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > + host->ops.hs400_complete = renesas_sdhi_hs400_complete; > } > > num_irqs = platform_irq_count(pdev); > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index aeafa1c68ce2..a2415fb5dde0 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -997,34 +997,7 @@ static int tmio_multi_io_quirk(struct mmc_card *card, > return blk_size; > } > > -static int tmio_mmc_prepare_hs400_tuning(struct mmc_host *mmc, > - struct mmc_ios *ios) > -{ > - struct tmio_mmc_host *host = mmc_priv(mmc); > - > - if (host->prepare_hs400_tuning) > - host->prepare_hs400_tuning(host); > - > - return 0; > -} > - > -static void tmio_mmc_hs400_downgrade(struct mmc_host *mmc) > -{ > - struct tmio_mmc_host *host = mmc_priv(mmc); > - > - if (host->hs400_downgrade) > - host->hs400_downgrade(host); > -} > - > -static void tmio_mmc_hs400_complete(struct mmc_host *mmc) > -{ > - struct tmio_mmc_host *host = mmc_priv(mmc); > - > - if (host->hs400_complete) > - host->hs400_complete(host); > -} > - > -static const struct mmc_host_ops tmio_mmc_ops = { > +static struct mmc_host_ops tmio_mmc_ops = { > .request = tmio_mmc_request, > .set_ios = tmio_mmc_set_ios, > .get_ro = tmio_mmc_get_ro, > @@ -1033,9 +1006,6 @@ static const struct mmc_host_ops tmio_mmc_ops = { > .multi_io_quirk = tmio_multi_io_quirk, > .hw_reset = tmio_mmc_hw_reset, > .execute_tuning = tmio_mmc_execute_tuning, > - .prepare_hs400_tuning = tmio_mmc_prepare_hs400_tuning, > - .hs400_downgrade = tmio_mmc_hs400_downgrade, > - .hs400_complete = tmio_mmc_hs400_complete, > }; > > static int tmio_mmc_init_ocr(struct tmio_mmc_host *host) > -- > 2.20.1 >
> > + host->ops.prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning; > > + host->ops.hs400_downgrade = renesas_sdhi_disable_scc; > > Would it make sens to rename renesas_sdhi_disable_scc() to > renesas_sdhi_hs400_downgrade() to fit the pattern that it's called from > the mmc_ops? I also thought about it and came to the conclusion that this is more readable. To downgrade from HS400, we need to disable SCC. But no big deal to change it if you/others think the other pattern...
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 22eaabe513d0..0f07cc1aee34 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -321,8 +321,9 @@ static unsigned int renesas_sdhi_init_tuning(struct tmio_mmc_host *host) SH_MOBILE_SDHI_SCC_DTCNTL_TAPNUM_MASK; } -static void renesas_sdhi_hs400_complete(struct tmio_mmc_host *host) +static void renesas_sdhi_hs400_complete(struct mmc_host *mmc) { + struct tmio_mmc_host *host = mmc_priv(mmc); struct renesas_sdhi *priv = host_to_priv(host); sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN & @@ -376,8 +377,9 @@ static void renesas_sdhi_reset_scc(struct tmio_mmc_host *host, SH_MOBILE_SDHI_SCC_CKSEL)); } -static void renesas_sdhi_disable_scc(struct tmio_mmc_host *host) +static void renesas_sdhi_disable_scc(struct mmc_host *mmc) { + struct tmio_mmc_host *host = mmc_priv(mmc); struct renesas_sdhi *priv = host_to_priv(host); renesas_sdhi_reset_scc(host, priv); @@ -412,9 +414,12 @@ static void renesas_sdhi_reset_hs400_mode(struct tmio_mmc_host *host, sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL)); } -static void renesas_sdhi_prepare_hs400_tuning(struct tmio_mmc_host *host) +static int renesas_sdhi_prepare_hs400_tuning(struct mmc_host *mmc, struct mmc_ios *ios) { + struct tmio_mmc_host *host = mmc_priv(mmc); + renesas_sdhi_reset_hs400_mode(host, host_to_priv(host)); + return 0; } #define SH_MOBILE_SDHI_MAX_TAP 3 @@ -899,10 +904,9 @@ int renesas_sdhi_probe(struct platform_device *pdev, host->execute_tuning = renesas_sdhi_execute_tuning; host->check_retune = renesas_sdhi_check_scc_error; - host->prepare_hs400_tuning = - renesas_sdhi_prepare_hs400_tuning; - host->hs400_downgrade = renesas_sdhi_disable_scc; - host->hs400_complete = renesas_sdhi_hs400_complete; + host->ops.prepare_hs400_tuning = renesas_sdhi_prepare_hs400_tuning; + host->ops.hs400_downgrade = renesas_sdhi_disable_scc; + host->ops.hs400_complete = renesas_sdhi_hs400_complete; } num_irqs = platform_irq_count(pdev); diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c index aeafa1c68ce2..a2415fb5dde0 100644 --- a/drivers/mmc/host/tmio_mmc_core.c +++ b/drivers/mmc/host/tmio_mmc_core.c @@ -997,34 +997,7 @@ static int tmio_multi_io_quirk(struct mmc_card *card, return blk_size; } -static int tmio_mmc_prepare_hs400_tuning(struct mmc_host *mmc, - struct mmc_ios *ios) -{ - struct tmio_mmc_host *host = mmc_priv(mmc); - - if (host->prepare_hs400_tuning) - host->prepare_hs400_tuning(host); - - return 0; -} - -static void tmio_mmc_hs400_downgrade(struct mmc_host *mmc) -{ - struct tmio_mmc_host *host = mmc_priv(mmc); - - if (host->hs400_downgrade) - host->hs400_downgrade(host); -} - -static void tmio_mmc_hs400_complete(struct mmc_host *mmc) -{ - struct tmio_mmc_host *host = mmc_priv(mmc); - - if (host->hs400_complete) - host->hs400_complete(host); -} - -static const struct mmc_host_ops tmio_mmc_ops = { +static struct mmc_host_ops tmio_mmc_ops = { .request = tmio_mmc_request, .set_ios = tmio_mmc_set_ios, .get_ro = tmio_mmc_get_ro, @@ -1033,9 +1006,6 @@ static const struct mmc_host_ops tmio_mmc_ops = { .multi_io_quirk = tmio_multi_io_quirk, .hw_reset = tmio_mmc_hw_reset, .execute_tuning = tmio_mmc_execute_tuning, - .prepare_hs400_tuning = tmio_mmc_prepare_hs400_tuning, - .hs400_downgrade = tmio_mmc_hs400_downgrade, - .hs400_complete = tmio_mmc_hs400_complete, }; static int tmio_mmc_init_ocr(struct tmio_mmc_host *host)
After various refactoring, we can populate the mmc_ops callbacks directly and don't need to have wrappers for them anymore. Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> --- drivers/mmc/host/renesas_sdhi_core.c | 18 ++++++++++------ drivers/mmc/host/tmio_mmc_core.c | 32 +--------------------------- 2 files changed, 12 insertions(+), 38 deletions(-)