Re: [PATCH v2 2/3] dt-bindings: clock, reset: rockchip: Add support for rk3562

[Kever Yang <kever.yang@rock-chips.com>]

Hi Krzysztof,

On 2024/12/27 16:28, Krzysztof Kozlowski wrote:
> On Tue, Dec 24, 2024 at 05:23:09PM +0800, Kever Yang wrote:
>> From: Finley Xiao <finley.xiao@rock-chips.com>
>>
>> Add the dt-bindings header for the rk3562, that gets shared between
>> the clock controller and the clock references in the dts.
>> Add softreset ID for rk3562.
>>
>> Signed-off-by: Finley Xiao <finley.xiao@rock-chips.com>
>> Signed-off-by: Liang Chen <cl@rock-chips.com>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>
>> Changes in v2:
>> - rename the file to rockchip,rk3562-cru.h
>> - remove CLK_NR_CLKS
>> - add new file for reset ID
>> - update to use dual license
>>
>>   .../dt-bindings/clock/rockchip,rk3562-cru.h   | 377 ++++++++++++++++++
>>   .../dt-bindings/reset/rockchip,rk3562-cru.h   | 360 +++++++++++++++++
>
> No, that's not a separate patch. Headers *ALWAYS* go with the bindings
> patch.
Will fix.
>>   2 files changed, 737 insertions(+)
>>   create mode 100644 include/dt-bindings/clock/rockchip,rk3562-cru.h
>>   create mode 100644 include/dt-bindings/reset/rockchip,rk3562-cru.h
>>
>> diff --git a/include/dt-bindings/clock/rockchip,rk3562-cru.h b/include/dt-bindings/clock/rockchip,rk3562-cru.h
>> new file mode 100644
>> index 000000000000..ad07ad3a12ad
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/rockchip,rk3562-cru.h
>> @@ -0,0 +1,377 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> Why not using license requested by checkpatch?

The checkpatch does not report error/warning for this license, and this 
is the same as many other SoCs.

Which license is recommend in the header file?

>
>> +/*
>> + * Copyright (c) 2022-2024 Rockchip Electronics Co., Ltd.
>> + * Author: Finley Xiao <finley.xiao@rock-chips.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3562_H
>> +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3562_H
>> +
>> +/* cru-clocks indices */
>> +
>> +/* cru plls */
>> +#define PLL_APLL			1
> Start with 0. Your other binding also starts with 0, so be consistent.
Will fix.
>
>> +#define PLL_GPLL			2
>> +#define PLL_VPLL			3
>> +#define PLL_HPLL			4
>> +#define PLL_CPLL			5
>> +#define PLL_DPLL			6
>> +
>> +/* cru clocks */
> Missing clock for 7. You are not supposed to have any holes here.

Will fix.

Thanks,

- Kever

>> +#define ARMCLK				8
>> +#define CLK_GPU				9
>> +#define ACLK_RKNN			10
> Best regards,
> Krzysztof
>
>