Re: [PATCH 1/2] clk: divider: Fix the divisor for rates above 4.29 GHz

[Sebastian Reichel]

Hi,

On Fri, Nov 06, 2026 at 06:00:29PM +0000, David Laight wrote:
> On Fri, 06 Nov 2026 16:51:50 +0100
> Jerome Brunet <jbrunet@xxxxxxxxxxxx> wrote:
> 
> > On 2026-11-04 16:52:04-05:00, Brian Masney wrote:
> > > > DIV_ROUND_UP_ULL() takes a 32-bit divisor, so on 64-bit platforms a
> > > > requested rate above 4.29 GHz is truncated. A divider fed by a 5.94 GHz
> > > > PLL then computes a divisor of 7, not 1, for 5.94 GHz.
> > > > 
> > > > Use DIV64_U64_ROUND_UP(), which takes a 64-bit divisor.
> > > > 
> > > > Fixes: 9556f9dad8f5 ("clk: divider: handle integer overflow when dividing large clock rates")
> > > > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > > > Signed-off-by: Carlo Caione <ccaione@xxxxxxxxxxxx>  
> > > 
> > > Reviewed-by: Brian Masney <masneyb@xxxxxxxxxx>
> > > 
> > > Since this is a core change to the framework, I'd also like to see what
> > > Jerome thinks before we merge this.  
> > 
> > I've had not problem with the change initially but then the AI told me that
> > it brings back the overflow problem fixed by 9556f9dad8f5.
> > 
> > At first I thought it was crazy because it would mean parent_rate + rate is
> > more than 18 billion GHz ... BUT, a common way to ask for the maximum rate a
> > clock supports is clk_round_rate(clk, ULONG_MAX). This could trigger the overflow
> > and there are instance of this in the kernel right now.
> > 
> > A way to make the calculation without the overflow could be:
> > parent_rate / rate + (parent_rate % rate ? 1 : 0)
> > 
> 
> You just want:
> 	parent_rate ? (parent_rate - 1) / rate + 1 : 0
> it is even likely here that you can assume parent_rate in non-zero.

Looks sensible. I suggest to just add this locally in
drivers/clk/clk-divider.c until another user turns up:

static unsigned long div_clk_round_up(unsigned long dividend, unsigned long divisor)
{
    return dividend ? (dividend - 1) / divisor + 1 : 0;
}

-- Sebastian

Attachment: signature.asc
Description: PGP signature