Re: [PATCH v6 04/12] dt-bindings: reset: nuvoton: add binding for ma35d1 IP reset control

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
+ *
+ * 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






[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux