| Message ID | 20230328021912.177301-5-ychuang570808@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Introduce Nuvoton ma35d1 SoC | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dt-meta-schema | fail | build log |
Quoting Jacky Huang (2023-03-27 19:19:04) > +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>; > + }; This is prescribing driver details to the system-management node. The reset-controller node should be removed, the #reset-cells moved up one level, and the "nuvoton,ma35d1-sys" should match a driver that registers an auxiliary device for reset functionality. Is anything besides child nodes like 'reset-controller' using the syscon@40460000?
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. > + }; > + }; > +... > + > 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. > +/* > + * 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
Dear Stephen, On 2023/3/29 上午 01:48, Stephen Boyd wrote: > Quoting Jacky Huang (2023-03-27 19:19:04) >> +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>; >> + }; > This is prescribing driver details to the system-management node. The > reset-controller node should be removed, the #reset-cells moved up one > level, and the "nuvoton,ma35d1-sys" should match a driver that registers > an auxiliary device for reset functionality. Is anything besides child > nodes like 'reset-controller' using the syscon@40460000? OK, I will modify it as: sys: system-management@40460000 { compatible = "nuvoton,ma35d1-sys", "syscon",; reg = <0x0 0x40460000 0x0 0x200>; #reset-cells = <1>; }; }; And my reset driver will use compatible "nuvoton,ma35d1-sys", instead of "nuvoton,ma35d1-reset". There' no other node like reset-controller. Best regards, Jack 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
On 29/03/2023 11:12, Jacky Huang wrote: >>> + }; >>> + }; >>> +... >>> + >>> 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 '+' ? The simplest is to use the same license as your binding, so (GPL-2.0-only OR BSD-2-Clause) . > >>> +/* >>> + * 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. That's not the usual way. IDs start from 0 or 1 and gets incremented by 1, without gaps. Remember that IDs cannot change and your register bits could have some changes (e.g. HW errata). Best regards, Krzysztof
Dear Krzysztof, On 2023/3/29 下午 05:27, Krzysztof Kozlowski wrote: > On 29/03/2023 11:12, Jacky Huang wrote: >>>> + }; >>>> + }; >>>> +... >>>> + >>>> 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 '+' ? > The simplest is to use the same license as your binding, so > (GPL-2.0-only OR BSD-2-Clause) . OK, I got it. Thank you very much. >>>> +/* >>>> + * 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. > That's not the usual way. IDs start from 0 or 1 and gets incremented by > 1, without gaps. Remember that IDs cannot change and your register bits > could have some changes (e.g. HW errata). > > > Best regards, > Krzysztof > OK, I will modify the ID as contiguous number, and create a lookup table to translate ID into bitmap. Best Regards, Jacky Huang
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>; + }; + }; +... + 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) */ +/* + * 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 +#define MA35D1_RESET_I2S0 61 +#define MA35D1_RESET_SC0 64 +#define MA35D1_RESET_SC1 65 +#define MA35D1_RESET_QSPI1 68 +#define MA35D1_RESET_SPI3 70 +#define MA35D1_RESET_EPWM0 80 +#define MA35D1_RESET_EPWM1 81 +#define MA35D1_RESET_QEI0 86 +#define MA35D1_RESET_QEI1 87 +#define MA35D1_RESET_ECAP0 90 +#define MA35D1_RESET_ECAP1 91 +#define MA35D1_RESET_CANFD2 92 +#define MA35D1_RESET_ADC0 95 +#define MA35D1_RESET_TMR4 96 +#define MA35D1_RESET_TMR5 97 +#define MA35D1_RESET_TMR6 98 +#define MA35D1_RESET_TMR7 99 +#define MA35D1_RESET_TMR8 100 +#define MA35D1_RESET_TMR9 101 +#define MA35D1_RESET_TMR10 102 +#define MA35D1_RESET_TMR11 103 +#define MA35D1_RESET_UART8 104 +#define MA35D1_RESET_UART9 105 +#define MA35D1_RESET_UART10 106 +#define MA35D1_RESET_UART11 107 +#define MA35D1_RESET_UART12 108 +#define MA35D1_RESET_UART13 109 +#define MA35D1_RESET_UART14 110 +#define MA35D1_RESET_UART15 111 +#define MA35D1_RESET_UART16 112 +#define MA35D1_RESET_I2S1 113 +#define MA35D1_RESET_I2C4 114 +#define MA35D1_RESET_I2C5 115 +#define MA35D1_RESET_EPWM2 116 +#define MA35D1_RESET_ECAP2 117 +#define MA35D1_RESET_QEI2 118 +#define MA35D1_RESET_CANFD3 119 +#define MA35D1_RESET_KPI 120 +#define MA35D1_RESET_GIC 124 +#define MA35D1_RESET_SSMCC 126 +#define MA35D1_RESET_SSPCC 127 +#define MA35D1_RESET_COUNT 128 + +#endif