| Message ID | 1534999460-15111-7-git-send-email-yamada.masahiro@socionext.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | mmc: tmio: refactor TMIO core a bit and add UniPhier SD/eMMC controller support | expand |
> + 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.
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 Thu, Aug 30, 2018 at 01:51:49PM +0200, Ulf Hansson wrote: > 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? I can resend this as a proper patch if you prefer it to be handled incrementally.
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 >
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));
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> --- Changes in v3: None Changes in v2: None drivers/mmc/host/tmio_mmc.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)