linux-kernel - Re: [RESEND PATCH v6 4/5] dt-bindings: clock: rockchip: Add RK3506 clock and reset unit

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <48108089-775a-428c-8dd1-a75344823ace@kwiboo.se>
Date: Fri, 7 Nov 2025 15:59:12 +0100
From: Jonas Karlman <jonas@...boo.se>
To: Elaine Zhang <zhangqing@...k-chips.com>
Cc: mturquette@...libre.com, sboyd@...nel.org, sugar.zhang@...k-chips.com,
 heiko@...ech.de, robh@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-clk@...r.kernel.org,
 linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
 huangtao@...k-chips.com, finley.xiao@...k-chips.com
Subject: Re: [RESEND PATCH v6 4/5] dt-bindings: clock: rockchip: Add RK3506
 clock and reset unit

Hi Elaine,

On 11/7/2025 2:37 AM, Elaine Zhang wrote:
> From: Finley Xiao <finley.xiao@...k-chips.com>
> 
> Add device tree bindings for clock and reset unit on RK3506 SoC.
> Add clock and reset IDs for RK3506 SoC.
> 
> Signed-off-by: Finley Xiao <finley.xiao@...k-chips.com>
> Signed-off-by: Elaine Zhang <zhangqing@...k-chips.com>
> ---
>  .../bindings/clock/rockchip,rk3506-cru.yaml   |  51 ++++
>  .../dt-bindings/clock/rockchip,rk3506-cru.h   | 285 ++++++++++++++++++
>  .../dt-bindings/reset/rockchip,rk3506-cru.h   | 211 +++++++++++++
>  3 files changed, 547 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3506-cru.yaml
>  create mode 100644 include/dt-bindings/clock/rockchip,rk3506-cru.h
>  create mode 100644 include/dt-bindings/reset/rockchip,rk3506-cru.h

[snip]

> diff --git a/include/dt-bindings/reset/rockchip,rk3506-cru.h b/include/dt-bindings/reset/rockchip,rk3506-cru.h
> new file mode 100644
> index 000000000000..f38cc066009b
> --- /dev/null
> +++ b/include/dt-bindings/reset/rockchip,rk3506-cru.h
> @@ -0,0 +1,211 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright (c) 2023-2025 Rockchip Electronics Co., Ltd.
> + * Author: Finley Xiao <finley.xiao@...k-chips.com>
> + */
> +
> +#ifndef _DT_BINDINGS_REST_ROCKCHIP_RK3506_H
> +#define _DT_BINDINGS_REST_ROCKCHIP_RK3506_H
> +
> +/* CRU-->SOFTRST_CON00 */
> +#define SRST_NCOREPORESET0_AC		0
> +#define SRST_NCOREPORESET1_AC		1
> +#define SRST_NCOREPORESET2_AC		2
> +#define SRST_NCORESET0_AC		3
> +#define SRST_NCORESET1_AC		4
> +#define SRST_NCORESET2_AC		5
> +#define SRST_NL2RESET_AC		6
> +#define SRST_ARESETN_CORE_BIU_AC	7
> +#define SRST_HRESETN_M0_AC		8
> +
> +/* CRU-->SOFTRST_CON02 */
> +#define SRST_NDBGRESET			9
> +#define SRST_PRESETN_CORE_BIU		10
> +#define SRST_RESETN_PMU			11
> +
> +/* CRU-->SOFTRST_CON03 */
> +#define SRST_PRESETN_DBG		12
> +#define SRST_POTRESETN_DBG		13
> +#define SRST_PRESETN_CORE_GRF		14
> +#define SRST_RESETN_CORE_EMA_DETECT	15
> +#define SRST_RESETN_REF_PVTPLL_CORE	16
> +#define SRST_PRESETN_GPIO1		17
> +#define SRST_DBRESETN_GPIO1		18
> +
> +/* CRU-->SOFTRST_CON04 */
> +#define SRST_ARESETN_CORE_PERI_BIU	19
> +#define SRST_ARESETN_DSMC		20
> +#define SRST_PRESETN_DSMC		21
> +#define SRST_RESETN_FLEXBUS		22
> +#define SRST_ARESETN_FLEXBUS		23
> +#define SRST_HRESETN_FLEXBUS		24
> +#define SRST_ARESETN_DSMC_SLV		25
> +#define SRST_HRESETN_DSMC_SLV		26
> +#define SRST_RESETN_DSMC_SLV		27
> +
> +/* CRU-->SOFTRST_CON05 */
> +#define SRST_ARESETN_BUS_BIU		28
> +#define SRST_HRESETN_BUS_BIU		29
> +#define SRST_PRESETN_BUS_BIU		30
> +#define SRST_ARESETN_SYSRAM		31
> +#define SRST_HRESETN_SYSRAM		32
> +#define SRST_ARESETN_DMAC0		33
> +#define SRST_ARESETN_DMAC1		34
> +#define SRST_HRESETN_M0			35
> +#define SRST_RESETN_M0_JTAG		36
> +#define SRST_HRESETN_CRYPTO		37

My original question and your reply from v5 follow below.

On 11/7/2025 2:24 AM, zhangqing wrote:
>> Is there a reason why this (and the RV1126B) reset names now include the
>> RESETN name in all reset constant?
>>
>> For RK3528 and prior mainline SoCs the RESETN part of the name has been
>> striped from the constant, suggest we also strip the RESETN part for
>> RK3506 and RV1126B for consistency with other RK SoCs.
>>
> The current practice is to separate the reset id from the clk id.
> 
> Follow with others RK socs(RK3528、RK3588、RK3576.....)

and

On 11/7/2025 2:44 AM, zhangqing wrote:
> The reset id and rst-rk3506.c were automatically generated from our trm 
> using tools, while some of the previous chips were filled in manually.
> 
> It is not recommended to manually modify the content generated by the 
> tool to avoid unnecessary errors.
> 
> It is not necessary to change SRST_HRESETN_CRYPTO to SRST_H_CRYPTO.

Why do we now need to have two part of the indices name to represent it
is a reset, i.e. the SRST_ prefix and the RESETN in middle of the name.

	resets = <&cru SRST_ARESETN_MAC1>;

The example above seem unnecessary verbose with 3x reset:
- resets as prop name
- SRST_ prefix (software? reset) in the indices name
- RESETN in the indices name

Maybe the SRST_ prefix could be dropped if we now have RESETN as part of
the indices name?

Your tool could be updated to strip the verbose naming. For RK3528 I
used a small script/tool to strip away the RESETN name and to generate
the macros used in rst driver.

Regards,
Jonas

[snip]