Hi Krzysztof,

Thanks for your comments.


On Mon, 23 May 2022 at 12:01, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
On 22/05/2022 17:50, Tomer Maimon wrote:
> Add binding document and device tree binding
> constants for Nuvoton BMC NPCM8XX reset controller.
>
> Signed-off-by: Tomer Maimon <tmaimon77@gmail.com>
> ---
>  .../bindings/reset/nuvoton,npcm-reset.txt     |  17 ++-
>  .../dt-bindings/reset/nuvoton,npcm8xx-reset.h | 124 ++++++++++++++++++
>  2 files changed, 139 insertions(+), 2 deletions(-)
>  create mode 100644 include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
>
> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> index cb1613092ee7..b7eb8615b68b 100644
> --- a/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> +++ b/Documentation/devicetree/bindings/reset/nuvoton,npcm-reset.txt
> @@ -1,14 +1,15 @@
>  Nuvoton NPCM Reset controller

>  Required properties:
> -- compatible : "nuvoton,npcm750-reset" for NPCM7XX BMC
> +- compatible : "nuvoton,npcm750-reset" for Poleg NPCM7XX BMC.
> +               "nuvoton,npcm845-reset" for Arbel NPCM8XX BMC.
>  - reg : specifies physical base address and size of the register.
>  - #reset-cells: must be set to 2
>  - syscon: a phandle to access GCR registers.

>  Optional property:
>  - nuvoton,sw-reset-number - Contains the software reset number to restart the SoC.
> -  NPCM7xx contain four software reset that represent numbers 1 to 4.
> +  NPCM7xx and NPCM8xx contain four software reset that represent numbers 1 to 4.

>    If 'nuvoton,sw-reset-number' is not specified software reset is disabled.

> @@ -32,3 +33,15 @@ example:
>          };

>  The index could be found in <dt-bindings/reset/nuvoton,npcm7xx-reset.h>.
> +
> +Specifying reset lines connected to IP NPCM8XX modules
> +======================================================

we prefer to use the same explanation as the NPCM7XX reset explanation in the reset binding document.
No need to document consumers. Just mention the header.

> +example:
> +
> +        spi0: spi@..... {
> +                ...
> +                resets = <&rstc NPCM8XX_RESET_IPSRST2 NPCM8XX_RESET_PSPI1>;
> +                ...
> +        };
> +
> +The index could be found in <dt-bindings/reset/nuvoton,npcm8xx-reset.h>.
> diff --git a/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> new file mode 100644
> index 000000000000..4b832a0fd1dd
> --- /dev/null
> +++ b/include/dt-bindings/reset/nuvoton,npcm8xx-reset.h
> @@ -0,0 +1,124 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license.
O.K. 

> +// Copyright (c) 2022 Nuvoton Technology corporation.
> +
> +#ifndef _DT_BINDINGS_NPCM8XX_RESET_H
> +#define _DT_BINDINGS_NPCM8XX_RESET_H
> +
> +#define NPCM8XX_RESET_IPSRST1                0x20
> +#define NPCM8XX_RESET_IPSRST2                0x24
> +#define NPCM8XX_RESET_IPSRST3                0x34
> +#define NPCM8XX_RESET_IPSRST4                0x74

What are these? All IDs should be incremental, decimal and start from 0.
Register offset, we use the same method in NPCM7xx. please refer
https://elixir.bootlin.com/linux/v5.18/source/include/dt-bindings/reset/nuvoton,npcm7xx-reset.h

and the driver asserts the reset according to the reset include definitions 

> +
> +/* Reset lines on IP1 reset module (NPCM8XX_RESET_IPSRST1) */
> +#define NPCM8XX_RESET_GDMA0          3

IDs start from 0 and do not have holes.
This represents the reset BIT in the reset register. 


