Re: [PATCH 1/1] riscv: remove dram_init_banksize()

2023-10-06 Thread Anup Patel
On Tue, Sep 26, 2023 at 12:47 PM Heinrich Schuchardt
 wrote:
>
> Remove dram_init_banksize() on the architecture level.
>
> Limiting used RAM to under 4 GiB is only necessary for CPUs which have a
> DMA issue. SoC specific code already exists for FU540, FU740, JH7110.
>
> Not all RISC-V boards will have memory below 4 GiB.
>
> A weak implementation of dram_init_banksize() exists in common/board_f.c.
>
> See the discussion in
> https://lore.kernel.org/u-boot/545fe813-cb1e-469c-a131-0025c77ae...@canonical.com/T/
>
> Signed-off-by: Heinrich Schuchardt 

Looks good to me.

Reviewed-by: Anup Patel 

Regards,
Anup

> ---
>  arch/riscv/cpu/generic/dram.c | 16 
>  1 file changed, 16 deletions(-)
>
> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> index 94d8018407..1b51bae9b6 100644
> --- a/arch/riscv/cpu/generic/dram.c
> +++ b/arch/riscv/cpu/generic/dram.c
> @@ -20,19 +20,3 @@ int dram_init_banksize(void)
>  {
> return fdtdec_setup_memory_banksize();
>  }
> -
> -phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
> -{
> -   /*
> -* Ensure that we run from first 4GB so that all
> -* addresses used by U-Boot are 32bit addresses.
> -*
> -* This in-turn ensures that 32bit DMA capable
> -* devices work fine because DMA mapping APIs will
> -* provide 32bit DMA addresses only.
> -*/
> -   if (gd->ram_top >= SZ_4G)
> -   return SZ_4G - 1;
> -
> -   return gd->ram_top;
> -}
> --
> 2.40.1
>


Re: [PATCH 1/1] riscv: remove dram_init_banksize()

2023-10-04 Thread Heinrich Schuchardt

On 9/26/23 09:46, Wu, Fei wrote:

On 9/26/2023 3:16 PM, Heinrich Schuchardt wrote:

Remove dram_init_banksize() on the architecture level.

Limiting used RAM to under 4 GiB is only necessary for CPUs which have a
DMA issue. SoC specific code already exists for FU540, FU740, JH7110.

Not all RISC-V boards will have memory below 4 GiB.

A weak implementation of dram_init_banksize() exists in common/board_f.c.

See the discussion in
https://lore.kernel.org/u-boot/545fe813-cb1e-469c-a131-0025c77ae...@canonical.com/T/


Thanks you Heinrich, this patch itself is okay to me, but it still
doesn't address the problem in that thread. In setup_dest_addr():

#ifdef CFG_SYS_SDRAM_BASE
 gd->ram_base = CFG_SYS_SDRAM_BASE;
#endif
 gd->ram_top = gd->ram_base + get_effective_memsize();
 gd->ram_top = board_get_usable_ram_top(gd->mon_len);

ram_base is modified in #ifdef so that ram_base and
get_effective_memsize() is not well paired. e.g. two ranges of memory
[2GB, 3GB) and [4GB, 6GB), ram_base is set to 2GB of 1st range and
get_effective_memsize() returns 2GB of 2nd range, ram_top will be set to
4GB which is not desired. Will you address this too?


No, that is an independent issue. I only want to get rid of the generic 
restriction to memory below 4 GiB on RISC-V.


I would recommend to set ram_top to the highest available memory address 
of the highest RAM bank with some special handling of the ARM LPAE case 
or 32bit in general. But this is not a RISC-V specific topic.


Best regards

Heinrich



Best Regards,
Fei.


Signed-off-by: Heinrich Schuchardt 
---
  arch/riscv/cpu/generic/dram.c | 16 
  1 file changed, 16 deletions(-)

diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
index 94d8018407..1b51bae9b6 100644
--- a/arch/riscv/cpu/generic/dram.c
+++ b/arch/riscv/cpu/generic/dram.c
@@ -20,19 +20,3 @@ int dram_init_banksize(void)
  {
return fdtdec_setup_memory_banksize();
  }
