Re: [PATCH 06/18] rockchip: pine64: pinebook: migrate to rockchip_early_misc_init_r

[Dragan Simic]

Hello Quentin and Kever,

Please see my comments below.

On 2024-02-01 05:02, Dragan Simic wrote:

On 2024-02-01 03:48, Kever Yang wrote:

On 2024/1/23 22:49, Quentin Schulz wrote:

From: Quentin Schulz <quentin.sch...@theobroma-systems.com>

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

The patch subject should include "pinebook-pro" instead of just
"pinebook".  Those are two quite different devices.

Please see my comments on your 08/18 patch in this series, for the
RockPro64, which also apply to the Pinebook Pro.

We might actually introduce some issues with this change.  I'll get
back later with a more detailed explanation, together with a proposed
fix, after I check it all in detail.

This applies to some other patches in this series as well.

Cc: Quentin Schulz <foss+ub...@0leil.net>
Signed-off-by: Quentin Schulz <quentin.sch...@theobroma-systems.com>

Reviewed-by: Kever Yang <kever.y...@rock-chips.com>

Thanks,
- Kever

---

board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c | 18 ++----------------

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

diff --git a/board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c b/board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c

index 4ad780767ea..2408a367305 100644
--- a/board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c
+++ b/board/pine64/pinebook-pro-rk3399/pinebook-pro-rk3399.c
@@ -11,7 +11,6 @@
  #include <asm/arch-rockchip/clock.h>
  #include <asm/arch-rockchip/grf_rk3399.h>
  #include <asm/arch-rockchip/hardware.h>
-#include <asm/arch-rockchip/misc.h>
  #include <linux/printk.h>
  #include <power/regulator.h>
  @@ -54,23 +53,10 @@ static void setup_iodomain(void)
        rk_setreg(&pmugrf->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 ret;
+       return 0;
  }
  #endif