Messages in this thread Patch in this message | | | Date | Wed, 29 Mar 2023 17:12:52 +0800 | Subject | Re: [PATCH v6 04/12] dt-bindings: reset: nuvoton: add binding for ma35d1 IP reset control | From | Jacky Huang <> |
| |
Dear Krzysztof,
On 2023/3/29 下午 04:17, Krzysztof Kozlowski wrote: > On 28/03/2023 04:19, Jacky Huang wrote: >> From: Jacky Huang <ychuang3@nuvoton.com> > Subject: drop second/last, redundant "bindings". The "dt-bindings" > prefix is already stating that these are bindings. > > This is a friendly reminder during the review process. > > It seems my previous comments were not fully addressed. Maybe my > feedback got lost between the quotes, maybe you just forgot to apply it. > Please go back to the previous discussion and either implement all > requested changes or keep discussing them. > > Thank you. > >> Add the dt-bindings header for Nuvoton ma35d1, that gets shared >> between the reset controller and reset references in the dts. >> Add documentation to describe nuvoton ma35d1 reset driver. >> >> Signed-off-by: Jacky Huang <ychuang3@nuvoton.com> >> --- >> .../bindings/reset/nuvoton,ma35d1-reset.yaml | 44 +++++++ >> .../dt-bindings/reset/nuvoton,ma35d1-reset.h | 108 ++++++++++++++++++ >> 2 files changed, 152 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml >> create mode 100644 include/dt-bindings/reset/nuvoton,ma35d1-reset.h >> >> diff --git a/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml >> new file mode 100644 >> index 000000000000..342717257e5c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/reset/nuvoton,ma35d1-reset.yaml >> @@ -0,0 +1,44 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/reset/nuvoton,ma35d1-reset.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Nuvoton MA35D1 Reset Controller >> + >> +maintainers: >> + - Chi-Fang Li <cfli0@nuvoton.com> >> + - Jacky Huang <ychuang3@nuvoton.com> >> + >> +description: >> + The system reset controller can be used to reset various peripheral >> + controllers in MA35D1 SoC. >> + >> +properties: >> + compatible: >> + const: nuvoton,ma35d1-reset >> + >> + '#reset-cells': >> + const: 1 >> + >> +required: >> + - compatible >> + - '#reset-cells' >> + >> +additionalProperties: false >> + >> +examples: >> + # system reset controller node: >> + - | >> + >> + system-management@40460000 { >> + compatible = "nuvoton,ma35d1-sys", "syscon", "simple-mfd"; >> + reg = <0x40460000 0x200>; >> + >> + reset-controller { >> + compatible = "nuvoton,ma35d1-reset"; >> + #reset-cells = <1>; > You do not take any resources here, thus this should be rather part of > the parent node.
I will modify it as: sys: system-management@40460000 { compatible = "nuvoton,ma35d1-sys", "syscon",; reg = <0x0 0x40460000 0x0 0x200>; #reset-cells = <1>; }; };
>> + }; >> + }; >> +... >> + >> diff --git a/include/dt-bindings/reset/nuvoton,ma35d1-reset.h b/include/dt-bindings/reset/nuvoton,ma35d1-reset.h >> new file mode 100644 >> index 000000000000..f3088bc0afec >> --- /dev/null >> +++ b/include/dt-bindings/reset/nuvoton,ma35d1-reset.h >> @@ -0,0 +1,108 @@ >> +/* SPDX-License-Identifier: (GPL-2.0+ OR BSD-2-Clause) */ > Weird license, why 2.0+? > You already got here comment about license in previous version of this > patch. >
In fact, I always be confused with the license. It should be right if I just copy one from the same directory /include/dt-bindings/reset/. But, when I find there are several versions there, I was confused and just select one of them.
/* SPDX-License-Identifier: GPL-2.0-only */ /* SPDX-License-Identifier: GPL-2.0+ */ /* SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)*/ /* SPDX-License-Identifier: (GPL-2.0+ or MIT) */ /* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */ So, should I fix the license as
/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */ just remove the '+' ?
>> +/* >> + * Copyright (C) 2023 Nuvoton Technologies. >> + * Author: Chi-Fen Li <cfli0@nuvoton.com> >> + * >> + * Device Tree binding constants for MA35D1 reset controller. >> + */ >> + >> +#ifndef __DT_BINDINGS_RESET_MA35D1_H >> +#define __DT_BINDINGS_RESET_MA35D1_H >> + >> +#define MA35D1_RESET_CHIP 0 >> +#define MA35D1_RESET_CA35CR0 1 >> +#define MA35D1_RESET_CA35CR1 2 >> +#define MA35D1_RESET_CM4 3 >> +#define MA35D1_RESET_PDMA0 4 >> +#define MA35D1_RESET_PDMA1 5 >> +#define MA35D1_RESET_PDMA2 6 >> +#define MA35D1_RESET_PDMA3 7 >> +#define MA35D1_RESET_DISP 9 >> +#define MA35D1_RESET_VCAP0 10 >> +#define MA35D1_RESET_VCAP1 11 >> +#define MA35D1_RESET_GFX 12 >> +#define MA35D1_RESET_VDEC 13 >> +#define MA35D1_RESET_WHC0 14 >> +#define MA35D1_RESET_WHC1 15 >> +#define MA35D1_RESET_GMAC0 16 >> +#define MA35D1_RESET_GMAC1 17 >> +#define MA35D1_RESET_HWSEM 18 >> +#define MA35D1_RESET_EBI 19 >> +#define MA35D1_RESET_HSUSBH0 20 >> +#define MA35D1_RESET_HSUSBH1 21 >> +#define MA35D1_RESET_HSUSBD 22 >> +#define MA35D1_RESET_USBHL 23 >> +#define MA35D1_RESET_SDH0 24 >> +#define MA35D1_RESET_SDH1 25 >> +#define MA35D1_RESET_NAND 26 >> +#define MA35D1_RESET_GPIO 27 >> +#define MA35D1_RESET_MCTLP 28 >> +#define MA35D1_RESET_MCTLC 29 >> +#define MA35D1_RESET_DDRPUB 30 >> +#define MA35D1_RESET_TMR0 34 >> +#define MA35D1_RESET_TMR1 35 >> +#define MA35D1_RESET_TMR2 36 >> +#define MA35D1_RESET_TMR3 37 >> +#define MA35D1_RESET_I2C0 40 >> +#define MA35D1_RESET_I2C1 41 >> +#define MA35D1_RESET_I2C2 42 >> +#define MA35D1_RESET_I2C3 43 >> +#define MA35D1_RESET_QSPI0 44 >> +#define MA35D1_RESET_SPI0 45 >> +#define MA35D1_RESET_SPI1 46 >> +#define MA35D1_RESET_SPI2 47 >> +#define MA35D1_RESET_UART0 48 >> +#define MA35D1_RESET_UART1 49 >> +#define MA35D1_RESET_UART2 50 >> +#define MA35D1_RESET_UAER3 51 >> +#define MA35D1_RESET_UART4 52 >> +#define MA35D1_RESET_UART5 53 >> +#define MA35D1_RESET_UART6 54 >> +#define MA35D1_RESET_UART7 55 >> +#define MA35D1_RESET_CANFD0 56 >> +#define MA35D1_RESET_CANFD1 57 >> +#define MA35D1_RESET_EADC0 60 > Why do you have gaps here? These should be IDs, not register offsets. > > Best regards, > Krzysztof >
The reset controller registers are composed of four 32-bits register. And the ID will be translated into the corresponding bit of these register. Each ID is mapped to a unique bit position.
Best regards, Jacky Huang
| |