Re: [PATCH v2 05/20] rockchip: pine64: pinephone-pro: migrate to rockchip_early_misc_init_r

2024-02-12 Thread Peter Robinson 
On Fri, 9 Feb 2024 at 09:50, Quentin Schulz  wrote:
>
> From: Quentin Schulz 
>
> Compared to the original misc_init_r from Rockchip mach code,
> setup_iodomain() is added and rockchip_setup_macaddr() is not called.
>
> It is assumed adding rockchip_setup_macaddr() back is fine.
> Let's use rockchip_early_misc_init_r instead of reimplementing the whole
> misc_init_r from Rockchip (the side effect being that
> rockchip_setup_macaddr() is back).

Same as for my comment on the Pinebook Pro, there's no wired ethernet
on these devices and the GMAC driver isn't enabled, I vaguely remember
we might have excluded rockchip_setup_macaddr because of an error but
that was some time ago.

Reviewed-by: Peter Robinson 

> Cc: Quentin Schulz 
> Reviewed-by: Kever Yang 
> Signed-off-by: Quentin Schulz 
> ---
>  .../pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c  | 17 
> ++---
>  1 file changed, 2 insertions(+), 15 deletions(-)
>
> diff --git a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c 
> b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
> index b6ccbb9c1c4..de75ee329d8 100644
> --- a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
> +++ b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
> @@ -12,7 +12,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>
>  #define GRF_IO_VSEL_BT565_GPIO2AB 1
> @@ -56,23 +55,11 @@ static void setup_iodomain(void)
> rk_setreg(>soc_con0, 1 << PMUGRF_CON0_VSEL_SHIFT);
>  }
>
> -int misc_init_r(void)
> +int rockchip_early_misc_init_r(void)
>  {
> -   const u32 cpuid_offset = 0x7;
> -   const u32 cpuid_length = 0x10;
> -   u8 cpuid[cpuid_length];
> -   int ret;
> -
> setup_iodomain();
>
> -   ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
> -   if (ret)
> -   return ret;
> -
> -   ret = rockchip_cpuid_set(cpuid, cpuid_length);
> -   if (ret)
> -   return ret;
> +   return 0;
>
> -   return ret;
>  }
>  #endif
>
> --
> 2.43.0
>

Re: [PATCH v2 05/20] rockchip: pine64: pinephone-pro: migrate to rockchip_early_misc_init_r

2024-02-09 Thread Dragan Simic 

On 2024-02-09 10:50, Quentin Schulz wrote:

From: Quentin Schulz 

Compared to the original misc_init_r from Rockchip mach code,
setup_iodomain() is added and rockchip_setup_macaddr() is not called.

It is assumed adding rockchip_setup_macaddr() back is fine.
Let's use rockchip_early_misc_init_r instead of reimplementing the 
whole

misc_init_r from Rockchip (the side effect being that
rockchip_setup_macaddr() is back).

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 


Looking good to me.  Thank you for including the changes suggested
in my earlier review.

Reviewed-by: Dragan Simic 


---
 .../pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c  | 17 
++---

 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
index b6ccbb9c1c4..de75ee329d8 100644
--- a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
+++ b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
@@ -12,7 +12,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 

 #define GRF_IO_VSEL_BT565_GPIO2AB 1
@@ -56,23 +55,11 @@ static void setup_iodomain(void)
rk_setreg(>soc_con0, 1 << PMUGRF_CON0_VSEL_SHIFT);
 }

-int misc_init_r(void)
+int rockchip_early_misc_init_r(void)
 {
-   const u32 cpuid_offset = 0x7;
-   const u32 cpuid_length = 0x10;
-   u8 cpuid[cpuid_length];
-   int ret;
-
setup_iodomain();

-   ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
-   if (ret)
-   return ret;
-
-   ret = rockchip_cpuid_set(cpuid, cpuid_length);
-   if (ret)
-   return ret;
+   return 0;

-   return ret;
 }
 #endif

[PATCH v2 05/20] rockchip: pine64: pinephone-pro: migrate to rockchip_early_misc_init_r

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

Compared to the original misc_init_r from Rockchip mach code,
setup_iodomain() is added and rockchip_setup_macaddr() is not called.

It is assumed adding rockchip_setup_macaddr() back is fine.
Let's use rockchip_early_misc_init_r instead of reimplementing the whole
misc_init_r from Rockchip (the side effect being that
rockchip_setup_macaddr() is back).

Cc: Quentin Schulz 
Reviewed-by: Kever Yang 
Signed-off-by: Quentin Schulz 
---
 .../pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c  | 17 ++---
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c 
b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
index b6ccbb9c1c4..de75ee329d8 100644
--- a/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
+++ b/board/pine64/pinephone-pro-rk3399/pinephone-pro-rk3399.c
@@ -12,7 +12,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 #define GRF_IO_VSEL_BT565_GPIO2AB 1
@@ -56,23 +55,11 @@ static void setup_iodomain(void)
rk_setreg(>soc_con0, 1 << PMUGRF_CON0_VSEL_SHIFT);
 }
 
-int misc_init_r(void)
+int rockchip_early_misc_init_r(void)
 {
-   const u32 cpuid_offset = 0x7;
-   const u32 cpuid_length = 0x10;
-   u8 cpuid[cpuid_length];
-   int ret;
-
setup_iodomain();
 
-   ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
-   if (ret)
-   return ret;
-
-   ret = rockchip_cpuid_set(cpuid, cpuid_length);
-   if (ret)
-   return ret;
+   return 0;
 
-   return ret;
 }
 #endif

-- 
2.43.0