Message ID | 1534999460-15111-1-git-send-email-yamada.masahiro@socionext.com |
---|---|
Headers | show |
Series | mmc: tmio: refactor TMIO core a bit and add UniPhier SD/eMMC controller support | expand |
[...] > +static void uniphier_sd_external_dma_request(struct tmio_mmc_host *host, > + struct tmio_mmc_data *pdata) > +{ > + struct uniphier_sd_priv *priv = uniphier_sd_priv(host); > + struct dma_chan *chan; > + > + chan = dma_request_chan(mmc_dev(host->mmc), "rx-tx"); > + if (IS_ERR(chan)) { > + dev_warn(mmc_dev(host->mmc), > + "failed to request DMA channel. falling back to PIO\n"); > + return; /* just use PIO even for -EPROBE_DEFER */ > + } > + > + /* this driver uses a single channel for both RX an TX */ > + priv->chan = chan; > + host->chan_rx = chan; > + host->chan_tx = chan; > + > + tasklet_init(&host->dma_issue, uniphier_sd_external_dma_issue, > + (unsigned long)host); Isn't it time to get rid of tasklets for tmio? No worries, I don't require that to be changed prior $subject patch, however using threaded IRQs is a justified modernization for the tmio variants. [...] Kind regards Uffe
On Thu, Aug 23, 2018 at 12:43:31PM +0200, Ulf Hansson wrote: > On 23 August 2018 at 06:44, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > Add UniPhier SD/eMMC controller support. > > > > As a preparation, I changed tmio_mmc_set_clock() to a platform hook. > > The clock rate setting is platform-specific, and UniPhier variants > > will add another way. I thought it would be better to split this > > to a hook to avoid a mess. > > > > V3 is rebase on top of Linus' tree. > > I have queued patch 1 -> 4, waiting for further reviews for patch 5 and 6. I have re-tested patches 1-4 as applied to mmc/next and it all works fine. For those Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com> Now, going to check patches 5+6... > > I dropped 6/7 because Renesas added more quirks > > in the current MW. Totally fine. We need to cleanup HS400 support more, then we can try the simplification you did for tmio_mmc.c, too. Thanks!
On 29 August 2018 at 10:49, Wolfram Sang <wsa@the-dreams.de> wrote: > >> + if (divisor <= 1) { >> + clk_sel = 1; >> + clk = 0; >> + } else { >> + clk_sel = 0; >> + /* bit7 set: 1/512, ... bit0 set:1/4, all bits clear: 1/2 */ >> + clk = roundup_pow_of_two(divisor) >> 2; >> + } > > What about > > clk_sel = (divisor <= 1); > clk = clk_sel ? 0 : (roundup_pow_of_two(divisor) >> 2) > > More concise, but I think still readable. I don't mind super much, > though. Right. So, may I apply Yamada-san's original patch (adding your tested/reviewed-by tag) and the above as a code-cleanup for you to address on top? Kind regards Uffe
On 23 August 2018 at 06:44, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1) > clock <<= 1; > > ... is too tricky, hence I replaced with > > roundup_pow_of_two(divisor) >> 2 > > '(clk >> 22) & 0x1' is the bit test for the 1/1 divisor, but > it is not clear. 'divisor <= 1' is easier to understand. > > Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Applied for next, thanks! Kind regards Uffe > --- > > Changes in v3: None > Changes in v2: None > > drivers/mmc/host/tmio_mmc.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index b031a77..e04c322 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -48,19 +48,27 @@ static void tmio_mmc_clk_stop(struct tmio_mmc_host *host) > static void tmio_mmc_set_clock(struct tmio_mmc_host *host, > unsigned int new_clock) > { > - u32 clk = 0, clock; > + unsigned int clock, divisor; > + u32 clk = 0; > + int clk_sel; > > if (new_clock == 0) { > tmio_mmc_clk_stop(host); > return; > } > > - clock = host->mmc->f_min; > + divisor = host->pdata->hclk / new_clock; > > - for (clk = 0x80000080; new_clock >= (clock << 1); clk >>= 1) > - clock <<= 1; > + if (divisor <= 1) { > + clk_sel = 1; > + clk = 0; > + } else { > + clk_sel = 0; > + /* bit7 set: 1/512, ... bit0 set:1/4, all bits clear: 1/2 */ > + clk = roundup_pow_of_two(divisor) >> 2; > + } > > - host->pdata->set_clk_div(host->pdev, (clk >> 22) & 1); > + host->pdata->set_clk_div(host->pdev, clk_sel); > > sd_ctrl_write16(host, CTL_SD_CARD_CLK_CTL, ~CLK_CTL_SCLKEN & > sd_ctrl_read16(host, CTL_SD_CARD_CLK_CTL)); > -- > 2.7.4 >