[PATCH v1 0/2] Add RISC-V 32 NOMMU support

[PATCH v1 0/2] Add RISC-V 32 NOMMU support

[Jesse Taube]

This patch-set aims to add NOMMU support to RV32.
Many people want to build simple emulators or HDL
models of RISC-V this patch makes it posible to
run linux on them.

Yimin Gu is the original author of this set.
Submitted here:
https://lists.buildroot.org/pipermail/buildroot/2022-November/656134.html

Though Jesse T rewrote the Dconf.

The new set:
https://lists.buildroot.org/pipermail/buildroot/2022-December/658258.html


Jesse Taube (1):
  riscv: configs: Add nommu decfconfig for RV32

Yimin Gu (1):
  riscv: Kconfig: Allow RV32 to build with no MMU

 arch/riscv/Kconfig                           |  5 ++---
 arch/riscv/configs/rv32_nommu_virt_defconfig | 16 ++++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 arch/riscv/configs/rv32_nommu_virt_defconfig

-- 
2.39.0

[PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Jesse Taube]

From: Yimin Gu <ustcymgu@gmail.com>

Some RISC-V 32bit ores do not have an MMU, and the kernel should be
able to build for them. This patch enables the RV32 to be built with
no MMU support.

Signed-off-by: Yimin Gu <ustcymgu@gmail.com>
CC: Jesse Taube <Mr.Bossman075@gmail.com>
Tested-By: Waldemar Brodkorb <wbx@openadk.org>
Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
---
 arch/riscv/Kconfig | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 59d18881f35b..49759dbe6a8f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -163,8 +163,8 @@ config MMU
 
 config PAGE_OFFSET
 	hex
-	default 0xC0000000 if 32BIT
-	default 0x80000000 if 64BIT && !MMU
+	default 0xC0000000 if 32BIT && MMU
+	default 0x80000000 if !MMU
 	default 0xff60000000000000 if 64BIT
 
 config KASAN_SHADOW_OFFSET
@@ -262,7 +262,6 @@ config ARCH_RV32I
 	select GENERIC_LIB_ASHRDI3
 	select GENERIC_LIB_LSHRDI3
 	select GENERIC_LIB_UCMPDI2
-	select MMU
 
 config ARCH_RV64I
 	bool "RV64I"
-- 
2.39.0

[PATCH v1 2/2] riscv: configs: Add nommu decfconfig for RV32

[Jesse Taube]

32bit risc-v can be configured to run without MMU. This patch adds
an example configuration for RV32 nommu virtual machine.

Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
Cc: Yimin Gu <ustcymgu@gmail.com>
---
 arch/riscv/configs/rv32_nommu_virt_defconfig | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
 create mode 100644 arch/riscv/configs/rv32_nommu_virt_defconfig

diff --git a/arch/riscv/configs/rv32_nommu_virt_defconfig b/arch/riscv/configs/rv32_nommu_virt_defconfig
new file mode 100644
index 000000000000..460907253a80
--- /dev/null
+++ b/arch/riscv/configs/rv32_nommu_virt_defconfig
@@ -0,0 +1,16 @@
+CONFIG_BLK_DEV_INITRD=y
+# CONFIG_MMU is not set
+CONFIG_COMPAT_32BIT_TIME=y
+CONFIG_SOC_VIRT=y
+CONFIG_NONPORTABLE=y
+CONFIG_ARCH_RV32I=y
+CONFIG_BINFMT_FLAT=y
+CONFIG_SLOB=y
+CONFIG_VIRTIO_BLK=y
+CONFIG_SERIAL_8250=y
+CONFIG_SERIAL_8250_CONSOLE=y
+CONFIG_SERIAL_OF_PLATFORM=y
+CONFIG_VIRTIO_MMIO=y
+CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
+CONFIG_EXT2_FS=y
+CONFIG_PRINTK_TIME=y
-- 
2.39.0

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[kernel test robot]

