[PATCH v4 07/10] ARM: rzn1: basic support for Renesas RZ/N1 SoC

[Marek Vasut]

On 4/17/23 20:57, Ralph Siemsen wrote:
> On Mon, Apr 17, 2023 at 07:15:07PM +0200, Marek Vasut wrote:
>> On 3/8/23 21:26, Ralph Siemsen wrote:
>>
>> [...]
>>
>>> +++ b/arch/arm/Kconfig
>>> @@ -1031,6 +1031,21 @@ config ARCH_RMOBILE
>>>      imply SYS_THUMB_BUILD
>>>      imply ARCH_MISC_INIT if DISPLAY_CPUINFO
>>> +config ARCH_RZN1
>>> +    bool "Reneasa RZ/N1 SoC"
>>> +    select CLK
>>> +    select CLK_RENESAS
>>> +    select CLK_R9A06G032
>>
>> Does 'select CLK_R9A06G032' automatically activate the dependencies 
>> like 'CLK_RENESAS' too ?
> 
> Do you mean things like CLK_RCAR_CPG_LIB for example?
> 
> The new clock driver has no dependencies (except perhaps ARCH_RZN1). 
> Since this clock driver is essential to boot, I figured it was best to 
> select it in Kconfig, rather than require each board defconfig to 
> specify the same options.
> 
> If there is a better/preferred approach, I will implement it.

I mean, if you drop 'CLK' and 'CLK_RENESAS' from this list, do they 
still get selected because 'CLK_R8A06G032' is in this list ? If yes, 
then drop them from this list, else keep them.

>>> +    select DM
>>> +    select DM_ETH
>>> +    select DM_SERIAL
>>> +    select PINCTRL
>>> +    select PINCONF
>>> +    select REGMAP
>>> +    select SYSRESET
>>> +    select SYSRESET_SYSCON
>>> +    imply CMD_DM
>>> +
>>>  config ARCH_SNAPDRAGON
>>>      bool "Qualcomm Snapdragon SoCs"
>>>      select ARM64
>>> @@ -2207,6 +2222,8 @@ source "arch/arm/mach-owl/Kconfig"
>>>  source "arch/arm/mach-rmobile/Kconfig"
>>> +source "arch/arm/mach-rzn1/Kconfig"
>>
>> Should this be in mach-rmobile (which, maybe, should be renamed to 
>> mach-renesas) ?
> 
> I vaguely recall that I discussed this with someone (possibly you), 
> likely on IRC. And I think the conclusion was there was a significant 
> enough difference (possibly Cortex-A7 versus A9?).
> 
> I don't have a strong preference, if moving to mach-rmobile makes sense, 
> let me know and I can give it a go.

I think it does make sense to keep all the renesas stuff in mach-rmobile 
. We can rename that to mach-renesas in some subsequent patch.

>>> +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
>>> +void enable_caches(void)
>>> +{
>>
>> Why not enable icache with icache_enable() too ?
> 
> The I-cache is enabled quite early in arch/arm/cpu/armv7/start.S

Add a comment please.