> +#define NPCM8XX_RESET_UDC1           5
> +#define NPCM8XX_RESET_GMAC3          6
> +#define NPCM8XX_RESET_UART_2_3               7
> +#define NPCM8XX_RESET_UDC2           8
> +#define NPCM8XX_RESET_PECI           9
> +#define NPCM8XX_RESET_AES            10
> +#define NPCM8XX_RESET_UART_0_1               11
> +#define NPCM8XX_RESET_MC             12
> +#define NPCM8XX_RESET_SMB2           13
> +#define NPCM8XX_RESET_SMB3           14
> +#define NPCM8XX_RESET_SMB4           15
> +#define NPCM8XX_RESET_SMB5           16
> +#define NPCM8XX_RESET_PWM_M0         18
> +#define NPCM8XX_RESET_TIMER_0_4              19
> +#define NPCM8XX_RESET_TIMER_5_9              20
> +#define NPCM8XX_RESET_GMAC4          21
> +#define NPCM8XX_RESET_UDC4           22
> +#define NPCM8XX_RESET_UDC5           23
> +#define NPCM8XX_RESET_UDC6           24
> +#define NPCM8XX_RESET_UDC3           25
> +#define NPCM8XX_RESET_ADC            27
> +#define NPCM8XX_RESET_SMB6           28
> +#define NPCM8XX_RESET_SMB7           29
> +#define NPCM8XX_RESET_SMB0           30
> +#define NPCM8XX_RESET_SMB1           31
> +
> +/* Reset lines on IP2 reset module (NPCM8XX_RESET_IPSRST2) */
> +#define NPCM8XX_RESET_MFT0           0
> +#define NPCM8XX_RESET_MFT1           1
> +#define NPCM8XX_RESET_MFT2           2
> +#define NPCM8XX_RESET_MFT3           3
> +#define NPCM8XX_RESET_MFT4           4
> +#define NPCM8XX_RESET_MFT5           5
> +#define NPCM8XX_RESET_MFT6           6
> +#define NPCM8XX_RESET_MFT7           7
> +#define NPCM8XX_RESET_MMC            8
> +#define NPCM8XX_RESET_GFX_SYS                10
> +#define NPCM8XX_RESET_AHB_PCIBRG     11
> +#define NPCM8XX_RESET_VDMA           12
> +#define NPCM8XX_RESET_ECE            13
> +#define NPCM8XX_RESET_VCD            14
> +#define NPCM8XX_RESET_VIRUART1               16
> +#define NPCM8XX_RESET_VIRUART2               17
> +#define NPCM8XX_RESET_SIOX1          18
> +#define NPCM8XX_RESET_SIOX2          19
> +#define NPCM8XX_RESET_BT             20
> +#define NPCM8XX_RESET_3DES           21
> +#define NPCM8XX_RESET_PSPI2          23
> +#define NPCM8XX_RESET_GMAC2          25
> +#define NPCM8XX_RESET_USBH1          26
> +#define NPCM8XX_RESET_GMAC1          28
> +#define NPCM8XX_RESET_CP1            31
> +
> +/* Reset lines on IP3 reset module (NPCM8XX_RESET_IPSRST3) */
> +#define NPCM8XX_RESET_PWM_M1         0
> +#define NPCM8XX_RESET_SMB12          1
> +#define NPCM8XX_RESET_SPIX           2
> +#define NPCM8XX_RESET_SMB13          3
> +#define NPCM8XX_RESET_UDC0           4
> +#define NPCM8XX_RESET_UDC7           5
> +#define NPCM8XX_RESET_UDC8           6
> +#define NPCM8XX_RESET_UDC9           7
> +#define NPCM8XX_RESET_USBHUB         8
> +#define NPCM8XX_RESET_PCI_MAILBOX    9
> +#define NPCM8XX_RESET_GDMA1          10
> +#define NPCM8XX_RESET_GDMA2          11
> +#define NPCM8XX_RESET_SMB14          12
> +#define NPCM8XX_RESET_SHA            13
> +#define NPCM8XX_RESET_SEC_ECC                14
> +#define NPCM8XX_RESET_PCIE_RC                15
> +#define NPCM8XX_RESET_TIMER_10_14    16
> +#define NPCM8XX_RESET_RNG            17
> +#define NPCM8XX_RESET_SMB15          18
> +#define NPCM8XX_RESET_SMB8           19
> +#define NPCM8XX_RESET_SMB9           20
> +#define NPCM8XX_RESET_SMB10          21
> +#define NPCM8XX_RESET_SMB11          22
> +#define NPCM8XX_RESET_ESPI           23
> +#define NPCM8XX_RESET_USB_PHY_1              24
> +#define NPCM8XX_RESET_USB_PHY_2              25
> +


Best regards,
Krzysztof

Best regards,

Tomer