Re: [PATCH v2 13/20] rockchip: migrate hardware.h inclusion into appropriate files

2024-02-09 Thread Johan Jonker



On 2/9/24 10:50, Quentin Schulz wrote:
> From: Quentin Schulz 
> 
> hardware.h is only defining macros which are "wrappers" around writel().
> 
> writel() is however not available in hardware.h,  needs to be
> included. This means in order to use the wrappers in hardware.h, one
> also needs to include the  header.
> 
> However, this cannot be done currently because hardware.h is included in
> include/configs files, which are implicitly included by every code file
> by default, which makes the compilation of arch/arm/cpu/armv8/u-boot.lds
> fail because ALIGN (the ARM assembly directive) got redefined by some
> of the include files coming from .
> 
> Because nothing in the include/configs file actually use hardware.h,
> let's remove the inclusion of hardware.h from the include/configs files
> and explicitly add it wherever it is required.
> 
> This prepares for the next commit where  will be included in
> hardware.h.
> 
> Cc: Quentin Schulz 
> Reviewed-by: Kever Yang 
> Signed-off-by: Quentin Schulz 
> ---
>  arch/arm/mach-rockchip/rk3066/rk3066.c | 1 +
>  drivers/ram/rockchip/dmc-rk3368.c  | 1 +
>  drivers/ram/rockchip/sdram_rk3188.c| 1 +
>  drivers/ram/rockchip/sdram_rk3288.c| 1 +
>  include/configs/rk3036_common.h| 1 -
>  include/configs/rk3066_common.h| 1 -
>  include/configs/rk3188_common.h| 1 -
>  include/configs/rk322x_common.h| 1 -
>  include/configs/rk3288_common.h| 1 -
>  include/configs/rk3368_common.h| 1 -
>  include/configs/rv1108_common.h| 1 -
>  11 files changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/mach-rockchip/rk3066/rk3066.c 
> b/arch/arm/mach-rockchip/rk3066/rk3066.c
> index 78c7d894f90..6661b788295 100644
> --- a/arch/arm/mach-rockchip/rk3066/rk3066.c
> +++ b/arch/arm/mach-rockchip/rk3066/rk3066.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define GRF_BASE 0x20008000
>  
> diff --git a/drivers/ram/rockchip/dmc-rk3368.c 
> b/drivers/ram/rockchip/dmc-rk3368.c
> index f36be941a38..74d8aed571c 100644
> --- a/drivers/ram/rockchip/dmc-rk3368.c
> +++ b/drivers/ram/rockchip/dmc-rk3368.c
> @@ -20,6 +20,7 @@

>  #include 
>  #include 
>  #include 
> +#include 

>From codingstyle.rst:

You should follow this ordering in U-Boot. 
The common.h header (which is going away at some point) should always be first, 
followed by other headers in order, then headers with directories, then local 
files.
Within that order, sort your includes.

>  #include 
>  #include 
>  #include 
> diff --git a/drivers/ram/rockchip/sdram_rk3188.c 
> b/drivers/ram/rockchip/sdram_rk3188.c
> index ad9f936df55..16a68885f1f 100644
> --- a/drivers/ram/rockchip/sdram_rk3188.c
> +++ b/drivers/ram/rockchip/sdram_rk3188.c
> @@ -25,6 +25,7 @@

>  #include 
>  #include 
>  #include 
> +#include 

dito

>  #include 
>  #include 
>  
> diff --git a/drivers/ram/rockchip/sdram_rk3288.c 
> b/drivers/ram/rockchip/sdram_rk3288.c
> index c99118fd612..ec6bdcb2015 100644
> --- a/drivers/ram/rockchip/sdram_rk3288.c
> +++ b/drivers/ram/rockchip/sdram_rk3288.c
> @@ -25,6 +25,7 @@

>  #include 
>  #include 
>  #include 
> +#include 

dito

