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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 19/06/2025 10:00, dongxuyang@eswincomputing.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@51828000 {
> +        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@eswincomputing.com>
> + *	Xuyang Dong <dongxuyang@eswincomputing.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