Hi Jesse,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.2-rc4 next-20230120]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jesse-Taube/riscv-Kconfig-Allow-RV32-to-build-with-no-MMU/20230119-132822
patch link:    https://lore.kernel.org/r/20230119052642.1112171-2-Mr.Bossman075%40gmail.com
patch subject: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU
config: riscv-randconfig-c003-20230119 (https://download.01.org/0day-ci/archive/20230120/202301201538.zNlqgE4L-lkp@intel.com/config)
compiler: riscv32-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/4c6876d890ddeab1fb2cb42889027c6d1dbdd02f
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jesse-Taube/riscv-Kconfig-Allow-RV32-to-build-with-no-MMU/20230119-132822
        git checkout 4c6876d890ddeab1fb2cb42889027c6d1dbdd02f
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   riscv32-linux-ld: drivers/clk/clk-k210.o: in function `k210_pll_get_rate':
>> drivers/clk/clk-k210.c:498: undefined reference to `__udivdi3'


vim +498 drivers/clk/clk-k210.c

c6ca7616f7d5c2 Damien Le Moal 2021-02-10  480  
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  481  static unsigned long k210_pll_get_rate(struct clk_hw *hw,
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  482  				       unsigned long parent_rate)
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  483  {
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  484  	struct k210_pll *pll = to_k210_pll(hw);
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  485  	u32 reg = readl(pll->reg);
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  486  	u32 r, f, od;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  487  
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  488  	if (reg & K210_PLL_BYPASS)
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  489  		return parent_rate;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  490  
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  491  	if (!(reg & K210_PLL_PWRD))
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  492  		return 0;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  493  
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  494  	r = FIELD_GET(K210_PLL_CLKR, reg) + 1;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  495  	f = FIELD_GET(K210_PLL_CLKF, reg) + 1;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  496  	od = FIELD_GET(K210_PLL_CLKOD, reg) + 1;
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  497  
c6ca7616f7d5c2 Damien Le Moal 2021-02-10 @498  	return (u64)parent_rate * f / (r * od);
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  499  }
c6ca7616f7d5c2 Damien Le Moal 2021-02-10  500  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

Hello!

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.

On Thu, Jan 19, 2023 at 12:26:41AM -0500, Jesse Taube wrote:
> From: Yimin Gu <ustcymgu@gmail.com>
> 
> Some RISC-V 32bit ores do not have an MMU, and the kernel should be

 s/ores/cores

> able to build for them. This patch enables the RV32 to be built with
> no MMU support.
> 
> Signed-off-by: Yimin Gu <ustcymgu@gmail.com>
> CC: Jesse Taube <Mr.Bossman075@gmail.com>
> Tested-By: Waldemar Brodkorb <wbx@openadk.org>

And the automation complains that this tag is not "Tested-by:"

Thanks,
Conor.

> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> ---
>  arch/riscv/Kconfig | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 59d18881f35b..49759dbe6a8f 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -163,8 +163,8 @@ config MMU
>  
>  config PAGE_OFFSET
>  	hex
> -	default 0xC0000000 if 32BIT
> -	default 0x80000000 if 64BIT && !MMU
> +	default 0xC0000000 if 32BIT && MMU
> +	default 0x80000000 if !MMU
>  	default 0xff60000000000000 if 64BIT
>  
>  config KASAN_SHADOW_OFFSET
> @@ -262,7 +262,6 @@ config ARCH_RV32I
>  	select GENERIC_LIB_ASHRDI3
>  	select GENERIC_LIB_LSHRDI3
>  	select GENERIC_LIB_UCMPDI2
> -	select MMU
>  
>  config ARCH_RV64I
>  	bool "RV64I"
> -- 
> 2.39.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 2/2] riscv: configs: Add nommu decfconfig for RV32

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

Hey Jesse,

Same here w/ a single nit.
I'll leave judging whether supporting rv32 nommu is worthwhile to others!

On Thu, Jan 19, 2023 at 12:26:42AM -0500, Jesse Taube wrote:
> riscv: configs: Add nommu decfconfig for RV32

typo: defconfig

Thanks,
Conor.