>  #include 
>  #include 
>  #include 
> diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h
> index c2abd14e114..0bf9e8b9a2e 100644
> --- a/include/configs/rk3036_common.h
> +++ b/include/configs/rk3036_common.h
> @@ -5,7 +5,6 @@
>  #ifndef __CONFIG_RK3036_COMMON_H
>  #define __CONFIG_RK3036_COMMON_H
>  
> -#include 
>  #include "rockchip-common.h"
>  
>  #define CFG_SYS_HZ_CLOCK 2400
> diff --git a/include/configs/rk3066_common.h b/include/configs/rk3066_common.h
> index d70c8f77d48..6a3b6900463 100644
> --- a/include/configs/rk3066_common.h
> +++ b/include/configs/rk3066_common.h
> @@ -6,7 +6,6 @@
>  #ifndef __CONFIG_RK3066_COMMON_H
>  #define __CONFIG_RK3066_COMMON_H
>  
> -#include 
>  #include "rockchip-common.h"
>  
>  #define CFG_IRAM_BASE0x1008
> diff --git a/include/configs/rk3188_common.h b/include/configs/rk3188_common.h
> index a8cee1e44d4..98f2c25f3cf 100644
> --- a/include/configs/rk3188_common.h
> +++ b/include/configs/rk3188_common.h
> @@ -6,7 +6,6 @@
>  #ifndef __CONFIG_RK3188_COMMON_H
>  #define __CONFIG_RK3188_COMMON_H
>  
> -#include 
>  #include "rockchip-common.h"
>  
>  #define CFG_IRAM_BASE0x1008
> diff --git a/include/configs/rk322x_common.h b/include/configs/rk322x_common.h
> index 15f77df3e17..bab4ca015f7 100644
> --- a/include/configs/rk322x_common.h
> +++ b/include/configs/rk322x_common.h
> @@ -5,7 +5,6 @@
>  #ifndef __CONFIG_RK322X_COMMON_H
>  #define __CONFIG_RK322X_COMMON_H
>  
> -#include 
>  #include "rockchip-common.h"
>  
>  #define CFG_SYS_HZ_CLOCK 2400
> diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
> index 3063076a97a..0c449e31099 100644
> --- a/include/configs/rk3288_common.h
> +++ b/include/configs/rk3288_common.h
> @@ 

Re: [PATCH v2 13/20] rockchip: migrate hardware.h inclusion into appropriate files

2024-02-09 Thread Quentin Schulz

Hi Johan,

On 2/9/24 14:07, Johan Jonker wrote:
[...]



 From codingstyle.rst:

You should follow this ordering in U-Boot.
The common.h header (which is going away at some point) should always be first,
followed by other headers in order, then headers with directories, then local 
files.
Within that order, sort your includes.



Good catch indeed. I'll avoid resending a 20-patch series today though.

I have the following diff that can be applied on top of that commit:

"""
diff --git a/drivers/ram/rockchip/dmc-rk3368.c 
b/drivers/ram/rockchip/dmc-rk3368.c

index 74d8aed571c..859fc47030f 100644
--- a/drivers/ram/rockchip/dmc-rk3368.c
+++ b/drivers/ram/rockchip/dmc-rk3368.c
@@ -17,10 +17,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/drivers/ram/rockchip/sdram_rk3188.c 
b/drivers/ram/rockchip/sdram_rk3188.c

index 16a68885f1f..d23d4231798 100644
--- a/drivers/ram/rockchip/sdram_rk3188.c
+++ b/drivers/ram/rockchip/sdram_rk3188.c
@@ -22,10 +22,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 

diff --git a/drivers/ram/rockchip/sdram_rk3288.c 
b/drivers/ram/rockchip/sdram_rk3288.c

index ec6bdcb2015..3177051dd12 100644
--- a/drivers/ram/rockchip/sdram_rk3288.c
+++ b/drivers/ram/rockchip/sdram_rk3288.c
@@ -22,10 +22,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
"""

You'll note that drivers/ram/rockchip/dmc-rk3368.c currently already 
doesn't follow the coding style :)


@Kever, up to you if you want me to send a v3 for that or if you can 
apply the diff above yourself before merging.


Cheers,
Quentin


[PATCH v2 13/20] rockchip: migrate hardware.h inclusion into appropriate files

2024-02-09 Thread Quentin Schulz
From: Quentin Schulz 

hardware.h is only defining macros which are "wrappers" around writel().

writel() is however not available in hardware.h,  needs to be
included. This means in order to use the wrappers in hardware.h, one
also needs to include the  header.

However, this cannot be done currently because hardware.h is included in
include/configs files, which are implicitly included by every code file
by default, which makes the compilation of arch/arm/cpu/armv8/u-boot.lds
fail because ALIGN (the ARM assembly directive) got redefined by some
of the include files coming from .

Because nothing in the include/configs file actually use hardware.h,
let's remove the inclusion of hardware.h from the include/configs files
and explicitly add it wherever it is required.

This prepares for the next commit where  will be included in
hardware.h.

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 arch/arm/mach-rockchip/rk3066/rk3066.c | 1 +
 drivers/ram/rockchip/dmc-rk3368.c  | 1 +
 drivers/ram/rockchip/sdram_rk3188.c| 1 +
 drivers/ram/rockchip/sdram_rk3288.c| 1 +
 include/configs/rk3036_common.h| 1 -
 include/configs/rk3066_common.h| 1 -
 include/configs/rk3188_common.h| 1 -
 include/configs/rk322x_common.h| 1 -
 include/configs/rk3288_common.h| 1 -
 include/configs/rk3368_common.h| 1 -
 include/configs/rv1108_common.h| 1 -
 11 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/arm/mach-rockchip/rk3066/rk3066.c 
