Re: [PATCH 3/5] watchdog: Make RZV2HWDT driver depend on ARCH_R9A09G47

[Krzysztof Kozlowski <krzk@kernel.org>]

On 24/01/2025 13:55, Biju Das wrote:
> Hi Krzysztof Kozlowski,
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: 24 January 2025 12:42
>> Subject: Re: [PATCH 3/5] watchdog: Make RZV2HWDT driver depend on ARCH_R9A09G47
>>
>> On 24/01/2025 11:57, Biju Das wrote:
>>> Hi Krzysztof Kozlowski,
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Sent: 24 January 2025 10:35
>>>> Subject: Re: [PATCH 3/5] watchdog: Make RZV2HWDT driver depend on
>>>> ARCH_R9A09G47
>>>>
>>>> On 24/01/2025 11:20, Biju Das wrote:
>>>>>>
>>>>>>> +	depends on ARCH_R9A09G047 || ARCH_R9A09G057 || COMPILE_TEST
>>>>>>
>>>>>> But this is just wrong. You are supposed to depend on renesas ARHC,
>>>>>> not your individual SoC (and this is what you called here "ARCH_R9A...").
>>>>>>
>>>>>> Greg many times gave strong opinion that even full ARCH is wrong
>>>>>> and we managed to convince him that it has a meaning (or he did not
>>>>>> want to keep discussing). But restricting it per soc is pointless
>>>>>> and impossible to defend in
>>>> discussion.
>>>>>
>>>>> Currently for building RZ/G3E WDT, I need to always have RZ/V2H SoC config.
>>>>> which is pointless. May be ARCH_RENESAS should ok in this case??
>>>> Assuming ARCH_RENESAS covers your individual SoCs above, yes, that's
>>>> the way for driver to limit themselves to usable family.
>>>
>>> ARCH_RENESAS has ARM, ARM64 and RISC based SoCs.
>>>
>>> Currently it covers ARCH_RCAR_GEN1, ARCH_RCAR_GEN2,  ARCH_RCAR_GEN3,
>>> ARCH_RCAR_GEN4, ARCH_RMOBILE, ARCH_RZG2L, ARCH_RZN1 Family SOCs and
>>> rest of the individual SoCs such as RZ/V2H abnd RZ/g3E.
>>
>>
>> Rather tell me why this is supposed to be different than other vendors?
> 
> It is not different from other vendors. 
> 
> See, for eg:
> config S3C2410_WATCHDOG                                                          
>  557         tristate "S3C6410/S5Pv210/Exynos Watchdog"                               
>  558         depends on ARCH_S3C64XX || ARCH_S5PV210 || ARCH_EXYNOS || COMPILE_TEST  

You see - only one ARCH_EXYNOS.

That's the arch and vendor. Exynos is the entire arch for arm32 and
arm64 consisting of all of SoCs.

S3C and S5P are entirely different, much older archs - these even could
not be combined in one image with Exynos some time ago.

> 
> 
> 575 config SA1100_WATCHDOG                                                           
>  576         tristate "SA1100/PXA2xx watchdog"                                        
>  577         depends on ARCH_SA1100 || ARCH_PXA || COMPILE_TEST      
> 
> and many more.

Again: only one SA1100, one PXA. Not per each PXA SoC.

So these prove my point - use only your ARCH
> 
> 
>>
>> || ARM64 is already used solution
> 
> If you are correct, then all should depend on either on ARM or ARM64 or RISCV etc...
> 
> 
>>
>>>
>>> Since most of IP's in RZ/V2H and RZ/G3E are identical we could
>>> introduce a new family SoC ARCH_RZG3E_RZV2H to cover both or top level ARCH_RENESAS??
>>
>> You should not write drivers per SoCs (or even two or there SoCs) and there is really no need to
>> restrict them per each SoC.
> 
> If I am not wrong, The watchdog subsystem uses similar approach.
> 
>>
>> Otherwise come with arguments to my first question: why do you need exception here from generic kernel
>> approach?
> 
> It is not deviating from generic kernel approach as lot of vendors are doing this way.
> eg:
> 
> config OMAP_WATCHDOG                                                             
>           tristate "OMAP Watchdog"                                                 
>          depends on ARCH_OMAP16XX || ARCH_OMAP2PLUS || COMPILE_TEST    

Anyway, that's ancient OMAP, we speak about new devices.

> 
> 
>  config DAVINCI_WATCHDOG                                                          
>          tristate "DaVinci watchdog"                                              
>           depends on ARCH_DAVINCI || ARCH_KEYSTONE || COMPILE_TEST   

Different ARCH, not SoCs!

> 
> 
>  config K3_RTI_WATCHDOG                                                           
>          tristate "Texas Instruments K3 RTI watchdog"                             
>          depends on ARCH_K3 || COMPILE_TEST   

Dependency on ARCH.

Do you understand the difference between ARCH and SoC (ARCH_R9A09G47 is
the SoC - individual or family)?



Best regards,
Krzysztof