From mboxrd@z Thu Jan 1 00:00:00 1970 From: jbrunet@baylibre.com (Jerome Brunet) Date: Tue, 15 May 2018 16:48:40 +0200 Subject: [PATCH meson-clk-next] clk: meson: meson8b: enable CLK_MESON_MPLL_ROUND_CLOSEST for all MPLLs In-Reply-To: <1524927694.28715.2.camel@baylibre.com> References: <20180428132316.20108-1-martin.blumenstingl@googlemail.com> <1524927694.28715.2.camel@baylibre.com> Message-ID: <1526395720.2897.51.camel@baylibre.com> To: linus-amlogic@lists.infradead.org List-Id: linus-amlogic.lists.infradead.org On Sat, 2018-04-28 at 17:01 +0200, Jerome Brunet wrote: > On Sat, 2018-04-28 at 15:23 +0200, Martin Blumenstingl wrote: > > Until recently the clk-mpll implementation always used > > DIV_ROUND_UP_ULL(). However, since commit 1b0e500dc606e9 ("clk: meson: > > mpll: add round closest support") the default was changed to round down > > the SDM divisor. > > This broke RGMII Ethernet on Meson8b (which uses MPLL2 as RGMII TX > > clock). With the old implementation the MPLL2 output was 249999701Hz, > > but with "round closest" disabled the output is 212500000Hz. > > > > Enabling CLK_MESON_MPLL_ROUND_CLOSEST for all MPLL2 clocks on Meson8b > > fixes this because we now get the same 249999701Hz from MPLL2 as before. > > It should either be rounding up or down, not down or 'more down' > I guess I need to revisit this series. In the mean time, I'll drop it. > Thanks for reporting the issue Hi Martin, I think I found the problem in the mpll round closest code. It does not seem to be a 32bit overflow, as we had before. Apparently, I got confused with rounding up and down stuff :( I'll post a v2 soon. I'll wait for your feedback before applying it this time Thanks again for reporting the issue. Cheers Jerome > > > > > Fixes: 1b0e500dc606e9 ("clk: meson: mpll: add round closest support") > > Signed-off-by: Martin Blumenstingl > > --- > > drivers/clk/meson/meson8b.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > > index d0524ec71aad..2639a892a5d4 100644 > > --- a/drivers/clk/meson/meson8b.c > > +++ b/drivers/clk/meson/meson8b.c > > @@ -382,6 +382,7 @@ static struct clk_regmap meson8b_mpll0_div = { > > .width = 1, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll0_div", > > @@ -423,6 +424,7 @@ static struct clk_regmap meson8b_mpll1_div = { > > .width = 9, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll1_div", > > @@ -464,6 +466,7 @@ static struct clk_regmap meson8b_mpll2_div = { > > .width = 9, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll2_div", > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: Message-ID: <1526395720.2897.51.camel@baylibre.com> Subject: Re: [PATCH meson-clk-next] clk: meson: meson8b: enable CLK_MESON_MPLL_ROUND_CLOSEST for all MPLLs From: Jerome Brunet To: Martin Blumenstingl , narmstrong@baylibre.com, linux-amlogic@lists.infradead.org Cc: mturquette@baylibre.com, sboyd@kernel.org, carlo@caione.org, khilman@baylibre.com, linux-clk@vger.kernel.org Date: Tue, 15 May 2018 16:48:40 +0200 In-Reply-To: <1524927694.28715.2.camel@baylibre.com> References: <20180428132316.20108-1-martin.blumenstingl@googlemail.com> <1524927694.28715.2.camel@baylibre.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 List-ID: On Sat, 2018-04-28 at 17:01 +0200, Jerome Brunet wrote: > On Sat, 2018-04-28 at 15:23 +0200, Martin Blumenstingl wrote: > > Until recently the clk-mpll implementation always used > > DIV_ROUND_UP_ULL(). However, since commit 1b0e500dc606e9 ("clk: meson: > > mpll: add round closest support") the default was changed to round down > > the SDM divisor. > > This broke RGMII Ethernet on Meson8b (which uses MPLL2 as RGMII TX > > clock). With the old implementation the MPLL2 output was 249999701Hz, > > but with "round closest" disabled the output is 212500000Hz. > > > > Enabling CLK_MESON_MPLL_ROUND_CLOSEST for all MPLL2 clocks on Meson8b > > fixes this because we now get the same 249999701Hz from MPLL2 as before. > > It should either be rounding up or down, not down or 'more down' > I guess I need to revisit this series. In the mean time, I'll drop it. > Thanks for reporting the issue Hi Martin, I think I found the problem in the mpll round closest code. It does not seem to be a 32bit overflow, as we had before. Apparently, I got confused with rounding up and down stuff :( I'll post a v2 soon. I'll wait for your feedback before applying it this time Thanks again for reporting the issue. Cheers Jerome > > > > > Fixes: 1b0e500dc606e9 ("clk: meson: mpll: add round closest support") > > Signed-off-by: Martin Blumenstingl > > --- > > drivers/clk/meson/meson8b.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > > index d0524ec71aad..2639a892a5d4 100644 > > --- a/drivers/clk/meson/meson8b.c > > +++ b/drivers/clk/meson/meson8b.c > > @@ -382,6 +382,7 @@ static struct clk_regmap meson8b_mpll0_div = { > > .width = 1, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll0_div", > > @@ -423,6 +424,7 @@ static struct clk_regmap meson8b_mpll1_div = { > > .width = 9, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll1_div", > > @@ -464,6 +466,7 @@ static struct clk_regmap meson8b_mpll2_div = { > > .width = 9, > > }, > > .lock = &meson_clk_lock, > > + .flags = CLK_MESON_MPLL_ROUND_CLOSEST, > > }, > > .hw.init = &(struct clk_init_data){ > > .name = "mpll2_div", > >