diff mbox series

cmd: cache: Fix non-cached memory cachability

Message ID 20200423160155.9258-1-patrice.chotard@st.com
State Superseded
Headers show
Series cmd: cache: Fix non-cached memory cachability | expand

Commit Message

Patrice CHOTARD April 23, 2020, 4:01 p.m. UTC
If dcache is switched OFF to ON state and if non-cached memory is
used, this non-cached memory must be re-declared as uncached to mmu
each time dcache is set ON.

Issue found on STM32MP1 platform using dwc_eth_qos ethernet driver,
when going from dcache OFF to dcache ON state, ethernet driver issued
TX timeout errors when performing dhcp or ping.

It can be reproduced with the following sequence:

dhcp
while true ; do
  ping 192.168.1.300 ;
  dcache off ;
  ping 192.168.1.300 ;
  dcache on ;
done

Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
Cc: Marek Vasut <marex at denx.de>
Cc: Joe Hershberger <joe.hershberger at ni.com>
Cc: Ramon Fried <rfried.dev at gmail.com>
Cc: Stephen Warren <swarren at nvidia.com>
---

 arch/arm/include/asm/system.h |  1 +
 arch/arm/lib/cache.c          | 13 ++++++++++---
 cmd/cache.c                   |  3 +++
 3 files changed, 14 insertions(+), 3 deletions(-)

Comments

Marek Vasut April 27, 2020, 10:22 a.m. UTC | #1
On 4/23/20 6:01 PM, Patrice Chotard wrote:

[...]

> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> index 007d4ebc49..7f3cfb407c 100644
> --- a/arch/arm/lib/cache.c
> +++ b/arch/arm/lib/cache.c
> @@ -73,6 +73,15 @@ static unsigned long noncached_start;
>  static unsigned long noncached_end;
>  static unsigned long noncached_next;
>  
> +void noncached_set_region(void)
> +{

Make this a __weak function and let architectures override it.

> +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
> +	mmu_set_region_dcache_behaviour(noncached_start,
> +					noncached_end - noncached_start,
> +					DCACHE_OFF);
> +#endif
> +}
> +
>  void noncached_init(void)
>  {
>  	phys_addr_t start, end;
> @@ -89,9 +98,7 @@ void noncached_init(void)
>  	noncached_end = end;
>  	noncached_next = start;
>  
> -#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
> -	mmu_set_region_dcache_behaviour(noncached_start, size, DCACHE_OFF);
> -#endif
> +	noncached_set_region();
>  }
>  
>  phys_addr_t noncached_alloc(size_t size, size_t align)
> diff --git a/cmd/cache.c b/cmd/cache.c
> index 27dcec0931..86fbaf8dd6 100644
> --- a/cmd/cache.c
> +++ b/cmd/cache.c
> @@ -64,6 +64,9 @@ static int do_dcache(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  			break;
>  		case 1:
>  			dcache_enable();
> +#ifdef CONFIG_SYS_NONCACHED_MEMORY

See above, then you won't need this ifdeffery.

> +			noncached_set_region();
> +#endif
>  			break;
>  		case 2:
>  			flush_dcache_all();
>
Patrice CHOTARD April 27, 2020, 2:39 p.m. UTC | #2
Hi Marek

On 4/27/20 12:22 PM, Marek Vasut wrote:
> On 4/23/20 6:01 PM, Patrice Chotard wrote:
>
> [...]
>
>> diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
>> index 007d4ebc49..7f3cfb407c 100644
>> --- a/arch/arm/lib/cache.c
>> +++ b/arch/arm/lib/cache.c
>> @@ -73,6 +73,15 @@ static unsigned long noncached_start;
>>  static unsigned long noncached_end;
>>  static unsigned long noncached_next;
>>  
>> +void noncached_set_region(void)
>> +{
> Make this a __weak function and let architectures override it.

Ok



>
>> +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
>> +	mmu_set_region_dcache_behaviour(noncached_start,
>> +					noncached_end - noncached_start,
>> +					DCACHE_OFF);
>> +#endif
>> +}
>> +
>>  void noncached_init(void)
>>  {
>>  	phys_addr_t start, end;
>> @@ -89,9 +98,7 @@ void noncached_init(void)
>>  	noncached_end = end;
>>  	noncached_next = start;
>>  
>> -#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
>> -	mmu_set_region_dcache_behaviour(noncached_start, size, DCACHE_OFF);
>> -#endif
>> +	noncached_set_region();
>>  }
>>  
>>  phys_addr_t noncached_alloc(size_t size, size_t align)
>> diff --git a/cmd/cache.c b/cmd/cache.c
>> index 27dcec0931..86fbaf8dd6 100644
>> --- a/cmd/cache.c
>> +++ b/cmd/cache.c
>> @@ -64,6 +64,9 @@ static int do_dcache(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>  			break;
>>  		case 1:
>>  			dcache_enable();
>> +#ifdef CONFIG_SYS_NONCACHED_MEMORY
> See above, then you won't need this ifdeffery.

Ok

Thanks

>
>> +			noncached_set_region();
>> +#endif
>>  			break;
>>  		case 2:
>>  			flush_dcache_all();
>>
diff mbox series

Patch

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 81ccead112..06fb458301 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -584,6 +584,7 @@  void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
 
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
 void noncached_init(void);
+void noncached_set_region(void);
 phys_addr_t noncached_alloc(size_t size, size_t align);
 #endif /* CONFIG_SYS_NONCACHED_MEMORY */
 
diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
index 007d4ebc49..7f3cfb407c 100644
--- a/arch/arm/lib/cache.c
+++ b/arch/arm/lib/cache.c
@@ -73,6 +73,15 @@  static unsigned long noncached_start;
 static unsigned long noncached_end;
 static unsigned long noncached_next;
 
+void noncached_set_region(void)
+{
+#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
+	mmu_set_region_dcache_behaviour(noncached_start,
+					noncached_end - noncached_start,
+					DCACHE_OFF);
+#endif
+}
+
 void noncached_init(void)
 {
 	phys_addr_t start, end;
@@ -89,9 +98,7 @@  void noncached_init(void)
 	noncached_end = end;
 	noncached_next = start;
 
-#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
-	mmu_set_region_dcache_behaviour(noncached_start, size, DCACHE_OFF);
-#endif
+	noncached_set_region();
 }
 
 phys_addr_t noncached_alloc(size_t size, size_t align)
diff --git a/cmd/cache.c b/cmd/cache.c
index 27dcec0931..86fbaf8dd6 100644
--- a/cmd/cache.c
+++ b/cmd/cache.c
@@ -64,6 +64,9 @@  static int do_dcache(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 			break;
 		case 1:
 			dcache_enable();
+#ifdef CONFIG_SYS_NONCACHED_MEMORY
+			noncached_set_region();
+#endif
 			break;
 		case 2:
 			flush_dcache_all();