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

[Kever Yang]

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@xxxxxxxxxxxxxx>
> >
> > 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@xxxxxxxxxxxxxx>
> > Signed-off-by: Liang Chen <cl@xxxxxxxxxxxxxx>
> > Signed-off-by: Kever Yang <kever.yang@xxxxxxxxxxxxxx>
> > ---
> >
> > 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@xxxxxxxxxxxxxx>
> > + */
> > +
> > +#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