diff mbox series

[v3,1/3] clk: k210: remove an implicit 64-bit division

Message ID 20230301002657.352637-2-Mr.Bossman075@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series Add RISC-V 32 NOMMU support | expand

Commit Message

Jesse Taube March 1, 2023, 12:26 a.m. UTC 
From: Conor Dooley <conor.dooley@microchip.com>

The K210 clock driver depends on SOC_CANAAN, which is only selectable
when !MMU on RISC-V. !MMU is not possible on 32-bit yet, but patches
have been sent for its enabling. The kernel test robot reported this
implicit 64-bit division there.

Replace the implicit division with an explicit one.

Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/linux-riscv/202301201538.zNlqgE4L-lkp@intel.com/
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
V1->V2:
 - new commit
V2->V3:
 - No change
---
 drivers/clk/clk-k210.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Damien Le Moal March 1, 2023, 1:19 a.m. UTC | #1 
On 3/1/23 09:26, Jesse Taube wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The K210 clock driver depends on SOC_CANAAN, which is only selectable
> when !MMU on RISC-V. !MMU is not possible on 32-bit yet, but patches
> have been sent for its enabling. The kernel test robot reported this
> implicit 64-bit division there.
> 
> Replace the implicit division with an explicit one.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/linux-riscv/202301201538.zNlqgE4L-lkp@intel.com/
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>

Looks OK to me.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Stephen Boyd March 6, 2023, 10:31 p.m. UTC | #2 
Quoting Jesse Taube (2023-02-28 16:26:55)
> diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c
> index 67a7cb3503c3..4eed667eddaf 100644
> --- a/drivers/clk/clk-k210.c
> +++ b/drivers/clk/clk-k210.c
> @@ -495,7 +495,7 @@ static unsigned long k210_pll_get_rate(struct clk_hw *hw,
>         f = FIELD_GET(K210_PLL_CLKF, reg) + 1;
>         od = FIELD_GET(K210_PLL_CLKOD, reg) + 1;
>  
> -       return (u64)parent_rate * f / (r * od);
> +       return div_u64((u64)parent_rate * f, r * od);

The equation 'r * od' can't overflow 32-bits, right?
Conor Dooley March 6, 2023, 10:35 p.m. UTC | #3 
On Mon, Mar 06, 2023 at 02:31:00PM -0800, Stephen Boyd wrote:
> Quoting Jesse Taube (2023-02-28 16:26:55)
> > diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c
> > index 67a7cb3503c3..4eed667eddaf 100644
> > --- a/drivers/clk/clk-k210.c
> > +++ b/drivers/clk/clk-k210.c
> > @@ -495,7 +495,7 @@ static unsigned long k210_pll_get_rate(struct clk_hw *hw,
> >         f = FIELD_GET(K210_PLL_CLKF, reg) + 1;
> >         od = FIELD_GET(K210_PLL_CLKOD, reg) + 1;
> >  
> > -       return (u64)parent_rate * f / (r * od);
> > +       return div_u64((u64)parent_rate * f, r * od);
> 
> The equation 'r * od' can't overflow 32-bits, right?

Yah, I checked that when writing the patch. They're 4-bit fields:
> /*
>  * PLL control register bits.
>  */
> #define K210_PLL_CLKR		GENMASK(3, 0)
> #define K210_PLL_CLKF		GENMASK(9, 4)
> #define K210_PLL_CLKOD		GENMASK(13, 10)

Cheers,
Conor.
Stephen Boyd March 6, 2023, 10:37 p.m. UTC | #4 
Quoting Conor Dooley (2023-03-06 14:35:01)
> On Mon, Mar 06, 2023 at 02:31:00PM -0800, Stephen Boyd wrote:
> > Quoting Jesse Taube (2023-02-28 16:26:55)
> > > diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c
> > > index 67a7cb3503c3..4eed667eddaf 100644
> > > --- a/drivers/clk/clk-k210.c
> > > +++ b/drivers/clk/clk-k210.c
> > > @@ -495,7 +495,7 @@ static unsigned long k210_pll_get_rate(struct clk_hw *hw,
> > >         f = FIELD_GET(K210_PLL_CLKF, reg) + 1;
> > >         od = FIELD_GET(K210_PLL_CLKOD, reg) + 1;
> > >  
> > > -       return (u64)parent_rate * f / (r * od);
> > > +       return div_u64((u64)parent_rate * f, r * od);
> > 
> > The equation 'r * od' can't overflow 32-bits, right?
> 
> Yah, I checked that when writing the patch. They're 4-bit fields:

Awesome
Stephen Boyd March 6, 2023, 10:41 p.m. UTC | #5 
Quoting Jesse Taube (2023-02-28 16:26:55)
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The K210 clock driver depends on SOC_CANAAN, which is only selectable
> when !MMU on RISC-V. !MMU is not possible on 32-bit yet, but patches
> have been sent for its enabling. The kernel test robot reported this
> implicit 64-bit division there.
> 
> Replace the implicit division with an explicit one.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Link: https://lore.kernel.org/linux-riscv/202301201538.zNlqgE4L-lkp@intel.com/
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---

Seems better to merge this one-liner earlier to unblock 32-bit.

Applied to clk-fixes
Palmer Dabbelt March 6, 2023, 10:48 p.m. UTC | #6 
On Mon, 06 Mar 2023 14:41:11 PST (-0800), sboyd@kernel.org wrote:
> Quoting Jesse Taube (2023-02-28 16:26:55)
>> From: Conor Dooley <conor.dooley@microchip.com>
>> 
>> The K210 clock driver depends on SOC_CANAAN, which is only selectable
>> when !MMU on RISC-V. !MMU is not possible on 32-bit yet, but patches
>> have been sent for its enabling. The kernel test robot reported this
>> implicit 64-bit division there.
>> 
>> Replace the implicit division with an explicit one.
>> 
>> Reported-by: kernel test robot <lkp@intel.com>
>> Link: https://lore.kernel.org/linux-riscv/202301201538.zNlqgE4L-lkp@intel.com/
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>> ---
>
> Seems better to merge this one-liner earlier to unblock 32-bit.
>
> Applied to clk-fixes

Thanks!
diff mbox series

Patch

diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c
index 67a7cb3503c3..4eed667eddaf 100644
--- a/drivers/clk/clk-k210.c
+++ b/drivers/clk/clk-k210.c
@@ -495,7 +495,7 @@  static unsigned long k210_pll_get_rate(struct clk_hw *hw,
 	f = FIELD_GET(K210_PLL_CLKF, reg) + 1;
 	od = FIELD_GET(K210_PLL_CLKOD, reg) + 1;
 
-	return (u64)parent_rate * f / (r * od);
+	return div_u64((u64)parent_rate * f, r * od);
 }
 
 static const struct clk_ops k210_pll_ops = {