-
-phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
-{
-   /*
-* Ensure that we run from first 4GB so that all
-* addresses used by U-Boot are 32bit addresses.
-*
-* This in-turn ensures that 32bit DMA capable
-* devices work fine because DMA mapping APIs will
-* provide 32bit DMA addresses only.
-*/
-   if (gd->ram_top >= SZ_4G)
-   return SZ_4G - 1;
-
-   return gd->ram_top;
-}






Re: [PATCH 1/1] riscv: remove dram_init_banksize()

2023-09-26 Thread Wu, Fei
On 9/26/2023 3:16 PM, Heinrich Schuchardt wrote:
> Remove dram_init_banksize() on the architecture level.
> 
> Limiting used RAM to under 4 GiB is only necessary for CPUs which have a
> DMA issue. SoC specific code already exists for FU540, FU740, JH7110.
> 
> Not all RISC-V boards will have memory below 4 GiB.
> 
> A weak implementation of dram_init_banksize() exists in common/board_f.c.
> 
> See the discussion in
> https://lore.kernel.org/u-boot/545fe813-cb1e-469c-a131-0025c77ae...@canonical.com/T/
> 
Thanks you Heinrich, this patch itself is okay to me, but it still
doesn't address the problem in that thread. In setup_dest_addr():

#ifdef CFG_SYS_SDRAM_BASE
gd->ram_base = CFG_SYS_SDRAM_BASE;
#endif
gd->ram_top = gd->ram_base + get_effective_memsize();
gd->ram_top = board_get_usable_ram_top(gd->mon_len);

ram_base is modified in #ifdef so that ram_base and
get_effective_memsize() is not well paired. e.g. two ranges of memory
[2GB, 3GB) and [4GB, 6GB), ram_base is set to 2GB of 1st range and
get_effective_memsize() returns 2GB of 2nd range, ram_top will be set to
4GB which is not desired. Will you address this too?

Best Regards,
Fei.

> Signed-off-by: Heinrich Schuchardt 
> ---
>  arch/riscv/cpu/generic/dram.c | 16 
>  1 file changed, 16 deletions(-)
> 
> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> index 94d8018407..1b51bae9b6 100644
> --- a/arch/riscv/cpu/generic/dram.c
> +++ b/arch/riscv/cpu/generic/dram.c
> @@ -20,19 +20,3 @@ int dram_init_banksize(void)
>  {
>   return fdtdec_setup_memory_banksize();
>  }
> -
> -phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
> -{
> - /*
> -  * Ensure that we run from first 4GB so that all
> -  * addresses used by U-Boot are 32bit addresses.
> -  *
> -  * This in-turn ensures that 32bit DMA capable
> -  * devices work fine because DMA mapping APIs will
> -  * provide 32bit DMA addresses only.
> -  */
> - if (gd->ram_top >= SZ_4G)
> - return SZ_4G - 1;
> -
> - return gd->ram_top;
> -}



[PATCH 1/1] riscv: remove dram_init_banksize()

2023-09-26 Thread Heinrich Schuchardt
Remove dram_init_banksize() on the architecture level.

Limiting used RAM to under 4 GiB is only necessary for CPUs which have a
DMA issue. SoC specific code already exists for FU540, FU740, JH7110.

Not all RISC-V boards will have memory below 4 GiB.

A weak implementation of dram_init_banksize() exists in common/board_f.c.

See the discussion in
https://lore.kernel.org/u-boot/545fe813-cb1e-469c-a131-0025c77ae...@canonical.com/T/

Signed-off-by: Heinrich Schuchardt 
---
 arch/riscv/cpu/generic/dram.c | 16 
 1 file changed, 16 deletions(-)

diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
index 94d8018407..1b51bae9b6 100644
--- a/arch/riscv/cpu/generic/dram.c
+++ b/arch/riscv/cpu/generic/dram.c
@@ -20,19 +20,3 @@ int dram_init_banksize(void)
 {
return fdtdec_setup_memory_banksize();
 }
-
-phys_addr_t board_get_usable_ram_top(phys_size_t total_size)
-{
-   /*
-* Ensure that we run from first 4GB so that all
-* addresses used by U-Boot are 32bit addresses.
-*
-* This in-turn ensures that 32bit DMA capable
-* devices work fine because DMA mapping APIs will
-* provide 32bit DMA addresses only.
-*/
-   if (gd->ram_top >= SZ_4G)
-   return SZ_4G - 1;
-
-   return gd->ram_top;
-}
-- 
2.40.1