+David
On Wed, Oct 04, 2023 at 09:50:09AM +0200, Geert Uytterhoeven wrote:
> On Wed, Oct 4, 2023 at 8:42 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > The min_t() is often used as a shortcut for clamp(). Secondly, the
> > BIT(16) - 1 is specifically used as the value related to the bits in the
> > hardware and u16 is a software type that coincidentally has the same
> > maximum as the above mentioned bitfield.
>
> Technically it is two byte-sized registers forming a 16-bit field ;-)
>
> > Replace min_t()->clamp() in vc3_pll_round_rate().
> >
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > drivers/clk/clk-versaclock3.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-versaclock3.c b/drivers/clk/clk-versaclock3.c
> > index 3d7de355f8f6..50772f61096f 100644
> > --- a/drivers/clk/clk-versaclock3.c
> > +++ b/drivers/clk/clk-versaclock3.c
> > @@ -402,7 +402,7 @@ static long vc3_pll_round_rate(struct clk_hw *hw, unsigned long rate,
> > div_frc = rate % *parent_rate;
> > div_frc *= BIT(16) - 1;
> >
> > - vc3->div_frc = min_t(u64, div64_ul(div_frc, *parent_rate), U16_MAX);
> > + vc3->div_frc = clamp(div64_ul(div_frc, *parent_rate), 0, BIT(16) - 1);
>
> I'm not sure this is actually an improvement...
That's what Linus actually suggested to do.
> While I agree "BIT(16) - 1" matches the expression two lines above,
> I find it harder to read.
> Perhaps introducing a VC3_PLL2_FB_FRC_DIV_MAX definition may help.
Either way, but U16_MAX is really semantically wrong here.
> BTW, if the hardware wouldn't use two byte-sized registers, but a real
> bitifield, one could use FIELD_GET(mask, mask) instead.
> Second, clamping an unsigned value to zero is futile, and opens us to
> warnings like:
>
> warning: comparison of unsigned expression in ‘>= 0’ is always
> true [-Wtype-limits]
David, is your series fix this as well?
> > rate = (*parent_rate *
> > (vc3->div_int * VC3_2_POW_16 + vc3->div_frc) / VC3_2_POW_16);
> > } else {
--
With Best Regards,
Andy Shevchenko