b/arch/arm/mach-rockchip/rk3066/rk3066.c
index 78c7d894f90..6661b788295 100644
--- a/arch/arm/mach-rockchip/rk3066/rk3066.c
+++ b/arch/arm/mach-rockchip/rk3066/rk3066.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define GRF_BASE   0x20008000
 
diff --git a/drivers/ram/rockchip/dmc-rk3368.c 
b/drivers/ram/rockchip/dmc-rk3368.c
index f36be941a38..74d8aed571c 100644
--- a/drivers/ram/rockchip/dmc-rk3368.c
+++ b/drivers/ram/rockchip/dmc-rk3368.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/drivers/ram/rockchip/sdram_rk3188.c 
b/drivers/ram/rockchip/sdram_rk3188.c
index ad9f936df55..16a68885f1f 100644
--- a/drivers/ram/rockchip/sdram_rk3188.c
+++ b/drivers/ram/rockchip/sdram_rk3188.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
diff --git a/drivers/ram/rockchip/sdram_rk3288.c 
b/drivers/ram/rockchip/sdram_rk3288.c
index c99118fd612..ec6bdcb2015 100644
--- a/drivers/ram/rockchip/sdram_rk3288.c
+++ b/drivers/ram/rockchip/sdram_rk3288.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/include/configs/rk3036_common.h b/include/configs/rk3036_common.h
index c2abd14e114..0bf9e8b9a2e 100644
--- a/include/configs/rk3036_common.h
+++ b/include/configs/rk3036_common.h
@@ -5,7 +5,6 @@
 #ifndef __CONFIG_RK3036_COMMON_H
 #define __CONFIG_RK3036_COMMON_H
 
-#include 
 #include "rockchip-common.h"
 
 #define CFG_SYS_HZ_CLOCK   2400
diff --git a/include/configs/rk3066_common.h b/include/configs/rk3066_common.h
index d70c8f77d48..6a3b6900463 100644
--- a/include/configs/rk3066_common.h
+++ b/include/configs/rk3066_common.h
@@ -6,7 +6,6 @@
 #ifndef __CONFIG_RK3066_COMMON_H
 #define __CONFIG_RK3066_COMMON_H
 
-#include 
 #include "rockchip-common.h"
 
 #define CFG_IRAM_BASE  0x1008
diff --git a/include/configs/rk3188_common.h b/include/configs/rk3188_common.h
index a8cee1e44d4..98f2c25f3cf 100644
--- a/include/configs/rk3188_common.h
+++ b/include/configs/rk3188_common.h
@@ -6,7 +6,6 @@
 #ifndef __CONFIG_RK3188_COMMON_H
 #define __CONFIG_RK3188_COMMON_H
 
-#include 
 #include "rockchip-common.h"
 
 #define CFG_IRAM_BASE  0x1008
diff --git a/include/configs/rk322x_common.h b/include/configs/rk322x_common.h
index 15f77df3e17..bab4ca015f7 100644
--- a/include/configs/rk322x_common.h
+++ b/include/configs/rk322x_common.h
@@ -5,7 +5,6 @@
 #ifndef __CONFIG_RK322X_COMMON_H
 #define __CONFIG_RK322X_COMMON_H
 
-#include 
 #include "rockchip-common.h"
 
 #define CFG_SYS_HZ_CLOCK   2400
diff --git a/include/configs/rk3288_common.h b/include/configs/rk3288_common.h
index 3063076a97a..0c449e31099 100644
--- a/include/configs/rk3288_common.h
+++ b/include/configs/rk3288_common.h
@@ -6,7 +6,6 @@
 #ifndef __CONFIG_RK3288_COMMON_H
 #define __CONFIG_RK3288_COMMON_H
 
-#include 
 #include "rockchip-common.h"
 
 #define CFG_SYS_HZ_CLOCK   2400
diff --git a/include/configs/rk3368_common.h b/include/configs/rk3368_common.h
index ccb5369b901..d488f8d477a 100644
--- a/include/configs/rk3368_common.h
+++ b/include/configs/rk3368_common.h
@@ -8,7 +8,6 @@
 
 #include "rockchip-common.h"
 
-#include 
 #include 
 
 #define CFG_SYS_SDRAM_BASE 0
diff --git a/include/configs/rv1108_common.h b/include/configs/rv1108_common.h
index 3bf70a0e0ae..ff28236a21d 100644
---