Messages in this thread Patch in this message | | | Date | Fri, 20 Jan 2023 20:48:07 +0000 | From | Conor Dooley <> | Subject | Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU |
| |
On Fri, Jan 20, 2023 at 08:44:10PM +0000, Conor Dooley wrote: > On Fri, Jan 20, 2023 at 12:39:06PM -0500, Jesse Taube wrote: > > On 1/20/23 02:59, Conor Dooley wrote: > > > Since you'll have to re-submit, making sure that allowing !MMU on rv32 > > > doesn't break the build due to canaan k210 drivers being enabled despite > > > relying on 64-bit divisions, I've got some nits for you. > > Not sure what driver needs 64bit, but sense !MMU was only selected by 64BIT. > > LKP reported a build error for it: > https://lore.kernel.org/linux-riscv/202301201538.zNlqgE4L-lkp@intel.com/ > > > This should work. > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs > > index 69774bb362d6..b9835b8ede86 100644 > > --- a/arch/riscv/Kconfig.socs > > +++ b/arch/riscv/Kconfig.socs > > @@ -43,7 +43,7 @@ config SOC_VIRT > > > > config SOC_CANAAN > > bool "Canaan Kendryte K210 SoC" > > - depends on !MMU > > + depends on !MMU && 64BIT > > select CLINT_TIMER if RISCV_M_MODE > > select SERIAL_SIFIVE if TTY > > select SERIAL_SIFIVE_CONSOLE if TTY > > I don't think this is the correct fix for the problem - the drivers > really should not do implicit 64-bit divisions IMO. > Linux has division helpers for them in math64.h. > None of the other SoCs have a dependency on 64BIT and I'd not been keen > on adding on here. > > I suspect the fix is as simple as the below, but I'd need to go test it. > > Thanks, > Conor. > > --- 8< --- > From ecfa79ad1b24f68cfccb77d666e443293d52d066 Mon Sep 17 00:00:00 2001 > From: Conor Dooley <conor.dooley@microchip.com> > Date: Fri, 20 Jan 2023 20:36:29 +0000 > Subject: [PATCH] clk: k210: remove an implicit 64-bit division > > 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> > --- > Since it was always guarded such that it only ever built for 64-bit, I > am not sure that a fixes tag is needed, but it would be: > Fixes: c6ca7616f7d5 ("clk: Add RISC-V Canaan Kendryte K210 clock driver") > --- > drivers/clk/clk-k210.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/clk-k210.c b/drivers/clk/clk-k210.c > index 67a7cb3503c3..17c5bfb384ad 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(parent_rate * f, r * od);
Nope, that's wrong. I omitted the cast...
return div_u64((u64)parent_rate * f, r * od);
> } > > static const struct clk_ops k210_pll_ops = { > -- > 2.39.0 >
> _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv [unhandled content-type:application/pgp-signature] | |