[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.schulz at 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+uboot at 0leil.net>
>>> Signed-off-by: Quentin Schulz <quentin.schulz at theobroma-systems.com>
>> Reviewed-by: Kever Yang <kever.yang at 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
>>>