> 32bit risc-v can be configured to run without MMU. This patch adds
> an example configuration for RV32 nommu virtual machine.
> 
> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
> Cc: Yimin Gu <ustcymgu@gmail.com>
> ---
>  arch/riscv/configs/rv32_nommu_virt_defconfig | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>  create mode 100644 arch/riscv/configs/rv32_nommu_virt_defconfig
> 
> diff --git a/arch/riscv/configs/rv32_nommu_virt_defconfig b/arch/riscv/configs/rv32_nommu_virt_defconfig
> new file mode 100644
> index 000000000000..460907253a80
> --- /dev/null
> +++ b/arch/riscv/configs/rv32_nommu_virt_defconfig
> @@ -0,0 +1,16 @@
> +CONFIG_BLK_DEV_INITRD=y
> +# CONFIG_MMU is not set
> +CONFIG_COMPAT_32BIT_TIME=y
> +CONFIG_SOC_VIRT=y
> +CONFIG_NONPORTABLE=y
> +CONFIG_ARCH_RV32I=y
> +CONFIG_BINFMT_FLAT=y
> +CONFIG_SLOB=y
> +CONFIG_VIRTIO_BLK=y
> +CONFIG_SERIAL_8250=y
> +CONFIG_SERIAL_8250_CONSOLE=y
> +CONFIG_SERIAL_OF_PLATFORM=y
> +CONFIG_VIRTIO_MMIO=y
> +CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
> +CONFIG_EXT2_FS=y
> +CONFIG_PRINTK_TIME=y
> -- 
> 2.39.0
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Jesse Taube]

On 1/20/23 02:59, Conor Dooley wrote:
> Hello!
> 
> 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. 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

> On Thu, Jan 19, 2023 at 12:26:41AM -0500, Jesse Taube wrote:
>> From: Yimin Gu <ustcymgu@gmail.com>
>>
>> Some RISC-V 32bit ores do not have an MMU, and the kernel should be
> 
>   s/ores/cores
OH thanks sorry for the spelling mistakes.

Thanks,
Jesse Taube
>> able to build for them. This patch enables the RV32 to be built with
>> no MMU support.
>>
>> Signed-off-by: Yimin Gu <ustcymgu@gmail.com>
>> CC: Jesse Taube <Mr.Bossman075@gmail.com>
>> Tested-By: Waldemar Brodkorb <wbx@openadk.org>
> 
> And the automation complains that this tag is not "Tested-by:"
> 
> Thanks,
> Conor.
> 
>> Signed-off-by: Jesse Taube <Mr.Bossman075@gmail.com>
>> ---
>>   arch/riscv/Kconfig | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 59d18881f35b..49759dbe6a8f 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -163,8 +163,8 @@ config MMU
>>   
>>   config PAGE_OFFSET
>>   	hex
>> -	default 0xC0000000 if 32BIT
>> -	default 0x80000000 if 64BIT && !MMU
>> +	default 0xC0000000 if 32BIT && MMU
>> +	default 0x80000000 if !MMU
>>   	default 0xff60000000000000 if 64BIT
>>   
>>   config KASAN_SHADOW_OFFSET
>> @@ -262,7 +262,6 @@ config ARCH_RV32I
>>   	select GENERIC_LIB_ASHRDI3
>>   	select GENERIC_LIB_LSHRDI3
>>   	select GENERIC_LIB_UCMPDI2
>> -	select MMU
>>   
>>   config ARCH_RV64I
>>   	bool "RV64I"
>> -- 
>> 2.39.0
>>
>>

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]

Hey Jesse,

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);
 }
 
 static const struct clk_ops k210_pll_ops = {
-- 
2.39.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3376 bytes --]

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Jesse Taube]

On 1/20/23 15:48, Conor Dooley wrote:
> 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.
Oh I missed the bots email oops.

undefined reference to `__udivdi3'
Poor linker isn't linking with libgcc

>>
>> 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);
Ah that's a much better fix, shall I prepend this to the set and author you?

> 
>>   }
>>   
>>   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
> 

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

> > > 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);


> Ah that's a much better fix, shall I prepend this to the set and author you?

Uh, I'd rather check that it is a functional fix first!
I have the board, so I'll give it a go tomorrow and let you know.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v1 1/2] riscv: Kconfig: Allow RV32 to build with no MMU

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On Fri, Jan 20, 2023 at 08:57:38PM +0000, Conor Dooley wrote:
> > > > 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);
> 
> 
> > Ah that's a much better fix, shall I prepend this to the set and author you?
> 
> Uh, I'd rather check that it is a functional fix first!
> I have the board, so I'll give it a go tomorrow and let you know.

Gave it a go, board still works with revised version of this change in
place.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]