linux-kernel - Re: [PATCH v3 1/2] dt-bindings: reset: eswin: Documentation for eic7700 SoC

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <81bd7bd3-46c8-45c0-8266-44316597a406@kernel.org>
Date: Thu, 19 Jun 2025 19:39:14 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: dongxuyang@...incomputing.com, p.zabel@...gutronix.de, robh@...nel.org,
 krzk+dt@...nel.org, conor+dt@...nel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Cc: ningyu@...incomputing.com, linmin@...incomputing.com,
 huangyifeng@...incomputing.com
Subject: Re: [PATCH v3 1/2] dt-bindings: reset: eswin: Documentation for
 eic7700 SoC

On 19/06/2025 10:00, dongxuyang@...incomputing.com wrote:
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#reset-cells':
> +    const: 2

What is the meaning of the cells?

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#reset-cells'
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/reset/eswin,eic7700-reset.h>
> +
> +    reset-controller@...28000 {
> +        compatible = "eswin,eic7700-reset";
> +        reg = <0x51828000 0x80000>;
> +        #reset-cells = <2>;
> +    };
> diff --git a/include/dt-bindings/reset/eswin,eic7700-reset.h b/include/dt-bindings/reset/eswin,eic7700-reset.h
> new file mode 100644
> index 000000000000..8c3aa3c87ea4
> --- /dev/null

> +++ b/include/dt-bindings/reset/eswin,eic7700-reset.h
> @@ -0,0 +1,460 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright 2024, Beijing ESWIN Computing Technology Co., Ltd.. All rights reserved.

2025

> + *
> + * Device Tree binding constants for EIC7700 reset controller.
> + *
> + * Authors:
> + *	Yifeng Huang <huangyifeng@...incomputing.com>
> + *	Xuyang Dong <dongxuyang@...incomputing.com>
> + */
> +
> +#ifndef __DT_ESWIN_EIC7700_RESET_H__
> +#define __DT_ESWIN_EIC7700_RESET_H__
> +
> +#define SNOC_RST_CTRL 0
> +#define GPU_RST_CTRL 1
> +#define DSP_RST_CTRL 2
> +#define D2D_RST_CTRL 3
> +#define DDR_RST_CTRL 4
> +#define TCU_RST_CTRL 5
> +#define NPU_RST_CTRL 6
> +#define HSPDMA_RST_CTRL 7
> +#define PCIE_RST_CTRL 8
> +#define I2C_RST_CTRL 9
> +#define FAN_RST_CTRL 10
> +#define PVT_RST_CTRL 11
> +#define MBOX_RST_CTRL 12
> +#define UART_RST_CTRL 13


...

> +
> +/*TIMER1*/
> +#define SW_TIMER1_RSTN_0 0
> +#define SW_TIMER1_RSTN_1 1
> +#define SW_TIMER1_RSTN_2 2
> +#define SW_TIMER1_RSTN_3 3
> +#define SW_TIMER1_RSTN_4 4
> +#define SW_TIMER1_RSTN_5 5
> +#define SW_TIMER1_RSTN_6 6
> +#define SW_TIMER1_RSTN_7 7
> +#define SW_TIMER1_PRSTN 8
> +
> +/*TIMER2*/
> +#define SW_TIMER2_RSTN_0 0
> +#define SW_TIMER2_RSTN_1 1
> +#define SW_TIMER2_RSTN_2 2
> +#define SW_TIMER2_RSTN_3 3
> +#define SW_TIMER2_RSTN_4 4
> +#define SW_TIMER2_RSTN_5 5
> +#define SW_TIMER2_RSTN_6 6
> +#define SW_TIMER2_RSTN_7 7
> +#define SW_TIMER2_PRSTN 8

That's unreadable - missing indent before the value. Ids equal to block
number are not useful anyway.

The problem is: you still do not use this as an ABI. No driver usage at
all and (comment which I repeat very often) no point to add it to the
bindings. Look at other bindings how this is supposed to look like.


Best regards,
Krzysztof