linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC
@ 2024-08-25 13:10 Yixun Lan
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
                   ` (3 more replies)
  0 siblings, 4 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-25 13:10 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio, Yixun Lan

This series adds pinctrl support to SpacemiT's K1 SoC, the controller
uses a single register to describe all pin functions, including
bias pull up/down, drive strength, schmitter trigger, slew rate,
strong pull-up, mux mode. Later, we complete the pinctrl property of
uart device for the Bananapi-F3 board.

The pinctrl docs of K1 can be found here[1], and dts data of this series
are largely converted from vendor's code[2].

Note, we rewrite this series as an independent pinctrl driver for K1 SoC,
which mean it does not use pinctrl-single driver as the model anymore,
see the suggestion from Krzysztof at [3].

Link: https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned [1]
Link: https://gitee.com/bianbu-linux/linux-6.1/blob/bl-v1.0.y/arch/riscv/boot/dts/spacemit/k1-x_pinctrl.dtsi [2]
Link: https://lore.kernel.org/all/b7a01cba-9f68-4a6f-9795-b9103ee81d8b@kernel.org/ [3]
Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
Changes in v2:
- drop using pinctrl-single driver for K1
- rewite as independent pinctrl driver
- rebase to v6.11-rc5
- Link to v1: https://lore.kernel.org/r/20240719-02-k1-pinctrl-v1-0-239ac5b77dd6@gentoo.org

---
Yixun Lan (4):
      dt-binding: pinctrl: spacemit: add documents for K1 SoC
      pinctrl: spacemit: add support for SpacemiT K1 SoC
      riscv: dts: spacemit: add pinctrl support for K1 SoC
      riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3

 .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      |  134 +++
 arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts    |    3 +
 arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi       |   19 +
 arch/riscv/boot/dts/spacemit/k1.dtsi               |    5 +
 drivers/pinctrl/Kconfig                            |    1 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/spacemit/Kconfig                   |   17 +
 drivers/pinctrl/spacemit/Makefile                  |    3 +
 drivers/pinctrl/spacemit/pinctrl-k1.c              | 1012 ++++++++++++++++++++
 drivers/pinctrl/spacemit/pinctrl-k1.h              |   36 +
 include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  |  161 ++++
 11 files changed, 1392 insertions(+)
---
base-commit: 5be63fc19fcaa4c236b307420483578a56986a37
change-id: 20240708-02-k1-pinctrl-3a2b0ec13101
prerequisite-change-id: 20240626-k1-01-basic-dt-1aa31eeebcd2:v5
prerequisite-patch-id: 47dcf6861f7d434d25855b379e6d7ef4ce369c9c
prerequisite-patch-id: 77787fe82911923aff15ccf565e8fa451538c3a6
prerequisite-patch-id: b0bdb1742d96c5738f05262c3b0059102761390b
prerequisite-patch-id: 3927d39d8d77e35d5bfe53d9950da574ff8f2054
prerequisite-patch-id: a98039136a4796252a6029e474f03906f2541643
prerequisite-patch-id: c95f6dc0547a2a63a76e3cba0cf5c623b212b4e6
prerequisite-patch-id: 66e750e438ee959ddc2a6f0650814a2d8c989139
prerequisite-patch-id: 29a0fd8c36c1a4340f0d0b68a4c34d2b8abfb1ab
prerequisite-patch-id: 0bdfff661c33c380d1cf00a6c68688e05f88c0b3
prerequisite-patch-id: 99f15718e0bfbb7ed1a96dfa19f35841b004dae9

Best regards,
-- 
Yixun Lan <dlan@gentoo.org>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:10 [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC Yixun Lan
@ 2024-08-25 13:10 ` Yixun Lan
  2024-08-25 13:48   ` Krzysztof Kozlowski
                     ` (4 more replies)
  2024-08-25 13:10 ` [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT " Yixun Lan
                   ` (2 subsequent siblings)
  3 siblings, 5 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-25 13:10 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio, Yixun Lan

Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.

Two vendor specific properties are introduced here, As the pinctrl
has dedicated slew rate enable control - bit[7], so we have
spacemit,slew-rate-{enable,disable} for this. For the same reason,
creating spacemit,strong-pull-up for the strong pull up control.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
 include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
 2 files changed, 295 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
new file mode 100644
index 0000000000000..8adfc5ebbce37
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
@@ -0,0 +1,134 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SpacemiT K1 SoC Pin Controller
+
+maintainers:
+  - Yixun Lan <dlan@gentoo.org>
+
+properties:
+  compatible:
+    const: spacemit,k1-pinctrl
+
+  reg:
+    items:
+      - description: pinctrl io memory base
+
+patternProperties:
+  '-cfg$':
+    type: object
+    description: |
+      A pinctrl node should contain at least one subnode representing the
+      pinctrl groups available on the machine.
+
+    additionalProperties: false
+
+    patternProperties:
+      '-pins$':
+        type: object
+        description: |
+          Each subnode will list the pins it needs, and how they should
+          be configured, with regard to muxer configuration, bias, input
+          enable/disable, input schmitt trigger, slew-rate enable/disable,
+          slew-rate, drive strength, power source.
+        $ref: /schemas/pinctrl/pincfg-node.yaml
+
+        allOf:
+          - $ref: pincfg-node.yaml#
+          - $ref: pinmux-node.yaml#
+
+        properties:
+          pinmux:
+            description: |
+              The list of GPIOs and their mux settings that properties in the
+              node apply to. This should be set using the K1_PADCONF macro to
+              construct the value.
+            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
+
+          bias-disable: true
+
+          bias-pull-up: true
+
+          bias-pull-down: true
+
+          drive-strength-microamp:
+            description: |
+              typical current when output high level, but in mA.
+              1.8V output: 11, 21, 32, 42 (mA)
+              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          input-schmitt:
+            description: |
+              typical threshold for schmitt trigger.
+              0: buffer mode
+              1: trigger mode
+              2, 3: trigger mode
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1, 2, 3]
+
+          power-source:
+            description: external power supplies at 1.8v or 3.3v.
+            enum: [ 1800, 3300 ]
+
+          slew-rate:
+            description: |
+              slew rate for output buffer
+              0, 1: Slow speed
+              2: Medium speed
+              3: Fast speed
+            $ref: /schemas/types.yaml#/definitions/uint32
+            enum: [0, 1, 2, 3]
+
+          spacemit,slew-rate-enable:
+            description: enable slew rate.
+            type: boolean
+
+          spacemit,slew-rate-disable:
+            description: disable slew rate.
+            type: boolean
+
+          spacemit,strong-pull-up:
+            description: enable strong pull up.
+            type: boolean
+
+        required:
+          - pinmux
+
+        additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        pinctrl@d401e000 {
+            compatible = "spacemit,k1-pinctrl";
+            reg = <0x0 0xd401e000 0x0 0x400>;
+            #pinctrl-cells = <2>;
+            #gpio-range-cells = <3>;
+
+            uart0_2_cfg: uart0-2-cfg {
+                uart0-2-pins {
+                    pinmux = <K1_PADCONF(GPIO_68, 2)>,
+                             <K1_PADCONF(GPIO_69, 2)>;
+                    bias-pull-up;
+                    drive-strength-microamp = <32>;
+                };
+            };
+        };
+    };
+
+...
diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
new file mode 100644
index 0000000000000..13ef4aa6c53a3
--- /dev/null
+++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
@@ -0,0 +1,161 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
+ * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
+ *
+ */
+
+#ifndef _DT_BINDINGS_PINCTRL_K1_H
+#define _DT_BINDINGS_PINCTRL_K1_H
+
+#define PINMUX(pin, mux) \
+	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
+
+/* pin offset */
+#define PINID(x)	((x) + 1)
+
+#define GPIO_INVAL  0
+#define GPIO_00     PINID(0)
+#define GPIO_01     PINID(1)
+#define GPIO_02     PINID(2)
+#define GPIO_03     PINID(3)
+#define GPIO_04     PINID(4)
+#define GPIO_05     PINID(5)
+#define GPIO_06     PINID(6)
+#define GPIO_07     PINID(7)
+#define GPIO_08     PINID(8)
+#define GPIO_09     PINID(9)
+#define GPIO_10     PINID(10)
+#define GPIO_11     PINID(11)
+#define GPIO_12     PINID(12)
+#define GPIO_13     PINID(13)
+#define GPIO_14     PINID(14)
+#define GPIO_15     PINID(15)
+#define GPIO_16     PINID(16)
+#define GPIO_17     PINID(17)
+#define GPIO_18     PINID(18)
+#define GPIO_19     PINID(19)
+#define GPIO_20     PINID(20)
+#define GPIO_21     PINID(21)
+#define GPIO_22     PINID(22)
+#define GPIO_23     PINID(23)
+#define GPIO_24     PINID(24)
+#define GPIO_25     PINID(25)
+#define GPIO_26     PINID(26)
+#define GPIO_27     PINID(27)
+#define GPIO_28     PINID(28)
+#define GPIO_29     PINID(29)
+#define GPIO_30     PINID(30)
+#define GPIO_31     PINID(31)
+
+#define GPIO_32     PINID(32)
+#define GPIO_33     PINID(33)
+#define GPIO_34     PINID(34)
+#define GPIO_35     PINID(35)
+#define GPIO_36     PINID(36)
+#define GPIO_37     PINID(37)
+#define GPIO_38     PINID(38)
+#define GPIO_39     PINID(39)
+#define GPIO_40     PINID(40)
+#define GPIO_41     PINID(41)
+#define GPIO_42     PINID(42)
+#define GPIO_43     PINID(43)
+#define GPIO_44     PINID(44)
+#define GPIO_45     PINID(45)
+#define GPIO_46     PINID(46)
+#define GPIO_47     PINID(47)
+#define GPIO_48     PINID(48)
+#define GPIO_49     PINID(49)
+#define GPIO_50     PINID(50)
+#define GPIO_51     PINID(51)
+#define GPIO_52     PINID(52)
+#define GPIO_53     PINID(53)
+#define GPIO_54     PINID(54)
+#define GPIO_55     PINID(55)
+#define GPIO_56     PINID(56)
+#define GPIO_57     PINID(57)
+#define GPIO_58     PINID(58)
+#define GPIO_59     PINID(59)
+#define GPIO_60     PINID(60)
+#define GPIO_61     PINID(61)
+#define GPIO_62     PINID(62)
+#define GPIO_63     PINID(63)
+
+#define GPIO_64     PINID(64)
+#define GPIO_65     PINID(65)
+#define GPIO_66     PINID(66)
+#define GPIO_67     PINID(67)
+#define GPIO_68     PINID(68)
+#define GPIO_69     PINID(69)
+#define GPIO_70     PINID(70)
+#define GPIO_71     PINID(71)
+#define GPIO_72     PINID(72)
+#define GPIO_73     PINID(73)
+#define GPIO_74     PINID(74)
+#define GPIO_75     PINID(75)
+#define GPIO_76     PINID(76)
+#define GPIO_77     PINID(77)
+#define GPIO_78     PINID(78)
+#define GPIO_79     PINID(79)
+#define GPIO_80     PINID(80)
+#define GPIO_81     PINID(81)
+#define GPIO_82     PINID(82)
+#define GPIO_83     PINID(83)
+#define GPIO_84     PINID(84)
+#define GPIO_85     PINID(85)
+
+#define GPIO_101    PINID(89)
+#define GPIO_100    PINID(90)
+#define GPIO_99     PINID(91)
+#define GPIO_98     PINID(92)
+#define GPIO_103    PINID(93)
+#define GPIO_102    PINID(94)
+
+#define GPIO_104    PINID(109)
+#define GPIO_105    PINID(110)
+#define GPIO_106    PINID(111)
+#define GPIO_107    PINID(112)
+#define GPIO_108    PINID(113)
+#define GPIO_109    PINID(114)
+#define GPIO_110    PINID(115)
+
+#define GPIO_93     PINID(116)
+#define GPIO_94     PINID(117)
+#define GPIO_95     PINID(118)
+#define GPIO_96     PINID(119)
+#define GPIO_97     PINID(120)
+
+#define GPIO_86     PINID(122)
+#define GPIO_87     PINID(123)
+#define GPIO_88     PINID(124)
+#define GPIO_89     PINID(125)
+#define GPIO_90     PINID(126)
+#define GPIO_91     PINID(127)
+#define GPIO_92     PINID(128)
+
+#define GPIO_111    PINID(130)
+#define GPIO_112    PINID(131)
+#define GPIO_113    PINID(132)
+#define GPIO_114    PINID(133)
+#define GPIO_115    PINID(134)
+#define GPIO_116    PINID(135)
+#define GPIO_117    PINID(136)
+#define GPIO_118    PINID(137)
+#define GPIO_119    PINID(138)
+#define GPIO_120    PINID(139)
+#define GPIO_121    PINID(140)
+#define GPIO_122    PINID(141)
+#define GPIO_123    PINID(142)
+#define GPIO_124    PINID(143)
+#define GPIO_125    PINID(144)
+#define GPIO_126    PINID(145)
+#define GPIO_127    PINID(146)
+
+#define SLEW_RATE_SLOW0		0
+#define SLEW_RATE_SLOW1		1
+#define SLEW_RATE_MEDIUM	2
+#define SLEW_RATE_FAST		3
+
+#define K1_PADCONF(pin, func) (((pin) << 16) | (func))
+
+#endif /* _DT_BINDINGS_PINCTRL_K1_H */

-- 
2.45.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
  2024-08-25 13:10 [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC Yixun Lan
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
@ 2024-08-25 13:10 ` Yixun Lan
  2024-08-26  8:16   ` Inochi Amaoto
  2024-08-25 13:10 ` [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for " Yixun Lan
  2024-08-25 13:10 ` [PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 Yixun Lan
  3 siblings, 1 reply; 19+ messages in thread
From: Yixun Lan @ 2024-08-25 13:10 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio, Yixun Lan

SpacemiT's K1 SoC has a pinctrl controller which use single register
to describe all functions, which include bias pull up/down, drive
strength, schmitter trigger, slew rate, strong pull up, mux mode.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 drivers/pinctrl/Kconfig               |    1 +
 drivers/pinctrl/Makefile              |    1 +
 drivers/pinctrl/spacemit/Kconfig      |   17 +
 drivers/pinctrl/spacemit/Makefile     |    3 +
 drivers/pinctrl/spacemit/pinctrl-k1.c | 1012 +++++++++++++++++++++++++++++++++
 drivers/pinctrl/spacemit/pinctrl-k1.h |   36 ++
 6 files changed, 1070 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 7e4f93a3bc7ac..8358da47cd00d 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -583,6 +583,7 @@ source "drivers/pinctrl/qcom/Kconfig"
 source "drivers/pinctrl/realtek/Kconfig"
 source "drivers/pinctrl/renesas/Kconfig"
 source "drivers/pinctrl/samsung/Kconfig"
+source "drivers/pinctrl/spacemit/Kconfig"
 source "drivers/pinctrl/spear/Kconfig"
 source "drivers/pinctrl/sprd/Kconfig"
 source "drivers/pinctrl/starfive/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index cc809669405ab..553beead7ffa0 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -73,6 +73,7 @@ obj-y				+= qcom/
 obj-$(CONFIG_ARCH_REALTEK)      += realtek/
 obj-$(CONFIG_PINCTRL_RENESAS)	+= renesas/
 obj-$(CONFIG_PINCTRL_SAMSUNG)	+= samsung/
+obj-y				+= spacemit/
 obj-$(CONFIG_PINCTRL_SPEAR)	+= spear/
 obj-y				+= sprd/
 obj-$(CONFIG_SOC_STARFIVE)	+= starfive/
diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
new file mode 100644
index 0000000000000..168f8a5ffbb95
--- /dev/null
+++ b/drivers/pinctrl/spacemit/Kconfig
@@ -0,0 +1,17 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Sophgo SoC PINCTRL drivers
+#
+
+config PINCTRL_SPACEMIT_K1
+	tristate "SpacemiT K1 SoC Pinctrl driver"
+	depends on ARCH_SPACEMIT || COMPILE_TEST
+	depends on OF
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	help
+	  Say Y to select the pinctrl driver for K1 SoC.
+	  This pin controller allows selecting the mux function for
+	  each pin. This driver can also be built as a module called
+	  pinctrl-k1.
diff --git a/drivers/pinctrl/spacemit/Makefile b/drivers/pinctrl/spacemit/Makefile
new file mode 100644
index 0000000000000..fede1e80fe0ff
--- /dev/null
+++ b/drivers/pinctrl/spacemit/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-$(CONFIG_PINCTRL_SPACEMIT_K1)	+= pinctrl-k1.o
diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
new file mode 100644
index 0000000000000..9ed9530c78150
--- /dev/null
+++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
@@ -0,0 +1,1012 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
+
+#include <linux/bitfield.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/spinlock.h>
+#include <linux/module.h>
+
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+
+#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
+
+#include "../core.h"
+#include "../pinctrl-utils.h"
+#include "../pinconf.h"
+#include "../pinmux.h"
+#include "pinctrl-k1.h"
+
+/*
+ * +---------+----------+-----------+--------+--------+----------+--------+
+ * |   pull  |   drive  | schmitter |  slew  |  edge  |  strong  |   mux  |
+ * | up/down | strength |  trigger  |  rate  | detect |   pull   |  mode  |
+ * +---------+----------+-----------+--------+--------+----------+--------+
+ *   3 bits     3 bits     2 bits     1 bit    3 bits     1 bit    3 bits
+ */
+
+#define PAD_MUX			GENMASK(2, 0)
+#define PAD_STRONG_PULL		BIT(3)
+#define PAD_EDGE_RISE		BIT(4)
+#define PAD_EDGE_FALL		BIT(5)
+#define PAD_EDGE_CLEAR		BIT(6)
+#define PAD_SLEW_RATE		GENMASK(12, 11)
+#define PAD_SLEW_RATE_EN	BIT(7)
+#define PAD_SCHMITT		GENMASK(9, 8)
+#define PAD_DRIVE		GENMASK(12, 10)
+#define PAD_PULLDOWN		BIT(13)
+#define PAD_PULLUP		BIT(14)
+#define PAD_PULL_EN		BIT(15)
+
+#define spacemit_pin_to_reg(pctrl, pin)		((pctrl)->regs + (pin << 2))
+
+struct spacemit_pin {
+	u16				pin;
+	u16				flags;
+};
+
+struct spacemit_pinctrl {
+	struct device				*dev;
+	struct pinctrl_dev			*pctl_dev;
+	const struct spacemit_pinctrl_data	*data;
+	struct pinctrl_desc			pdesc;
+
+	struct list_head			gpiofuncs;
+
+	struct mutex				mutex;
+	raw_spinlock_t				lock;
+
+	void __iomem				*regs;
+};
+
+struct spacemit_pinctrl_data {
+	const struct pinctrl_pin_desc   *pins;
+	const struct spacemit_pin	*data;
+	u16				npins;
+};
+
+struct spacemit_gpiofunc_range {
+	u32		offset;
+	u32		npins;
+	u32		gpiofunc;
+	struct		list_head node;
+};
+
+struct spacemit_pin_mux_config {
+	const struct spacemit_pin	*pin;
+	u32		config;
+};
+
+struct spacemit_pin_drv_strength {
+	u8		val;
+	u32		mA;
+};
+
+static unsigned int spacemit_dt_get_pin(u32 value)
+{
+	return (value >> 16) & GENMASK(15, 0);
+}
+
+static unsigned int spacemit_dt_get_pin_mux(u32 value)
+{
+	return value & GENMASK(15, 0);
+}
+
+static const struct spacemit_pin *spacemit_get_pin(struct spacemit_pinctrl *pctrl,
+						   unsigned long pin)
+{
+	const struct spacemit_pin *pdata = pctrl->data->data;
+	int i;
+
+	for (i = 0; i < pctrl->data->npins; i++) {
+		if (pin == pdata[i].pin)
+			return &pdata[i];
+	}
+
+	return NULL;
+}
+
+static inline enum spacemit_pin_io_type spacemit_to_pin_io_type(
+	const struct spacemit_pin *pin)
+{
+	return K1_PIN_GET_IO_TYPE(pin->flags);
+}
+
+/* External: IO voltage via external source, can be 1.8V or 3.3V */
+static const char * const io_type_desc[] = {
+	"None",
+	"Fixed/1V8",
+	"Fixed/3V3",
+	"External",
+};
+
+static void spacemit_pctrl_dbg_show(struct pinctrl_dev *pctldev,
+				    struct seq_file *seq, unsigned int pin)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
+	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
+	void __iomem *reg;
+	u32 value;
+
+	seq_printf(seq, "pin: %u ", spin->pin);
+
+	seq_printf(seq, "type: %s ", io_type_desc[type]);
+
+	reg = spacemit_pin_to_reg(pctrl, spin->pin);
+	value = readl(reg);
+	seq_printf(seq, "mux: 0x%08x ", value);
+}
+
+/* use IO high level output current as the table */
+static struct spacemit_pin_drv_strength spacemit_ds_1v8_tbl[4] = {
+	{ (0 << 1), 11 },
+	{ (1 << 1), 21 },
+	{ (2 << 1), 32 },
+	{ (3 << 1), 42 },
+};
+
+static struct spacemit_pin_drv_strength spacemit_ds_3v3_tbl[8] = {
+	{ 0,  7 },
+	{ 2, 10 },
+	{ 4, 13 },
+	{ 6, 16 },
+	{ 1, 19 },
+	{ 3, 23 },
+	{ 5, 26 },
+	{ 7, 29 },
+};
+
+static inline u8 spacemit_get_ds_value(struct spacemit_pin_drv_strength *tbl,
+				       u32 num, u32 mA)
+{
+	int i;
+
+	for (i = 0; i < num; i++)
+		if (mA <= tbl[i].mA)
+			return tbl[i].val;
+
+	return tbl[num - 1].val;
+}
+
+static inline u8 spacemit_get_driver_strength(enum spacemit_pin_io_type type,
+					      u32 mA)
+{
+	switch (type) {
+	case IO_TYPE_1V8:
+		return spacemit_get_ds_value(spacemit_ds_1v8_tbl,
+					     ARRAY_SIZE(spacemit_ds_1v8_tbl),
+					     mA);
+	case IO_TYPE_3V3:
+		return spacemit_get_ds_value(spacemit_ds_3v3_tbl,
+					     ARRAY_SIZE(spacemit_ds_3v3_tbl),
+					     mA);
+	default:
+		return 0;
+	}
+}
+
+static int spacemit_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
+					 struct device_node *np,
+					 struct pinctrl_map **maps,
+					 unsigned int *num_maps)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct device *dev = pctrl->dev;
+	struct device_node *child;
+	struct pinctrl_map *map;
+	const char **grpnames;
+	const char *grpname;
+	int ngroups = 0;
+	int nmaps = 0;
+	int ret;
+
+	for_each_available_child_of_node(np, child)
+		ngroups += 1;
+
+	grpnames = devm_kcalloc(dev, ngroups, sizeof(*grpnames), GFP_KERNEL);
+	if (!grpnames)
+		return -ENOMEM;
+
+	map = devm_kcalloc(dev, ngroups * 2, sizeof(*map), GFP_KERNEL);
+	if (!map)
+		return -ENOMEM;
+
+	ngroups = 0;
+	mutex_lock(&pctrl->mutex);
+	for_each_available_child_of_node(np, child) {
+		int npins = of_property_count_u32_elems(child, "pinmux");
+		unsigned int *pins;
+		struct spacemit_pin_mux_config *pinmuxs;
+		u32 config;
+		int i;
+
+		if (npins < 1) {
+			dev_err(dev, "invalid pinctrl group %pOFn.%pOFn\n",
+				np, child);
+			ret = -EINVAL;
+			goto dt_failed;
+		}
+
+		grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn",
+					 np, child);
+		if (!grpname) {
+			ret = -ENOMEM;
+			goto dt_failed;
+		}
+
+		grpnames[ngroups++] = grpname;
+
+		pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
+		if (!pins) {
+			ret = -ENOMEM;
+			goto dt_failed;
+		}
+
+		pinmuxs = devm_kcalloc(dev, npins, sizeof(*pinmuxs), GFP_KERNEL);
+		if (!pinmuxs) {
+			ret = -ENOMEM;
+			goto dt_failed;
+		}
+
+		for (i = 0; i < npins; i++) {
+			ret = of_property_read_u32_index(child, "pinmux",
+							 i, &config);
+
+			if (ret)
+				goto dt_failed;
+
+			pins[i] = spacemit_dt_get_pin(config);
+			pinmuxs[i].config = config;
+			pinmuxs[i].pin = spacemit_get_pin(pctrl, pins[i]);
+
+			if (!pinmuxs[i].pin) {
+				dev_err(dev, "failed to get pin %d\n", pins[i]);
+				ret = -ENODEV;
+				goto dt_failed;
+			}
+		}
+
+		map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
+		map[nmaps].data.mux.function = np->name;
+		map[nmaps].data.mux.group = grpname;
+		nmaps += 1;
+
+		ret = pinctrl_generic_add_group(pctldev, grpname,
+						pins, npins, pinmuxs);
+		if (ret < 0) {
+			dev_err(dev, "failed to add group %s: %d\n", grpname, ret);
+			goto dt_failed;
+		}
+
+		ret = pinconf_generic_parse_dt_config(child, pctldev,
+						      &map[nmaps].data.configs.configs,
+						      &map[nmaps].data.configs.num_configs);
+		if (ret) {
+			dev_err(dev, "failed to parse pin config of group %s: %d\n",
+				grpname, ret);
+			goto dt_failed;
+		}
+
+		if (map[nmaps].data.configs.num_configs == 0)
+			continue;
+
+		map[nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
+		map[nmaps].data.configs.group_or_pin = grpname;
+		nmaps += 1;
+	}
+
+	ret = pinmux_generic_add_function(pctldev, np->name,
+					  grpnames, ngroups, NULL);
+	if (ret < 0) {
+		dev_err(dev, "error adding function %s: %d\n", np->name, ret);
+		goto function_failed;
+	}
+
+	*maps = map;
+	*num_maps = nmaps;
+	mutex_unlock(&pctrl->mutex);
+
+	return 0;
+
+dt_failed:
+	of_node_put(child);
+function_failed:
+	pinctrl_utils_free_map(pctldev, map, nmaps);
+	mutex_unlock(&pctrl->mutex);
+	return ret;
+}
+
+static const struct pinctrl_ops spacemit_pctrl_ops = {
+	.get_groups_count	= pinctrl_generic_get_group_count,
+	.get_group_name		= pinctrl_generic_get_group_name,
+	.get_group_pins		= pinctrl_generic_get_group_pins,
+	.pin_dbg_show		= spacemit_pctrl_dbg_show,
+	.dt_node_to_map		= spacemit_pctrl_dt_node_to_map,
+	.dt_free_map		= pinctrl_utils_free_map,
+};
+
+static int spacemit_pmx_set_mux(struct pinctrl_dev *pctldev,
+			      unsigned int fsel, unsigned int gsel)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct group_desc *group;
+	const struct spacemit_pin_mux_config *configs;
+	unsigned int i, mux;
+	void __iomem *reg;
+
+	group = pinctrl_generic_get_group(pctldev, gsel);
+	if (!group)
+		return -EINVAL;
+
+	configs = group->data;
+
+	for (i = 0; i < group->grp.npins; i++) {
+		const struct spacemit_pin *spin = configs[i].pin;
+		u32 value = configs[i].config;
+		unsigned long flags;
+
+		reg = spacemit_pin_to_reg(pctrl, spin->pin);
+		mux = spacemit_dt_get_pin_mux(value);
+
+		raw_spin_lock_irqsave(&pctrl->lock, flags);
+		value = readl_relaxed(reg) & ~PAD_MUX;
+		writel_relaxed(mux | value, reg);
+		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+	}
+
+	return 0;
+}
+
+static int spacemit_request_gpio(struct pinctrl_dev *pctldev,
+				 struct pinctrl_gpio_range *range,
+				 unsigned int pin)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	struct spacemit_gpiofunc_range *frange = NULL;
+	struct list_head *pos, *tmp;
+	void __iomem *reg;
+
+	list_for_each_safe(pos, tmp, &pctrl->gpiofuncs) {
+		frange = list_entry(pos, struct spacemit_gpiofunc_range, node);
+		if (pin >= frange->offset + frange->npins
+			|| pin < frange->offset)
+			continue;
+
+		reg = spacemit_pin_to_reg(pctrl, pin);
+		writel(frange->gpiofunc | PAD_EDGE_CLEAR, reg);
+
+		break;
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops spacemit_pmx_ops = {
+	.get_functions_count	= pinmux_generic_get_function_count,
+	.get_function_name	= pinmux_generic_get_function_name,
+	.get_function_groups	= pinmux_generic_get_function_groups,
+	.set_mux		= spacemit_pmx_set_mux,
+	.gpio_request_enable	= spacemit_request_gpio,
+};
+
+#define PIN_CONFIG_K1_SLEW_RATE_ENABLE	(PIN_CONFIG_END + 1)
+#define PIN_CONFIG_K1_STRONG_PULL_UP	(PIN_CONFIG_END + 2)
+
+static const struct pinconf_generic_params spacemit_pinconf_custom_params[] = {
+	{ "spacemit,slew-rate-disable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 0 },
+	{ "spacemit,slew-rate-enable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 1 },
+	{ "spacemit,strong-pull-up",	PIN_CONFIG_K1_STRONG_PULL_UP,	 1 },
+};
+
+static int spacemit_add_gpio_func(struct device_node *node, struct spacemit_pinctrl *pctrl)
+{
+	const char *propname = "spacemit,gpio-range";
+	const char *cellname = "#spacemit,gpio-range-cells";
+	struct of_phandle_args gpiospec;
+	struct spacemit_gpiofunc_range *range;
+	int ret, i;
+
+	for (i = 0; ; i++) {
+		ret = of_parse_phandle_with_args(node, propname, cellname,
+						 i, &gpiospec);
+		/* Do not treat it as error. Only treat it as end condition. */
+		if (ret) {
+			ret = 0;
+			break;
+		}
+		range = devm_kzalloc(pctrl->dev, sizeof(*range), GFP_KERNEL);
+		if (!range) {
+			ret = -ENOMEM;
+			break;
+		}
+		range->offset = gpiospec.args[0];
+		range->npins = gpiospec.args[1];
+		range->gpiofunc = gpiospec.args[2];
+		mutex_lock(&pctrl->mutex);
+		list_add_tail(&range->node, &pctrl->gpiofuncs);
+		mutex_unlock(&pctrl->mutex);
+	}
+
+	return ret;
+}
+
+static int spacemit_pinconf_get(struct pinctrl_dev *pctldev,
+				unsigned int pin, unsigned long *config)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	int param = pinconf_to_config_param(*config);
+	u32 value, arg;
+
+	if (!pin)
+		return -EINVAL;
+
+	value = readl(spacemit_pin_to_reg(pctrl, pin));
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		arg = !FIELD_GET(PAD_PULL_EN, value);
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		arg = FIELD_GET(PAD_PULLDOWN, value);
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		arg = FIELD_GET(PAD_PULLUP, value);
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH_UA:
+		arg = FIELD_GET(PAD_DRIVE, value);
+		break;
+	case PIN_CONFIG_INPUT_SCHMITT:
+		arg = FIELD_GET(PAD_SCHMITT, value);
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		arg = FIELD_GET(PAD_SLEW_RATE, value);
+		break;
+	case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
+		arg = FIELD_GET(PAD_SLEW_RATE_EN, value);
+		break;
+	case PIN_CONFIG_K1_STRONG_PULL_UP:
+		arg = FIELD_GET(PAD_STRONG_PULL, value);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+
+	return 0;
+}
+
+static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
+					    unsigned long *configs,
+					    unsigned int num_configs,
+					    u32 *value)
+{
+	enum spacemit_pin_io_type type;
+	int i, param;
+	u32 v = 0, voltage = 0, arg, val;
+#define ENABLE_DRV_STRENGTH	BIT(1)
+#define ENABLE_SLEW_RATE	BIT(2)
+	u32 flag = 0, drv_strength, slew_rate;
+
+	if (!spin)
+		return -EINVAL;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+		arg = pinconf_to_config_argument(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			v &= ~(PAD_PULL_EN | PAD_PULLDOWN | PAD_PULLUP);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			v &= ~PAD_PULLDOWN;
+			v |= PAD_PULL_EN;
+			v |= FIELD_PREP(PAD_PULLDOWN, arg);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:
+			v &= ~PAD_PULLUP;
+			v |= PAD_PULL_EN;
+			v |= FIELD_PREP(PAD_PULLUP, arg);
+			break;
+		case PIN_CONFIG_DRIVE_STRENGTH_UA:
+			flag |= ENABLE_DRV_STRENGTH;
+			drv_strength = arg;
+			break;
+		case PIN_CONFIG_INPUT_SCHMITT:
+			slew_rate = arg;
+			break;
+		case PIN_CONFIG_POWER_SOURCE:
+			voltage = arg;
+			break;
+		case PIN_CONFIG_SLEW_RATE:
+			flag |= ENABLE_SLEW_RATE;
+			break;
+		case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
+			if (arg)
+				v |= PAD_SLEW_RATE_EN;
+			else
+				v &= ~PAD_SLEW_RATE_EN;
+			break;
+		case PIN_CONFIG_K1_STRONG_PULL_UP:
+			v &= ~PAD_STRONG_PULL;
+			v |= FIELD_PREP(PAD_STRONG_PULL, arg);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	if (flag & ENABLE_DRV_STRENGTH) {
+		type = spacemit_to_pin_io_type(spin);
+
+		/* fix external io type */
+		if (type == IO_TYPE_EXTERNAL) {
+			switch (voltage) {
+			case 1800:
+				type = IO_TYPE_1V8;
+				break;
+			case 3300:
+				type = IO_TYPE_3V3;
+				break;
+			default:
+				pr_err("pin(%d) lack power source\n", spin->pin);
+				return -EINVAL;
+			}
+		}
+
+		val = spacemit_get_driver_strength(type, drv_strength);
+
+		v &= ~PAD_DRIVE;
+		v |= FIELD_PREP(PAD_DRIVE, val);
+	}
+
+	if (flag & ENABLE_SLEW_RATE) {
+		v &= ~PAD_SLEW_RATE;
+		v |= FIELD_PREP(PAD_SLEW_RATE, arg);
+		/* check, driver strength & slew rate */
+		if (flag & ENABLE_DRV_STRENGTH) {
+			if (slew_rate != FIELD_GET(PAD_SLEW_RATE, v))
+				pr_warn("%s: slew rate conflict with driver strength\n", __func__);
+		} else {
+			v &= ~PAD_SCHMITT;
+			v |= FIELD_PREP(PAD_SCHMITT, slew_rate);
+		}
+	}
+
+	*value = v;
+
+	return 0;
+}
+
+static int spacemit_pin_set_config(struct spacemit_pinctrl *pctrl,
+				 unsigned int pin,
+				 u32 value)
+{
+	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
+	void __iomem *reg;
+	unsigned long flags;
+	unsigned int mux;
+
+	if (!pin)
+		return -EINVAL;
+
+	reg = spacemit_pin_to_reg(pctrl, spin->pin);
+
+	raw_spin_lock_irqsave(&pctrl->lock, flags);
+	mux = readl_relaxed(reg) & PAD_MUX;
+	writel_relaxed(mux | value, reg);
+	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
+
+	return 0;
+}
+
+static int spacemit_pinconf_set(struct pinctrl_dev *pctldev,
+				unsigned int pin, unsigned long *configs,
+				unsigned int num_configs)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
+	u32 value;
+
+	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
+		return -EINVAL;
+
+	return spacemit_pin_set_config(pctrl, pin, value);
+}
+
+static int spacemit_pinconf_group_set(struct pinctrl_dev *pctldev,
+				      unsigned int gsel,
+				      unsigned long *configs,
+				      unsigned int num_configs)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct spacemit_pin *spin;
+	const struct group_desc *group;
+	u32 value;
+	int i;
+
+	group = pinctrl_generic_get_group(pctldev, gsel);
+	if (!group)
+		return -EINVAL;
+
+	spin = spacemit_get_pin(pctrl, group->grp.pins[0]);
+	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
+		return -EINVAL;
+
+	for (i = 0; i < group->grp.npins; i++)
+		spacemit_pin_set_config(pctrl, group->grp.pins[i], value);
+
+	return 0;
+}
+
+static void spacemit_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+				      struct seq_file *seq, unsigned int pin)
+{
+	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
+	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
+	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
+	void __iomem *reg;
+
+	reg = spacemit_pin_to_reg(pctrl, pin);
+	seq_printf(seq, " reg (0x%x), io (%d)\n", readl(reg), type);
+}
+
+static const struct pinconf_ops spacemit_pinconf_ops = {
+	.pin_config_get			= spacemit_pinconf_get,
+	.pin_config_set			= spacemit_pinconf_set,
+	.pin_config_group_set		= spacemit_pinconf_group_set,
+	.pin_config_dbg_show		= spacemit_pinconf_dbg_show,
+	.is_generic			= true,
+};
+
+static int spacemit_pinctrl_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	struct spacemit_pinctrl *pctrl;
+	const struct spacemit_pinctrl_data *pctrl_data;
+	int ret;
+
+	pctrl_data = device_get_match_data(dev);
+	if (!pctrl_data)
+		return -ENODEV;
+
+	if (pctrl_data->npins == 0)
+		return dev_err_probe(dev, -EINVAL, "invalid pin data\n");
+
+	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
+	if (!pctrl)
+		return -ENOMEM;
+
+	pctrl->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pctrl->regs))
+		return PTR_ERR(pctrl->regs);
+
+	pctrl->pdesc.name = dev_name(dev);
+	pctrl->pdesc.pins = pctrl_data->pins;
+	pctrl->pdesc.npins = pctrl_data->npins;
+	pctrl->pdesc.pctlops = &spacemit_pctrl_ops;
+	pctrl->pdesc.pmxops = &spacemit_pmx_ops;
+	pctrl->pdesc.confops = &spacemit_pinconf_ops;
+	pctrl->pdesc.num_custom_params = ARRAY_SIZE(spacemit_pinconf_custom_params);
+	pctrl->pdesc.custom_params = spacemit_pinconf_custom_params;
+	pctrl->pdesc.owner = THIS_MODULE;
+
+	pctrl->data = pctrl_data;
+	pctrl->dev = dev;
+	raw_spin_lock_init(&pctrl->lock);
+	INIT_LIST_HEAD(&pctrl->gpiofuncs);
+	mutex_init(&pctrl->mutex);
+
+	platform_set_drvdata(pdev, pctrl);
+
+	ret = devm_pinctrl_register_and_init(dev, &pctrl->pdesc,
+					     pctrl, &pctrl->pctl_dev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "fail to register pinctrl driver\n");
+	ret = spacemit_add_gpio_func(np, pctrl);
+	if (ret < 0)
+		return dev_err_probe(dev, ret,
+				     "fail to get gpio function\n");
+
+	return pinctrl_enable(pctrl->pctl_dev);
+}
+
+static const struct pinctrl_pin_desc k1_pin_desc[] = {
+	PINCTRL_PIN(GPIO_00, "GPIO_00"),
+	PINCTRL_PIN(GPIO_01, "GPIO_01"),
+	PINCTRL_PIN(GPIO_02, "GPIO_02"),
+	PINCTRL_PIN(GPIO_03, "GPIO_03"),
+	PINCTRL_PIN(GPIO_04, "GPIO_04"),
+	PINCTRL_PIN(GPIO_05, "GPIO_05"),
+	PINCTRL_PIN(GPIO_06, "GPIO_06"),
+	PINCTRL_PIN(GPIO_07, "GPIO_07"),
+	PINCTRL_PIN(GPIO_08, "GPIO_08"),
+	PINCTRL_PIN(GPIO_09, "GPIO_09"),
+	PINCTRL_PIN(GPIO_10, "GPIO_10"),
+	PINCTRL_PIN(GPIO_11, "GPIO_11"),
+	PINCTRL_PIN(GPIO_12, "GPIO_12"),
+	PINCTRL_PIN(GPIO_13, "GPIO_13"),
+	PINCTRL_PIN(GPIO_14, "GPIO_14"),
+	PINCTRL_PIN(GPIO_15, "GPIO_15"),
+	PINCTRL_PIN(GPIO_16, "GPIO_16"),
+	PINCTRL_PIN(GPIO_17, "GPIO_17"),
+	PINCTRL_PIN(GPIO_18, "GPIO_18"),
+	PINCTRL_PIN(GPIO_19, "GPIO_19"),
+	PINCTRL_PIN(GPIO_20, "GPIO_20"),
+	PINCTRL_PIN(GPIO_21, "GPIO_21"),
+	PINCTRL_PIN(GPIO_22, "GPIO_22"),
+	PINCTRL_PIN(GPIO_23, "GPIO_23"),
+	PINCTRL_PIN(GPIO_24, "GPIO_24"),
+	PINCTRL_PIN(GPIO_25, "GPIO_25"),
+	PINCTRL_PIN(GPIO_26, "GPIO_26"),
+	PINCTRL_PIN(GPIO_27, "GPIO_27"),
+	PINCTRL_PIN(GPIO_28, "GPIO_28"),
+	PINCTRL_PIN(GPIO_29, "GPIO_29"),
+	PINCTRL_PIN(GPIO_30, "GPIO_30"),
+	PINCTRL_PIN(GPIO_31, "GPIO_31"),
+	PINCTRL_PIN(GPIO_32, "GPIO_32"),
+	PINCTRL_PIN(GPIO_33, "GPIO_33"),
+	PINCTRL_PIN(GPIO_34, "GPIO_34"),
+	PINCTRL_PIN(GPIO_35, "GPIO_35"),
+	PINCTRL_PIN(GPIO_36, "GPIO_36"),
+	PINCTRL_PIN(GPIO_37, "GPIO_37"),
+	PINCTRL_PIN(GPIO_38, "GPIO_38"),
+	PINCTRL_PIN(GPIO_39, "GPIO_39"),
+	PINCTRL_PIN(GPIO_40, "GPIO_40"),
+	PINCTRL_PIN(GPIO_41, "GPIO_41"),
+	PINCTRL_PIN(GPIO_42, "GPIO_42"),
+	PINCTRL_PIN(GPIO_43, "GPIO_43"),
+	PINCTRL_PIN(GPIO_44, "GPIO_44"),
+	PINCTRL_PIN(GPIO_45, "GPIO_45"),
+	PINCTRL_PIN(GPIO_46, "GPIO_46"),
+	PINCTRL_PIN(GPIO_47, "GPIO_47"),
+	PINCTRL_PIN(GPIO_48, "GPIO_48"),
+	PINCTRL_PIN(GPIO_49, "GPIO_49"),
+	PINCTRL_PIN(GPIO_50, "GPIO_50"),
+	PINCTRL_PIN(GPIO_51, "GPIO_51"),
+	PINCTRL_PIN(GPIO_52, "GPIO_52"),
+	PINCTRL_PIN(GPIO_53, "GPIO_53"),
+	PINCTRL_PIN(GPIO_54, "GPIO_54"),
+	PINCTRL_PIN(GPIO_55, "GPIO_55"),
+	PINCTRL_PIN(GPIO_56, "GPIO_56"),
+	PINCTRL_PIN(GPIO_57, "GPIO_57"),
+	PINCTRL_PIN(GPIO_58, "GPIO_58"),
+	PINCTRL_PIN(GPIO_59, "GPIO_59"),
+	PINCTRL_PIN(GPIO_60, "GPIO_60"),
+	PINCTRL_PIN(GPIO_61, "GPIO_61"),
+	PINCTRL_PIN(GPIO_62, "GPIO_62"),
+	PINCTRL_PIN(GPIO_63, "GPIO_63"),
+	PINCTRL_PIN(GPIO_64, "GPIO_64"),
+	PINCTRL_PIN(GPIO_65, "GPIO_65"),
+	PINCTRL_PIN(GPIO_66, "GPIO_66"),
+	PINCTRL_PIN(GPIO_67, "GPIO_67"),
+	PINCTRL_PIN(GPIO_68, "GPIO_68"),
+	PINCTRL_PIN(GPIO_69, "GPIO_69"),
+	PINCTRL_PIN(GPIO_70, "GPIO_70/PRI_DTI"),
+	PINCTRL_PIN(GPIO_71, "GPIO_71/PRI_TMS"),
+	PINCTRL_PIN(GPIO_72, "GPIO_72/PRI_TCK"),
+	PINCTRL_PIN(GPIO_73, "GPIO_73/PRI_TDO"),
+	PINCTRL_PIN(GPIO_74, "GPIO_74"),
+	PINCTRL_PIN(GPIO_75, "GPIO_75"),
+	PINCTRL_PIN(GPIO_76, "GPIO_76"),
+	PINCTRL_PIN(GPIO_77, "GPIO_77"),
+	PINCTRL_PIN(GPIO_78, "GPIO_78"),
+	PINCTRL_PIN(GPIO_79, "GPIO_79"),
+	PINCTRL_PIN(GPIO_80, "GPIO_80"),
+	PINCTRL_PIN(GPIO_81, "GPIO_81"),
+	PINCTRL_PIN(GPIO_82, "GPIO_82"),
+	PINCTRL_PIN(GPIO_83, "GPIO_83"),
+	PINCTRL_PIN(GPIO_84, "GPIO_84"),
+	PINCTRL_PIN(GPIO_85, "GPIO_85"),
+	PINCTRL_PIN(GPIO_86, "GPIO_86"),
+	PINCTRL_PIN(GPIO_87, "GPIO_87"),
+	PINCTRL_PIN(GPIO_88, "GPIO_88"),
+	PINCTRL_PIN(GPIO_89, "GPIO_89"),
+	PINCTRL_PIN(GPIO_90, "GPIO_90"),
+	PINCTRL_PIN(GPIO_91, "GPIO_91"),
+	PINCTRL_PIN(GPIO_92, "GPIO_92"),
+	PINCTRL_PIN(GPIO_93, "GPIO_93/PWR_SCL"),
+	PINCTRL_PIN(GPIO_94, "GPIO_94/PWR_SDA"),
+	PINCTRL_PIN(GPIO_95, "GPIO_95/VCX0_EN"),
+	PINCTRL_PIN(GPIO_96, "GPIO_96/DVL0"),
+	PINCTRL_PIN(GPIO_97, "GPIO_97/DVL1"),
+	PINCTRL_PIN(GPIO_98,  "GPIO_98/QSPI_DAT3"),
+	PINCTRL_PIN(GPIO_99,  "GPIO_99/QSPI_DAT2"),
+	PINCTRL_PIN(GPIO_100, "GPIO_100/QSPI_DAT1"),
+	PINCTRL_PIN(GPIO_101, "GPIO_101/QSPI_DAT0"),
+	PINCTRL_PIN(GPIO_102, "GPIO_102/QSPI_CLK"),
+	PINCTRL_PIN(GPIO_103, "GPIO_103/QSPI_CSI"),
+	PINCTRL_PIN(GPIO_104, "GPIO_104/MMC1_DAT3"),
+	PINCTRL_PIN(GPIO_105, "GPIO_105/MMC1_DAT2"),
+	PINCTRL_PIN(GPIO_106, "GPIO_106/MMC1_DAT1"),
+	PINCTRL_PIN(GPIO_107, "GPIO_107/MMC1_DAT0"),
+	PINCTRL_PIN(GPIO_108, "GPIO_108/MMC1_CMD"),
+	PINCTRL_PIN(GPIO_109, "GPIO_109/MMC1_CLK"),
+	PINCTRL_PIN(GPIO_110, "GPIO_110"),
+	PINCTRL_PIN(GPIO_111, "GPIO_111"),
+	PINCTRL_PIN(GPIO_112, "GPIO_112"),
+	PINCTRL_PIN(GPIO_113, "GPIO_113"),
+	PINCTRL_PIN(GPIO_114, "GPIO_114"),
+	PINCTRL_PIN(GPIO_115, "GPIO_115"),
+	PINCTRL_PIN(GPIO_116, "GPIO_116"),
+	PINCTRL_PIN(GPIO_117, "GPIO_117"),
+	PINCTRL_PIN(GPIO_118, "GPIO_118"),
+	PINCTRL_PIN(GPIO_119, "GPIO_119"),
+	PINCTRL_PIN(GPIO_120, "GPIO_120"),
+	PINCTRL_PIN(GPIO_121, "GPIO_121"),
+	PINCTRL_PIN(GPIO_122, "GPIO_122"),
+	PINCTRL_PIN(GPIO_123, "GPIO_123"),
+	PINCTRL_PIN(GPIO_124, "GPIO_124"),
+	PINCTRL_PIN(GPIO_125, "GPIO_125"),
+	PINCTRL_PIN(GPIO_126, "GPIO_126"),
+	PINCTRL_PIN(GPIO_127, "GPIO_127"),
+};
+
+static const struct spacemit_pin k1_pin_data[ARRAY_SIZE(k1_pin_desc)] = {
+	K1_FUNC_PIN(GPIO_00, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_01, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_02, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_03, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_04, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_05, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_06, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_07, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_08, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_09, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_10, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_11, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_12, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_13, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_14, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_15, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_16, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_17, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_18, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_19, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_20, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_21, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_22, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_23, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_24, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_25, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_26, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_27, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_28, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_29, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_30, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_31, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_32, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_33, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_34, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_35, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_36, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_37, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_38, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_39, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_40, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_41, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_42, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_43, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_44, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_45, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_46, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_47, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_48, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_49, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_50, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_51, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_52, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_53, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_54, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_55, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_56, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_57, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_58, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_59, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_60, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_61, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_62, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_63, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_64, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_65, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_66, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_67, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_68, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_69, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_70, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_71, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_72, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_73, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_74, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_75, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_76, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_77, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_78, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_79, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_80, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_81, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_82, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_83, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_84, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_85, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_86, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_87, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_88, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_89, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_90, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_91, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_92, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_93, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_94, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_95, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_96, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_97, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_98, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_99, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_100, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_101, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_102, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_103, IO_TYPE_EXTERNAL),
+	K1_FUNC_PIN(GPIO_104, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_105, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_106, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_107, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_108, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_109, IO_TYPE_3V3),
+	K1_FUNC_PIN(GPIO_110, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_111, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_112, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_113, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_114, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_115, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_116, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_117, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_118, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_119, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_120, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_121, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_122, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_123, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_124, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_125, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_126, IO_TYPE_1V8),
+	K1_FUNC_PIN(GPIO_127, IO_TYPE_1V8),
+};
+
+static const struct spacemit_pinctrl_data k1_pinctrl_data = {
+	.pins = k1_pin_desc,
+	.data = k1_pin_data,
+	.npins = ARRAY_SIZE(k1_pin_desc),
+};
+
+static const struct of_device_id k1_pinctrl_ids[] = {
+	{ .compatible = "spacemit,k1-pinctrl", .data = &k1_pinctrl_data },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, k1_pinctrl_ids);
+
+static struct platform_driver k1_pinctrl_driver = {
+	.probe	= spacemit_pinctrl_probe,
+	.driver	= {
+		.name			= "k1-pinctrl",
+		.suppress_bind_attrs	= true,
+		.of_match_table		= k1_pinctrl_ids,
+	},
+};
+module_platform_driver(k1_pinctrl_driver);
+
+MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
+MODULE_DESCRIPTION("Pinctrl driver for the SpacemiT K1 SoC");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.h b/drivers/pinctrl/spacemit/pinctrl-k1.h
new file mode 100644
index 0000000000000..9d6589b626e9d
--- /dev/null
+++ b/drivers/pinctrl/spacemit/pinctrl-k1.h
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
+
+#ifndef _PINCTRL_SPACEMIT_K1_H
+#define _PINCTRL_SPACEMIT_K1_H
+
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+#include <linux/platform_device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinconf.h>
+
+enum spacemit_pin_io_type {
+	IO_TYPE_NONE = 0,
+	IO_TYPE_1V8,
+	IO_TYPE_3V3,
+	IO_TYPE_EXTERNAL,
+};
+
+#define K1_PIN_IO_TYPE		GENMASK(2, 1)
+
+#define K1_PIN_CAP_IO_TYPE(type)		\
+	FIELD_PREP_CONST(K1_PIN_IO_TYPE, type)
+#define K1_PIN_GET_IO_TYPE(val)			\
+	FIELD_GET(K1_PIN_IO_TYPE, val)
+
+#define K1_FUNC_PIN(_id, _io)				\
+	{							\
+		.pin = (_id),					\
+		.flags = (K1_PIN_CAP_IO_TYPE(_io))		\
+	}
+
+#endif /* _PINCTRL_SPACEMIT_K1_H */

-- 
2.45.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
  2024-08-25 13:10 [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC Yixun Lan
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
  2024-08-25 13:10 ` [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT " Yixun Lan
@ 2024-08-25 13:10 ` Yixun Lan
  2024-08-26  7:55   ` Inochi Amaoto
  2024-08-25 13:10 ` [PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 Yixun Lan
  3 siblings, 1 reply; 19+ messages in thread
From: Yixun Lan @ 2024-08-25 13:10 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio, Yixun Lan

Add pinctrl device tree data to SpacemiT's K1 SoC.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
Note, only minimal device tree data added in this series,
which just try to demonstrate this pinctrl driver, but
more dt data can be added later, in separate patches.
---
 arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 19 +++++++++++++++++++
 arch/riscv/boot/dts/spacemit/k1.dtsi         |  5 +++++
 2 files changed, 24 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
new file mode 100644
index 0000000000000..38ccaad1209f5
--- /dev/null
+++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0 OR MIT
+/*
+ * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
+ */
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
+
+&pinctrl {
+	uart0_2_cfg: uart0-2-cfg {
+		uart0-2-pins {
+			pinmux = <K1_PADCONF(GPIO_68, 2)>,
+				 <K1_PADCONF(GPIO_69, 2)>;
+
+			bias-pull-up;
+			drive-strength-microamp = <32>;
+		};
+	};
+};
diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 0777bf9e01183..a2d5f7d4a942a 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -416,6 +416,11 @@ uart9: serial@d4017800 {
 			status = "disabled";
 		};
 
+		pinctrl: pinctrl@d401e000 {
+			compatible = "spacemit,k1-pinctrl";
+			reg = <0x0 0xd401e000 0x0 0x400>;
+		};
+
 		plic: interrupt-controller@e0000000 {
 			compatible = "spacemit,k1-plic", "sifive,plic-1.0.0";
 			reg = <0x0 0xe0000000 0x0 0x4000000>;

-- 
2.45.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3
  2024-08-25 13:10 [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC Yixun Lan
                   ` (2 preceding siblings ...)
  2024-08-25 13:10 ` [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for " Yixun Lan
@ 2024-08-25 13:10 ` Yixun Lan
  3 siblings, 0 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-25 13:10 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio, Yixun Lan

Before pinctrl driver implemented, the uart0 controller reply on
bootloader for setting correct pin mux and configurations.

Now, let's add pinctrl property to uart0 of Bananapi-F3 board.

Signed-off-by: Yixun Lan <dlan@gentoo.org>
---
 arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
index 023274189b492..bc88d4de25a62 100644
--- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
+++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
@@ -4,6 +4,7 @@
  */
 
 #include "k1.dtsi"
+#include "k1-pinctrl.dtsi"
 
 / {
 	model = "Banana Pi BPI-F3";
@@ -15,5 +16,7 @@ chosen {
 };
 
 &uart0 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart0_2_cfg>;
 	status = "okay";
 };

-- 
2.45.2


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
@ 2024-08-25 13:48   ` Krzysztof Kozlowski
  2024-08-26  1:36     ` Yixun Lan
       [not found]     ` <66cbdc2a.050a0220.2d7994.f671SMTPIN_ADDED_BROKEN@mx.google.com>
  2024-08-25 14:22   ` Rob Herring (Arm)
                     ` (3 subsequent siblings)
  4 siblings, 2 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-25 13:48 UTC (permalink / raw)
  To: Yixun Lan, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On 25/08/2024 15:10, Yixun Lan wrote:
> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.


Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

It's "dt-bindings:"

> 
> Two vendor specific properties are introduced here, As the pinctrl
> has dedicated slew rate enable control - bit[7], so we have
> spacemit,slew-rate-{enable,disable} for this. For the same reason,
> creating spacemit,strong-pull-up for the strong pull up control.

Huh, no, use generic properties. More on that below



> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
>  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
>  2 files changed, 295 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> new file mode 100644
> index 0000000000000..8adfc5ebbce37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SpacemiT K1 SoC Pin Controller
> +
> +maintainers:
> +  - Yixun Lan <dlan@gentoo.org>
> +
> +properties:
> +  compatible:
> +    const: spacemit,k1-pinctrl
> +
> +  reg:
> +    items:
> +      - description: pinctrl io memory base
> +
> +patternProperties:
> +  '-cfg$':
> +    type: object
> +    description: |

Do not need '|' unless you need to preserve formatting.

> +      A pinctrl node should contain at least one subnode representing the
> +      pinctrl groups available on the machine.
> +
> +    additionalProperties: false

Keep it before description.

> +
> +    patternProperties:
> +      '-pins$':
> +        type: object
> +        description: |
> +          Each subnode will list the pins it needs, and how they should
> +          be configured, with regard to muxer configuration, bias, input
> +          enable/disable, input schmitt trigger, slew-rate enable/disable,
> +          slew-rate, drive strength, power source.
> +        $ref: /schemas/pinctrl/pincfg-node.yaml
> +
> +        allOf:
> +          - $ref: pincfg-node.yaml#
> +          - $ref: pinmux-node.yaml#

You are duplicating refs.

> +
> +        properties:
> +          pinmux:
> +            description: |
> +              The list of GPIOs and their mux settings that properties in the
> +              node apply to. This should be set using the K1_PADCONF macro to
> +              construct the value.
> +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux

Hm why you need the ref?

> +
> +          bias-disable: true
> +
> +          bias-pull-up: true
> +
> +          bias-pull-down: true
> +
> +          drive-strength-microamp:
> +            description: |
> +              typical current when output high level, but in mA.
> +              1.8V output: 11, 21, 32, 42 (mA)
> +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +
> +          input-schmitt:
> +            description: |
> +              typical threshold for schmitt trigger.
> +              0: buffer mode
> +              1: trigger mode
> +              2, 3: trigger mode
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          power-source:
> +            description: external power supplies at 1.8v or 3.3v.
> +            enum: [ 1800, 3300 ]
> +
> +          slew-rate:
> +            description: |
> +              slew rate for output buffer
> +              0, 1: Slow speed

Hm? Surprising, 0 is slow speed?

> +              2: Medium speed
> +              3: Fast speed
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          spacemit,slew-rate-enable:
> +            description: enable slew rate.

The presence of slew-rate enables it, doesn't it?

> +            type: boolean
> +
> +          spacemit,slew-rate-disable:
> +            description: disable slew rate.
> +            type: boolean

Just use slew-rate, 0 disable, some value to match real slew-rate.

> +
> +          spacemit,strong-pull-up:
> +            description: enable strong pull up.

Do not duplicate the property name in description. You did not say
anything useful here. What is "strong"? bias-pull-up takes also an argument.

> +            type: boolean
> +
> +        required:
> +          - pinmux
> +
> +        additionalProperties: false

This goes up, before description.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pinctrl@d401e000 {
> +            compatible = "spacemit,k1-pinctrl";
> +            reg = <0x0 0xd401e000 0x0 0x400>;
> +            #pinctrl-cells = <2>;
> +            #gpio-range-cells = <3>;

This wasn't ever tested... :(
...

> diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> new file mode 100644
> index 0000000000000..13ef4aa6c53a3
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> + *
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_K1_H
> +#define _DT_BINDINGS_PINCTRL_K1_H
> +
> +#define PINMUX(pin, mux) \
> +	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
> +
> +/* pin offset */
> +#define PINID(x)	((x) + 1)
> +
> +#define GPIO_INVAL  0
> +#define GPIO_00     PINID(0)

Not really, pin numbers are not bindings. Drop entire header.

...

> +
> +#define SLEW_RATE_SLOW0		0
> +#define SLEW_RATE_SLOW1		1
> +#define SLEW_RATE_MEDIUM	2
> +#define SLEW_RATE_FAST		3

Not a binding, either. No usage in the driver.

> +
> +#define K1_PADCONF(pin, func) (((pin) << 16) | (func))

Not a binding.



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
  2024-08-25 13:48   ` Krzysztof Kozlowski
@ 2024-08-25 14:22   ` Rob Herring (Arm)
  2024-08-26  3:09   ` Yixun Lan
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 19+ messages in thread
From: Rob Herring (Arm) @ 2024-08-25 14:22 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Albert Ou, Palmer Dabbelt, Inochi Amaoto, Icenowy Zheng,
	Meng Zhang, Jisheng Zhang, linux-riscv, Yangyu Chen,
	Paul Walmsley, Conor Dooley, devicetree, Linus Walleij,
	Jesse Taube, Conor Dooley, Meng Zhang, linux-kernel,
	Krzysztof Kozlowski, linux-gpio


On Sun, 25 Aug 2024 13:10:02 +0000, Yixun Lan wrote:
> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> 
> Two vendor specific properties are introduced here, As the pinctrl
> has dedicated slew rate enable control - bit[7], so we have
> spacemit,slew-rate-{enable,disable} for this. For the same reason,
> creating spacemit,strong-pull-up for the strong pull up control.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
>  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
>  2 files changed, 295 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml: patternProperties:-cfg$:patternProperties:-pins$:properties:drive-strength-microamp: '$ref' should not be valid under {'const': '$ref'}
	hint: Standard unit suffix properties don't need a type $ref
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.example.dtb: dsi-phy@10215000: drive-strength-microamp: 4000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/phy/mediatek,dsi-phy.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/phy/mediatek,dsi-phy.example.dtb: dsi-phy@10215000: drive-strength-microamp: 4000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8183-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins1:drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8183-pinctrl.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8183-pinctrl.example.dtb: pins1: drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8186-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8186-pinctrl.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8186-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8195-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8195-pinctrl.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8195-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8188-pinctrl.example.dtb: pinctrl@10005000: i2c0-pins:pins:drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/pinctrl/mediatek,mt8188-pinctrl.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/mediatek,mt8188-pinctrl.example.dtb: pins: drive-strength-microamp: 1000 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.example.dtb: pinctrl@d401e000: '#gpio-range-cells', '#pinctrl-cells' do not match any of the regexes: '-cfg$', 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.example.dtb: uart0-2-pins: drive-strength-microamp: 32 is not of type 'array'
	from schema $id: http://devicetree.org/schemas/property-units.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240825-02-k1-pinctrl-v2-1-ddd38a345d12@gentoo.org

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:48   ` Krzysztof Kozlowski
@ 2024-08-26  1:36     ` Yixun Lan
  2024-08-26  4:19       ` Inochi Amaoto
       [not found]     ` <66cbdc2a.050a0220.2d7994.f671SMTPIN_ADDED_BROKEN@mx.google.com>
  1 sibling, 1 reply; 19+ messages in thread
From: Yixun Lan @ 2024-08-26  1:36 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
	Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

Hi Krzysztof: 

On 15:48 Sun 25 Aug     , Krzysztof Kozlowski wrote:
> On 25/08/2024 15:10, Yixun Lan wrote:
> > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> 
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> It's "dt-bindings:"
Ok, will fix in next version

> 
> > 
> > Two vendor specific properties are introduced here, As the pinctrl
> > has dedicated slew rate enable control - bit[7], so we have
> > spacemit,slew-rate-{enable,disable} for this. For the same reason,
> > creating spacemit,strong-pull-up for the strong pull up control.
> 
> Huh, no, use generic properties. More on that below
> 
see my reply below

> 
> 
> > 
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > ---
> >  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
> >  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
> >  2 files changed, 295 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> > new file mode 100644
> > index 0000000000000..8adfc5ebbce37
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> > @@ -0,0 +1,134 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: SpacemiT K1 SoC Pin Controller
> > +
> > +maintainers:
> > +  - Yixun Lan <dlan@gentoo.org>
> > +
> > +properties:
> > +  compatible:
> > +    const: spacemit,k1-pinctrl
> > +
> > +  reg:
> > +    items:
> > +      - description: pinctrl io memory base
> > +
> > +patternProperties:
> > +  '-cfg$':
> > +    type: object
> > +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
Ok
> > +      A pinctrl node should contain at least one subnode representing the
> > +      pinctrl groups available on the machine.
> > +
> > +    additionalProperties: false
> 
> Keep it before description.
Ok
> 
> > +
> > +    patternProperties:
> > +      '-pins$':
> > +        type: object
> > +        description: |
> > +          Each subnode will list the pins it needs, and how they should
> > +          be configured, with regard to muxer configuration, bias, input
> > +          enable/disable, input schmitt trigger, slew-rate enable/disable,
> > +          slew-rate, drive strength, power source.
> > +        $ref: /schemas/pinctrl/pincfg-node.yaml
> > +
> > +        allOf:
> > +          - $ref: pincfg-node.yaml#
> > +          - $ref: pinmux-node.yaml#
> 
> You are duplicating refs.
ok, will fix it
> 
> > +
> > +        properties:
> > +          pinmux:
> > +            description: |
> > +              The list of GPIOs and their mux settings that properties in the
> > +              node apply to. This should be set using the K1_PADCONF macro to
> > +              construct the value.
> > +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
> 
> Hm why you need the ref?
> 
will drop it
> > +
> > +          bias-disable: true
> > +
> > +          bias-pull-up: true
> > +
> > +          bias-pull-down: true
> > +
> > +          drive-strength-microamp:
> > +            description: |
> > +              typical current when output high level, but in mA.
> > +              1.8V output: 11, 21, 32, 42 (mA)
> > +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +
> > +          input-schmitt:
> > +            description: |
> > +              typical threshold for schmitt trigger.
> > +              0: buffer mode
> > +              1: trigger mode
> > +              2, 3: trigger mode
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            enum: [0, 1, 2, 3]
> > +
> > +          power-source:
> > +            description: external power supplies at 1.8v or 3.3v.
> > +            enum: [ 1800, 3300 ]
> > +
> > +          slew-rate:
> > +            description: |
> > +              slew rate for output buffer
> > +              0, 1: Slow speed
> 
> Hm? Surprising, 0 is slow speed?
> 
from docs, section 3.3.2.2 MFPR Register Description
0, 1 are same, both for slow speed
https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned

> > +              2: Medium speed
> > +              3: Fast speed
> > +            $ref: /schemas/types.yaml#/definitions/uint32
> > +            enum: [0, 1, 2, 3]
> > +
> > +          spacemit,slew-rate-enable:
> > +            description: enable slew rate.
> 
> The presence of slew-rate enables it, doesn't it?
> 
yes, this should work, I will take this approach, thanks

> > +            type: boolean
> > +
> > +          spacemit,slew-rate-disable:
> > +            description: disable slew rate.
> > +            type: boolean
> 
> Just use slew-rate, 0 disable, some value to match real slew-rate.
> 
sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for
disabling slew rate.

> > +
> > +          spacemit,strong-pull-up:
> > +            description: enable strong pull up.
> 
> Do not duplicate the property name in description. You did not say
> anything useful here. What is "strong"? bias-pull-up takes also an argument.
> 
there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad
I don't know how 'strong' it is if in ohms, will see if can get
more info on this (may expand the description)

I think using 'bias-pull-up' property with argument should also work,
but it occur to me it's more intuitive to introduce a property here, which
reflect the underlying hardware functionality. this is similar to starfive's jh7100
bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154
(refer to exist code doesn't mean always correct, so I need advice here)

I will keep this property unless there is objection, please let me know

> > +            type: boolean
> > +
> > +        required:
> > +          - pinmux
> > +
> > +        additionalProperties: false
> 
> This goes up, before description.
> 
Ok
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> > +
> > +    soc {
> > +        #address-cells = <2>;
> > +        #size-cells = <2>;
> > +
> > +        pinctrl@d401e000 {
> > +            compatible = "spacemit,k1-pinctrl";
> > +            reg = <0x0 0xd401e000 0x0 0x400>;
> > +            #pinctrl-cells = <2>;
> > +            #gpio-range-cells = <3>;
> 
> This wasn't ever tested... :(
> ...
will drop it
> 
> > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> > new file mode 100644
> > index 0000000000000..13ef4aa6c53a3
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> > @@ -0,0 +1,161 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > +/*
> > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
> > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> > + *
> > + */
> > +
> > +#ifndef _DT_BINDINGS_PINCTRL_K1_H
> > +#define _DT_BINDINGS_PINCTRL_K1_H
> > +
> > +#define PINMUX(pin, mux) \
> > +	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
> > +
> > +/* pin offset */
> > +#define PINID(x)	((x) + 1)
> > +
> > +#define GPIO_INVAL  0
> > +#define GPIO_00     PINID(0)
> 
> Not really, pin numbers are not bindings. Drop entire header.
> 
Ok, I will move them to dts folder, which should be file
arch/riscv/boot/dts/spacemit/k1-pinctrl.h

> ...
> 
> > +
> > +#define SLEW_RATE_SLOW0		0
> > +#define SLEW_RATE_SLOW1		1
> > +#define SLEW_RATE_MEDIUM	2
> > +#define SLEW_RATE_FAST		3
> 
> Not a binding, either. No usage in the driver.
Ok, will drop it

> 
> > +
> > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func))
> 
> Not a binding.
> 
same, move to dts

> 
> 
> Best regards,
> Krzysztof

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
  2024-08-25 13:48   ` Krzysztof Kozlowski
  2024-08-25 14:22   ` Rob Herring (Arm)
@ 2024-08-26  3:09   ` Yixun Lan
  2024-08-26  4:23   ` Inochi Amaoto
       [not found]   ` <66cbf3bb.050a0220.2632ed.b191SMTPIN_ADDED_BROKEN@mx.google.com>
  4 siblings, 0 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-26  3:09 UTC (permalink / raw)
  To: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio


On 13:10 Sun 25 Aug     , Yixun Lan wrote:
> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> 
> Two vendor specific properties are introduced here, As the pinctrl
> has dedicated slew rate enable control - bit[7], so we have
> spacemit,slew-rate-{enable,disable} for this. For the same reason,
> creating spacemit,strong-pull-up for the strong pull up control.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
>  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
>  2 files changed, 295 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> new file mode 100644
> index 0000000000000..8adfc5ebbce37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SpacemiT K1 SoC Pin Controller
> +
> +maintainers:
> +  - Yixun Lan <dlan@gentoo.org>
> +
> +properties:
> +  compatible:
> +    const: spacemit,k1-pinctrl
> +
> +  reg:
> +    items:
> +      - description: pinctrl io memory base
> +
> +patternProperties:
> +  '-cfg$':
> +    type: object
> +    description: |
> +      A pinctrl node should contain at least one subnode representing the
> +      pinctrl groups available on the machine.
> +
> +    additionalProperties: false
> +
> +    patternProperties:
> +      '-pins$':
> +        type: object
> +        description: |
> +          Each subnode will list the pins it needs, and how they should
> +          be configured, with regard to muxer configuration, bias, input
> +          enable/disable, input schmitt trigger, slew-rate enable/disable,
> +          slew-rate, drive strength, power source.
> +        $ref: /schemas/pinctrl/pincfg-node.yaml
> +
> +        allOf:
> +          - $ref: pincfg-node.yaml#
> +          - $ref: pinmux-node.yaml#
> +
> +        properties:
> +          pinmux:
> +            description: |
> +              The list of GPIOs and their mux settings that properties in the
> +              node apply to. This should be set using the K1_PADCONF macro to
> +              construct the value.
> +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
> +
> +          bias-disable: true
> +
> +          bias-pull-up: true
> +
> +          bias-pull-down: true
> +
> +          drive-strength-microamp:
just realise here should use 'drive-strength' which will comply to hw
will fix in next version

> +            description: |
> +              typical current when output high level, but in mA.
> +              1.8V output: 11, 21, 32, 42 (mA)
> +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +
> +          input-schmitt:
> +            description: |
> +              typical threshold for schmitt trigger.
> +              0: buffer mode
> +              1: trigger mode
> +              2, 3: trigger mode
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          power-source:
> +            description: external power supplies at 1.8v or 3.3v.
> +            enum: [ 1800, 3300 ]
> +
> +          slew-rate:
> +            description: |
> +              slew rate for output buffer
> +              0, 1: Slow speed
> +              2: Medium speed
> +              3: Fast speed
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          spacemit,slew-rate-enable:
> +            description: enable slew rate.
> +            type: boolean
> +
> +          spacemit,slew-rate-disable:
> +            description: disable slew rate.
> +            type: boolean
> +
> +          spacemit,strong-pull-up:
> +            description: enable strong pull up.
> +            type: boolean
> +
> +        required:
> +          - pinmux
> +
> +        additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pinctrl@d401e000 {
> +            compatible = "spacemit,k1-pinctrl";
> +            reg = <0x0 0xd401e000 0x0 0x400>;
> +            #pinctrl-cells = <2>;
> +            #gpio-range-cells = <3>;
> +
> +            uart0_2_cfg: uart0-2-cfg {
> +                uart0-2-pins {
> +                    pinmux = <K1_PADCONF(GPIO_68, 2)>,
> +                             <K1_PADCONF(GPIO_69, 2)>;
> +                    bias-pull-up;
> +                    drive-strength-microamp = <32>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> new file mode 100644
> index 0000000000000..13ef4aa6c53a3
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> + *
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_K1_H
> +#define _DT_BINDINGS_PINCTRL_K1_H
> +
> +#define PINMUX(pin, mux) \
> +	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
> +
> +/* pin offset */
> +#define PINID(x)	((x) + 1)
> +
> +#define GPIO_INVAL  0
> +#define GPIO_00     PINID(0)
> +#define GPIO_01     PINID(1)
> +#define GPIO_02     PINID(2)
> +#define GPIO_03     PINID(3)
> +#define GPIO_04     PINID(4)
> +#define GPIO_05     PINID(5)
> +#define GPIO_06     PINID(6)
> +#define GPIO_07     PINID(7)
> +#define GPIO_08     PINID(8)
> +#define GPIO_09     PINID(9)
> +#define GPIO_10     PINID(10)
> +#define GPIO_11     PINID(11)
> +#define GPIO_12     PINID(12)
> +#define GPIO_13     PINID(13)
> +#define GPIO_14     PINID(14)
> +#define GPIO_15     PINID(15)
> +#define GPIO_16     PINID(16)
> +#define GPIO_17     PINID(17)
> +#define GPIO_18     PINID(18)
> +#define GPIO_19     PINID(19)
> +#define GPIO_20     PINID(20)
> +#define GPIO_21     PINID(21)
> +#define GPIO_22     PINID(22)
> +#define GPIO_23     PINID(23)
> +#define GPIO_24     PINID(24)
> +#define GPIO_25     PINID(25)
> +#define GPIO_26     PINID(26)
> +#define GPIO_27     PINID(27)
> +#define GPIO_28     PINID(28)
> +#define GPIO_29     PINID(29)
> +#define GPIO_30     PINID(30)
> +#define GPIO_31     PINID(31)
> +
> +#define GPIO_32     PINID(32)
> +#define GPIO_33     PINID(33)
> +#define GPIO_34     PINID(34)
> +#define GPIO_35     PINID(35)
> +#define GPIO_36     PINID(36)
> +#define GPIO_37     PINID(37)
> +#define GPIO_38     PINID(38)
> +#define GPIO_39     PINID(39)
> +#define GPIO_40     PINID(40)
> +#define GPIO_41     PINID(41)
> +#define GPIO_42     PINID(42)
> +#define GPIO_43     PINID(43)
> +#define GPIO_44     PINID(44)
> +#define GPIO_45     PINID(45)
> +#define GPIO_46     PINID(46)
> +#define GPIO_47     PINID(47)
> +#define GPIO_48     PINID(48)
> +#define GPIO_49     PINID(49)
> +#define GPIO_50     PINID(50)
> +#define GPIO_51     PINID(51)
> +#define GPIO_52     PINID(52)
> +#define GPIO_53     PINID(53)
> +#define GPIO_54     PINID(54)
> +#define GPIO_55     PINID(55)
> +#define GPIO_56     PINID(56)
> +#define GPIO_57     PINID(57)
> +#define GPIO_58     PINID(58)
> +#define GPIO_59     PINID(59)
> +#define GPIO_60     PINID(60)
> +#define GPIO_61     PINID(61)
> +#define GPIO_62     PINID(62)
> +#define GPIO_63     PINID(63)
> +
> +#define GPIO_64     PINID(64)
> +#define GPIO_65     PINID(65)
> +#define GPIO_66     PINID(66)
> +#define GPIO_67     PINID(67)
> +#define GPIO_68     PINID(68)
> +#define GPIO_69     PINID(69)
> +#define GPIO_70     PINID(70)
> +#define GPIO_71     PINID(71)
> +#define GPIO_72     PINID(72)
> +#define GPIO_73     PINID(73)
> +#define GPIO_74     PINID(74)
> +#define GPIO_75     PINID(75)
> +#define GPIO_76     PINID(76)
> +#define GPIO_77     PINID(77)
> +#define GPIO_78     PINID(78)
> +#define GPIO_79     PINID(79)
> +#define GPIO_80     PINID(80)
> +#define GPIO_81     PINID(81)
> +#define GPIO_82     PINID(82)
> +#define GPIO_83     PINID(83)
> +#define GPIO_84     PINID(84)
> +#define GPIO_85     PINID(85)
> +
> +#define GPIO_101    PINID(89)
> +#define GPIO_100    PINID(90)
> +#define GPIO_99     PINID(91)
> +#define GPIO_98     PINID(92)
> +#define GPIO_103    PINID(93)
> +#define GPIO_102    PINID(94)
> +
> +#define GPIO_104    PINID(109)
> +#define GPIO_105    PINID(110)
> +#define GPIO_106    PINID(111)
> +#define GPIO_107    PINID(112)
> +#define GPIO_108    PINID(113)
> +#define GPIO_109    PINID(114)
> +#define GPIO_110    PINID(115)
> +
> +#define GPIO_93     PINID(116)
> +#define GPIO_94     PINID(117)
> +#define GPIO_95     PINID(118)
> +#define GPIO_96     PINID(119)
> +#define GPIO_97     PINID(120)
> +
> +#define GPIO_86     PINID(122)
> +#define GPIO_87     PINID(123)
> +#define GPIO_88     PINID(124)
> +#define GPIO_89     PINID(125)
> +#define GPIO_90     PINID(126)
> +#define GPIO_91     PINID(127)
> +#define GPIO_92     PINID(128)
> +
> +#define GPIO_111    PINID(130)
> +#define GPIO_112    PINID(131)
> +#define GPIO_113    PINID(132)
> +#define GPIO_114    PINID(133)
> +#define GPIO_115    PINID(134)
> +#define GPIO_116    PINID(135)
> +#define GPIO_117    PINID(136)
> +#define GPIO_118    PINID(137)
> +#define GPIO_119    PINID(138)
> +#define GPIO_120    PINID(139)
> +#define GPIO_121    PINID(140)
> +#define GPIO_122    PINID(141)
> +#define GPIO_123    PINID(142)
> +#define GPIO_124    PINID(143)
> +#define GPIO_125    PINID(144)
> +#define GPIO_126    PINID(145)
> +#define GPIO_127    PINID(146)
> +
> +#define SLEW_RATE_SLOW0		0
> +#define SLEW_RATE_SLOW1		1
> +#define SLEW_RATE_MEDIUM	2
> +#define SLEW_RATE_FAST		3
> +
> +#define K1_PADCONF(pin, func) (((pin) << 16) | (func))
> +
> +#endif /* _DT_BINDINGS_PINCTRL_K1_H */
> 
> -- 
> 2.45.2

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-26  1:36     ` Yixun Lan
@ 2024-08-26  4:19       ` Inochi Amaoto
  0 siblings, 0 replies; 19+ messages in thread
From: Inochi Amaoto @ 2024-08-26  4:19 UTC (permalink / raw)
  To: Yixun Lan, Krzysztof Kozlowski
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
	Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On Mon, Aug 26, 2024 at 01:36:35AM GMT, Yixun Lan wrote:
> Hi Krzysztof: 
> 
> On 15:48 Sun 25 Aug     , Krzysztof Kozlowski wrote:
> > On 25/08/2024 15:10, Yixun Lan wrote:
> > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> > 
> > 
> > Please use subject prefixes matching the subsystem. You can get them for
> > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> > your patch is touching. For bindings, the preferred subjects are
> > explained here:
> > https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> > 
> > It's "dt-bindings:"
> Ok, will fix in next version
> 
> > 
> > > 
> > > Two vendor specific properties are introduced here, As the pinctrl
> > > has dedicated slew rate enable control - bit[7], so we have
> > > spacemit,slew-rate-{enable,disable} for this. For the same reason,
> > > creating spacemit,strong-pull-up for the strong pull up control.
> > 
> > Huh, no, use generic properties. More on that below
> > 
> see my reply below
> 
> > 
> > 
> > > 
> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > > ---
> > >  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
> > >  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
> > >  2 files changed, 295 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> > > new file mode 100644
> > > index 0000000000000..8adfc5ebbce37
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> > > @@ -0,0 +1,134 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: SpacemiT K1 SoC Pin Controller
> > > +
> > > +maintainers:
> > > +  - Yixun Lan <dlan@gentoo.org>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: spacemit,k1-pinctrl
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description: pinctrl io memory base
> > > +
> > > +patternProperties:
> > > +  '-cfg$':
> > > +    type: object
> > > +    description: |
> > 
> > Do not need '|' unless you need to preserve formatting.
> > 
> Ok
> > > +      A pinctrl node should contain at least one subnode representing the
> > > +      pinctrl groups available on the machine.
> > > +
> > > +    additionalProperties: false
> > 
> > Keep it before description.
> Ok
> > 
> > > +
> > > +    patternProperties:
> > > +      '-pins$':
> > > +        type: object
> > > +        description: |
> > > +          Each subnode will list the pins it needs, and how they should
> > > +          be configured, with regard to muxer configuration, bias, input
> > > +          enable/disable, input schmitt trigger, slew-rate enable/disable,
> > > +          slew-rate, drive strength, power source.
> > > +        $ref: /schemas/pinctrl/pincfg-node.yaml
> > > +
> > > +        allOf:
> > > +          - $ref: pincfg-node.yaml#
> > > +          - $ref: pinmux-node.yaml#
> > 
> > You are duplicating refs.
> ok, will fix it
> > 
> > > +
> > > +        properties:
> > > +          pinmux:
> > > +            description: |
> > > +              The list of GPIOs and their mux settings that properties in the
> > > +              node apply to. This should be set using the K1_PADCONF macro to
> > > +              construct the value.
> > > +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
> > 
> > Hm why you need the ref?
> > 
> will drop it
> > > +
> > > +          bias-disable: true
> > > +
> > > +          bias-pull-up: true
> > > +
> > > +          bias-pull-down: true
> > > +
> > > +          drive-strength-microamp:
> > > +            description: |
> > > +              typical current when output high level, but in mA.
> > > +              1.8V output: 11, 21, 32, 42 (mA)
> > > +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
> > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > +
> > > +          input-schmitt:
> > > +            description: |
> > > +              typical threshold for schmitt trigger.
> > > +              0: buffer mode
> > > +              1: trigger mode
> > > +              2, 3: trigger mode
> > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > +            enum: [0, 1, 2, 3]
> > > +
> > > +          power-source:
> > > +            description: external power supplies at 1.8v or 3.3v.
> > > +            enum: [ 1800, 3300 ]
> > > +
> > > +          slew-rate:
> > > +            description: |
> > > +              slew rate for output buffer
> > > +              0, 1: Slow speed
> > 
> > Hm? Surprising, 0 is slow speed?
> > 
> from docs, section 3.3.2.2 MFPR Register Description
> 0, 1 are same, both for slow speed
> https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned
> 

I suspect this should not be set separately, but with the 
drive-strength. The document shows that the DS field sets
both drive strength and slew rate. This at least tell the
value 0 and 1 may be different.

> > > +              2: Medium speed
> > > +              3: Fast speed
> > > +            $ref: /schemas/types.yaml#/definitions/uint32
> > > +            enum: [0, 1, 2, 3]
> > > +
> > > +          spacemit,slew-rate-enable:
> > > +            description: enable slew rate.
> > 
> > The presence of slew-rate enables it, doesn't it?
> > 
> yes, this should work, I will take this approach, thanks
> 
> > > +            type: boolean
> > > +
> > > +          spacemit,slew-rate-disable:
> > > +            description: disable slew rate.
> > > +            type: boolean
> > 
> > Just use slew-rate, 0 disable, some value to match real slew-rate.
> > 
> sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for
> disabling slew rate.
> 
> > > +
> > > +          spacemit,strong-pull-up:
> > > +            description: enable strong pull up.
> > 
> > Do not duplicate the property name in description. You did not say
> > anything useful here. What is "strong"? bias-pull-up takes also an argument.
> > 
> there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad
> I don't know how 'strong' it is if in ohms, will see if can get
> more info on this (may expand the description)
> 
> I think using 'bias-pull-up' property with argument should also work,
> but it occur to me it's more intuitive to introduce a property here, which
> reflect the underlying hardware functionality. this is similar to starfive's jh7100
> bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154
> (refer to exist code doesn't mean always correct, so I need advice here)
> 
> I will keep this property unless there is objection, please let me know
> 
> > > +            type: boolean
> > > +
> > > +        required:
> > > +          - pinmux
> > > +
> > > +        additionalProperties: false
> > 
> > This goes up, before description.
> > 
> Ok
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> > > +
> > > +    soc {
> > > +        #address-cells = <2>;
> > > +        #size-cells = <2>;
> > > +
> > > +        pinctrl@d401e000 {
> > > +            compatible = "spacemit,k1-pinctrl";
> > > +            reg = <0x0 0xd401e000 0x0 0x400>;
> > > +            #pinctrl-cells = <2>;
> > > +            #gpio-range-cells = <3>;
> > 
> > This wasn't ever tested... :(
> > ...
> will drop it
> > 
> > > diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> > > new file mode 100644
> > > index 0000000000000..13ef4aa6c53a3
> > > --- /dev/null
> > > +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> > > @@ -0,0 +1,161 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > > +/*
> > > + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
> > > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> > > + *
> > > + */
> > > +
> > > +#ifndef _DT_BINDINGS_PINCTRL_K1_H
> > > +#define _DT_BINDINGS_PINCTRL_K1_H
> > > +
> > > +#define PINMUX(pin, mux) \
> > > +	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
> > > +
> > > +/* pin offset */
> > > +#define PINID(x)	((x) + 1)
> > > +
> > > +#define GPIO_INVAL  0
> > > +#define GPIO_00     PINID(0)
> > 
> > Not really, pin numbers are not bindings. Drop entire header.
> > 
> Ok, I will move them to dts folder, which should be file
> arch/riscv/boot/dts/spacemit/k1-pinctrl.h
> 
> > ...
> > 
> > > +
> > > +#define SLEW_RATE_SLOW0		0
> > > +#define SLEW_RATE_SLOW1		1
> > > +#define SLEW_RATE_MEDIUM	2
> > > +#define SLEW_RATE_FAST		3
> > 
> > Not a binding, either. No usage in the driver.
> Ok, will drop it
> 
> > 
> > > +
> > > +#define K1_PADCONF(pin, func) (((pin) << 16) | (func))
> > 
> > Not a binding.
> > 
> same, move to dts
> 
> > 
> > 
> > Best regards,
> > Krzysztof
> 
> -- 
> Yixun Lan (dlan)
> Gentoo Linux Developer
> GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
                     ` (2 preceding siblings ...)
  2024-08-26  3:09   ` Yixun Lan
@ 2024-08-26  4:23   ` Inochi Amaoto
       [not found]   ` <66cbf3bb.050a0220.2632ed.b191SMTPIN_ADDED_BROKEN@mx.google.com>
  4 siblings, 0 replies; 19+ messages in thread
From: Inochi Amaoto @ 2024-08-26  4:23 UTC (permalink / raw)
  To: Yixun Lan, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On Sun, Aug 25, 2024 at 01:10:02PM GMT, Yixun Lan wrote:
> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> 
> Two vendor specific properties are introduced here, As the pinctrl
> has dedicated slew rate enable control - bit[7], so we have
> spacemit,slew-rate-{enable,disable} for this. For the same reason,
> creating spacemit,strong-pull-up for the strong pull up control.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
>  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
>  2 files changed, 295 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> new file mode 100644
> index 0000000000000..8adfc5ebbce37
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
> @@ -0,0 +1,134 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SpacemiT K1 SoC Pin Controller
> +
> +maintainers:
> +  - Yixun Lan <dlan@gentoo.org>
> +
> +properties:
> +  compatible:
> +    const: spacemit,k1-pinctrl
> +
> +  reg:
> +    items:
> +      - description: pinctrl io memory base
> +
> +patternProperties:
> +  '-cfg$':
> +    type: object
> +    description: |
> +      A pinctrl node should contain at least one subnode representing the
> +      pinctrl groups available on the machine.
> +
> +    additionalProperties: false
> +
> +    patternProperties:
> +      '-pins$':
> +        type: object
> +        description: |
> +          Each subnode will list the pins it needs, and how they should
> +          be configured, with regard to muxer configuration, bias, input
> +          enable/disable, input schmitt trigger, slew-rate enable/disable,
> +          slew-rate, drive strength, power source.
> +        $ref: /schemas/pinctrl/pincfg-node.yaml
> +
> +        allOf:
> +          - $ref: pincfg-node.yaml#
> +          - $ref: pinmux-node.yaml#
> +
> +        properties:
> +          pinmux:
> +            description: |
> +              The list of GPIOs and their mux settings that properties in the
> +              node apply to. This should be set using the K1_PADCONF macro to
> +              construct the value.
> +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
> +
> +          bias-disable: true
> +
> +          bias-pull-up: true
> +
> +          bias-pull-down: true
> +
> +          drive-strength-microamp:
> +            description: |
> +              typical current when output high level, but in mA.
> +              1.8V output: 11, 21, 32, 42 (mA)
> +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +

If there are no other runtime restrictions, you should add extra
check with the power-source, then you can always get vaild 
settings.

Regards,
Inochi

> +          input-schmitt:
> +            description: |
> +              typical threshold for schmitt trigger.
> +              0: buffer mode
> +              1: trigger mode
> +              2, 3: trigger mode
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          power-source:
> +            description: external power supplies at 1.8v or 3.3v.
> +            enum: [ 1800, 3300 ]
> +
> +          slew-rate:
> +            description: |
> +              slew rate for output buffer
> +              0, 1: Slow speed
> +              2: Medium speed
> +              3: Fast speed
> +            $ref: /schemas/types.yaml#/definitions/uint32
> +            enum: [0, 1, 2, 3]
> +
> +          spacemit,slew-rate-enable:
> +            description: enable slew rate.
> +            type: boolean
> +
> +          spacemit,slew-rate-disable:
> +            description: disable slew rate.
> +            type: boolean
> +
> +          spacemit,strong-pull-up:
> +            description: enable strong pull up.
> +            type: boolean
> +
> +        required:
> +          - pinmux
> +
> +        additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> +
> +    soc {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        pinctrl@d401e000 {
> +            compatible = "spacemit,k1-pinctrl";
> +            reg = <0x0 0xd401e000 0x0 0x400>;
> +            #pinctrl-cells = <2>;
> +            #gpio-range-cells = <3>;
> +
> +            uart0_2_cfg: uart0-2-cfg {
> +                uart0-2-pins {
> +                    pinmux = <K1_PADCONF(GPIO_68, 2)>,
> +                             <K1_PADCONF(GPIO_69, 2)>;
> +                    bias-pull-up;
> +                    drive-strength-microamp = <32>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> new file mode 100644
> index 0000000000000..13ef4aa6c53a3
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h
> @@ -0,0 +1,161 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright (c) 2022-2024 SpacemiT (Hangzhou) Technology Co. Ltd
> + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> + *
> + */
> +
> +#ifndef _DT_BINDINGS_PINCTRL_K1_H
> +#define _DT_BINDINGS_PINCTRL_K1_H
> +
> +#define PINMUX(pin, mux) \
> +	(((pin) & 0xffff) | (((mux) & 0xff) << 16))
> +
> +/* pin offset */
> +#define PINID(x)	((x) + 1)
> +
> +#define GPIO_INVAL  0
> +#define GPIO_00     PINID(0)
> +#define GPIO_01     PINID(1)
> +#define GPIO_02     PINID(2)
> +#define GPIO_03     PINID(3)
> +#define GPIO_04     PINID(4)
> +#define GPIO_05     PINID(5)
> +#define GPIO_06     PINID(6)
> +#define GPIO_07     PINID(7)
> +#define GPIO_08     PINID(8)
> +#define GPIO_09     PINID(9)
> +#define GPIO_10     PINID(10)
> +#define GPIO_11     PINID(11)
> +#define GPIO_12     PINID(12)
> +#define GPIO_13     PINID(13)
> +#define GPIO_14     PINID(14)
> +#define GPIO_15     PINID(15)
> +#define GPIO_16     PINID(16)
> +#define GPIO_17     PINID(17)
> +#define GPIO_18     PINID(18)
> +#define GPIO_19     PINID(19)
> +#define GPIO_20     PINID(20)
> +#define GPIO_21     PINID(21)
> +#define GPIO_22     PINID(22)
> +#define GPIO_23     PINID(23)
> +#define GPIO_24     PINID(24)
> +#define GPIO_25     PINID(25)
> +#define GPIO_26     PINID(26)
> +#define GPIO_27     PINID(27)
> +#define GPIO_28     PINID(28)
> +#define GPIO_29     PINID(29)
> +#define GPIO_30     PINID(30)
> +#define GPIO_31     PINID(31)
> +
> +#define GPIO_32     PINID(32)
> +#define GPIO_33     PINID(33)
> +#define GPIO_34     PINID(34)
> +#define GPIO_35     PINID(35)
> +#define GPIO_36     PINID(36)
> +#define GPIO_37     PINID(37)
> +#define GPIO_38     PINID(38)
> +#define GPIO_39     PINID(39)
> +#define GPIO_40     PINID(40)
> +#define GPIO_41     PINID(41)
> +#define GPIO_42     PINID(42)
> +#define GPIO_43     PINID(43)
> +#define GPIO_44     PINID(44)
> +#define GPIO_45     PINID(45)
> +#define GPIO_46     PINID(46)
> +#define GPIO_47     PINID(47)
> +#define GPIO_48     PINID(48)
> +#define GPIO_49     PINID(49)
> +#define GPIO_50     PINID(50)
> +#define GPIO_51     PINID(51)
> +#define GPIO_52     PINID(52)
> +#define GPIO_53     PINID(53)
> +#define GPIO_54     PINID(54)
> +#define GPIO_55     PINID(55)
> +#define GPIO_56     PINID(56)
> +#define GPIO_57     PINID(57)
> +#define GPIO_58     PINID(58)
> +#define GPIO_59     PINID(59)
> +#define GPIO_60     PINID(60)
> +#define GPIO_61     PINID(61)
> +#define GPIO_62     PINID(62)
> +#define GPIO_63     PINID(63)
> +
> +#define GPIO_64     PINID(64)
> +#define GPIO_65     PINID(65)
> +#define GPIO_66     PINID(66)
> +#define GPIO_67     PINID(67)
> +#define GPIO_68     PINID(68)
> +#define GPIO_69     PINID(69)
> +#define GPIO_70     PINID(70)
> +#define GPIO_71     PINID(71)
> +#define GPIO_72     PINID(72)
> +#define GPIO_73     PINID(73)
> +#define GPIO_74     PINID(74)
> +#define GPIO_75     PINID(75)
> +#define GPIO_76     PINID(76)
> +#define GPIO_77     PINID(77)
> +#define GPIO_78     PINID(78)
> +#define GPIO_79     PINID(79)
> +#define GPIO_80     PINID(80)
> +#define GPIO_81     PINID(81)
> +#define GPIO_82     PINID(82)
> +#define GPIO_83     PINID(83)
> +#define GPIO_84     PINID(84)
> +#define GPIO_85     PINID(85)
> +
> +#define GPIO_101    PINID(89)
> +#define GPIO_100    PINID(90)
> +#define GPIO_99     PINID(91)
> +#define GPIO_98     PINID(92)
> +#define GPIO_103    PINID(93)
> +#define GPIO_102    PINID(94)
> +
> +#define GPIO_104    PINID(109)
> +#define GPIO_105    PINID(110)
> +#define GPIO_106    PINID(111)
> +#define GPIO_107    PINID(112)
> +#define GPIO_108    PINID(113)
> +#define GPIO_109    PINID(114)
> +#define GPIO_110    PINID(115)
> +
> +#define GPIO_93     PINID(116)
> +#define GPIO_94     PINID(117)
> +#define GPIO_95     PINID(118)
> +#define GPIO_96     PINID(119)
> +#define GPIO_97     PINID(120)
> +
> +#define GPIO_86     PINID(122)
> +#define GPIO_87     PINID(123)
> +#define GPIO_88     PINID(124)
> +#define GPIO_89     PINID(125)
> +#define GPIO_90     PINID(126)
> +#define GPIO_91     PINID(127)
> +#define GPIO_92     PINID(128)
> +
> +#define GPIO_111    PINID(130)
> +#define GPIO_112    PINID(131)
> +#define GPIO_113    PINID(132)
> +#define GPIO_114    PINID(133)
> +#define GPIO_115    PINID(134)
> +#define GPIO_116    PINID(135)
> +#define GPIO_117    PINID(136)
> +#define GPIO_118    PINID(137)
> +#define GPIO_119    PINID(138)
> +#define GPIO_120    PINID(139)
> +#define GPIO_121    PINID(140)
> +#define GPIO_122    PINID(141)
> +#define GPIO_123    PINID(142)
> +#define GPIO_124    PINID(143)
> +#define GPIO_125    PINID(144)
> +#define GPIO_126    PINID(145)
> +#define GPIO_127    PINID(146)
> +
> +#define SLEW_RATE_SLOW0		0
> +#define SLEW_RATE_SLOW1		1
> +#define SLEW_RATE_MEDIUM	2
> +#define SLEW_RATE_FAST		3
> +
> +#define K1_PADCONF(pin, func) (((pin) << 16) | (func))
> +
> +#endif /* _DT_BINDINGS_PINCTRL_K1_H */
> 
> -- 
> 2.45.2
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
       [not found]     ` <66cbdc2a.050a0220.2d7994.f671SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2024-08-26  5:37       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 19+ messages in thread
From: Krzysztof Kozlowski @ 2024-08-26  5:37 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
	Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On 26/08/2024 03:36, Yixun Lan wrote:
> Hi Krzysztof: 
> 
> On 15:48 Sun 25 Aug     , Krzysztof Kozlowski wrote:
>> On 25/08/2024 15:10, Yixun Lan wrote:
>>> Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
>>
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
>> your patch is touching. For bindings, the preferred subjects are
>> explained here:
>> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>>
>> It's "dt-bindings:"
> Ok, will fix in next version
> 
>>
>>>
>>> Two vendor specific properties are introduced here, As the pinctrl
>>> has dedicated slew rate enable control - bit[7], so we have
>>> spacemit,slew-rate-{enable,disable} for this. For the same reason,
>>> creating spacemit,strong-pull-up for the strong pull up control.
>>
>> Huh, no, use generic properties. More on that below
>>
> see my reply below
> 
>>
>>
>>>
>>> Signed-off-by: Yixun Lan <dlan@gentoo.org>
>>> ---
>>>  .../bindings/pinctrl/spacemit,k1-pinctrl.yaml      | 134 +++++++++++++++++
>>>  include/dt-bindings/pinctrl/spacemit,k1-pinctrl.h  | 161 +++++++++++++++++++++
>>>  2 files changed, 295 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
>>> new file mode 100644
>>> index 0000000000000..8adfc5ebbce37
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/spacemit,k1-pinctrl.yaml
>>> @@ -0,0 +1,134 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pinctrl/spacemit,k1-pinctrl.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: SpacemiT K1 SoC Pin Controller
>>> +
>>> +maintainers:
>>> +  - Yixun Lan <dlan@gentoo.org>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: spacemit,k1-pinctrl
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: pinctrl io memory base
>>> +
>>> +patternProperties:
>>> +  '-cfg$':
>>> +    type: object
>>> +    description: |
>>
>> Do not need '|' unless you need to preserve formatting.
>>
> Ok
>>> +      A pinctrl node should contain at least one subnode representing the
>>> +      pinctrl groups available on the machine.
>>> +
>>> +    additionalProperties: false
>>
>> Keep it before description.
> Ok
>>
>>> +
>>> +    patternProperties:
>>> +      '-pins$':
>>> +        type: object
>>> +        description: |
>>> +          Each subnode will list the pins it needs, and how they should
>>> +          be configured, with regard to muxer configuration, bias, input
>>> +          enable/disable, input schmitt trigger, slew-rate enable/disable,
>>> +          slew-rate, drive strength, power source.
>>> +        $ref: /schemas/pinctrl/pincfg-node.yaml
>>> +
>>> +        allOf:
>>> +          - $ref: pincfg-node.yaml#
>>> +          - $ref: pinmux-node.yaml#
>>
>> You are duplicating refs.
> ok, will fix it
>>
>>> +
>>> +        properties:
>>> +          pinmux:
>>> +            description: |
>>> +              The list of GPIOs and their mux settings that properties in the
>>> +              node apply to. This should be set using the K1_PADCONF macro to
>>> +              construct the value.
>>> +            $ref: /schemas/pinctrl/pinmux-node.yaml#/properties/pinmux
>>
>> Hm why you need the ref?
>>
> will drop it
>>> +
>>> +          bias-disable: true
>>> +
>>> +          bias-pull-up: true
>>> +
>>> +          bias-pull-down: true
>>> +
>>> +          drive-strength-microamp:
>>> +            description: |
>>> +              typical current when output high level, but in mA.
>>> +              1.8V output: 11, 21, 32, 42 (mA)
>>> +              3.3V output: 7, 10, 13, 16, 19, 23, 26, 29 (mA)
>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>> +
>>> +          input-schmitt:
>>> +            description: |
>>> +              typical threshold for schmitt trigger.
>>> +              0: buffer mode
>>> +              1: trigger mode
>>> +              2, 3: trigger mode
>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>> +            enum: [0, 1, 2, 3]
>>> +
>>> +          power-source:
>>> +            description: external power supplies at 1.8v or 3.3v.
>>> +            enum: [ 1800, 3300 ]
>>> +
>>> +          slew-rate:
>>> +            description: |
>>> +              slew rate for output buffer
>>> +              0, 1: Slow speed
>>
>> Hm? Surprising, 0 is slow speed?
>>
> from docs, section 3.3.2.2 MFPR Register Description
> 0, 1 are same, both for slow speed
> https://developer.spacemit.com/documentation?token=An1vwTwKaigaXRkYfwmcznTXned

Don't store here register value.

> 
>>> +              2: Medium speed
>>> +              3: Fast speed
>>> +            $ref: /schemas/types.yaml#/definitions/uint32
>>> +            enum: [0, 1, 2, 3]
>>> +
>>> +          spacemit,slew-rate-enable:
>>> +            description: enable slew rate.
>>
>> The presence of slew-rate enables it, doesn't it?
>>
> yes, this should work, I will take this approach, thanks
> 
>>> +            type: boolean
>>> +
>>> +          spacemit,slew-rate-disable:
>>> +            description: disable slew rate.
>>> +            type: boolean
>>
>> Just use slew-rate, 0 disable, some value to match real slew-rate.
>>
> sounds good to me, since 0, 1 indicate same meaning, can re-use 0 for
> disabling slew rate.
> 
>>> +
>>> +          spacemit,strong-pull-up:
>>> +            description: enable strong pull up.
>>
>> Do not duplicate the property name in description. You did not say
>> anything useful here. What is "strong"? bias-pull-up takes also an argument.
>>
> there is a dedicated strong pull bit[3] for I2C, SD card kinds of pad
> I don't know how 'strong' it is if in ohms, will see if can get
> more info on this (may expand the description)
> 
> I think using 'bias-pull-up' property with argument should also work,
> but it occur to me it's more intuitive to introduce a property here, which
> reflect the underlying hardware functionality. this is similar to starfive's jh7100
> bindings/pinctrl/starfive,jh7100-pinctrl.yaml:154
> (refer to exist code doesn't mean always correct, so I need advice here)

No, avoid introducing properties which duplicate existing generic ones.

Same story was with qualcomm and it was possible to use specific value.

> 
> I will keep this property unless there is objection, please let me know
> 
>>> +            type: boolean
>>> +
>>> +        required:
>>> +          - pinmux
>>> +
>>> +        additionalProperties: false
>>
>> This goes up, before description.
>>
> Ok
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    #include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
>>> +
>>> +    soc {
>>> +        #address-cells = <2>;
>>> +        #size-cells = <2>;
>>> +
>>> +        pinctrl@d401e000 {
>>> +            compatible = "spacemit,k1-pinctrl";
>>> +            reg = <0x0 0xd401e000 0x0 0x400>;
>>> +            #pinctrl-cells = <2>;
>>> +            #gpio-range-cells = <3>;
>>
>> This wasn't ever tested... :(
>> ...
> will drop it

Test your patches instead.



Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
  2024-08-25 13:10 ` [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for " Yixun Lan
@ 2024-08-26  7:55   ` Inochi Amaoto
  2024-08-27  3:03     ` Yixun Lan
  0 siblings, 1 reply; 19+ messages in thread
From: Inochi Amaoto @ 2024-08-26  7:55 UTC (permalink / raw)
  To: Yixun Lan, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On Sun, Aug 25, 2024 at 01:10:04PM GMT, Yixun Lan wrote:
> Add pinctrl device tree data to SpacemiT's K1 SoC.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
> Note, only minimal device tree data added in this series,
> which just try to demonstrate this pinctrl driver, but
> more dt data can be added later, in separate patches.
> ---
>  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 19 +++++++++++++++++++
>  arch/riscv/boot/dts/spacemit/k1.dtsi         |  5 +++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> new file mode 100644
> index 0000000000000..38ccaad1209f5
> --- /dev/null
> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0 OR MIT
> +/*
> + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> + */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> +


> +&pinctrl {
> +	uart0_2_cfg: uart0-2-cfg {
> +		uart0-2-pins {
> +			pinmux = <K1_PADCONF(GPIO_68, 2)>,
> +				 <K1_PADCONF(GPIO_69, 2)>;
> +
> +			bias-pull-up;
> +			drive-strength-microamp = <32>;
> +		};
> +	};
> +};

No common file is needed at least for now. You can put it
in the board dts. Also, squash this into the next patch as
it is more related to the uart.

> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index 0777bf9e01183..a2d5f7d4a942a 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -416,6 +416,11 @@ uart9: serial@d4017800 {
>  			status = "disabled";
>  		};
>  
> +		pinctrl: pinctrl@d401e000 {
> +			compatible = "spacemit,k1-pinctrl";
> +			reg = <0x0 0xd401e000 0x0 0x400>;
> +		};
> +
>  		plic: interrupt-controller@e0000000 {
>  			compatible = "spacemit,k1-plic", "sifive,plic-1.0.0";
>  			reg = <0x0 0xe0000000 0x0 0x4000000>;
> 
> -- 
> 2.45.2
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
  2024-08-25 13:10 ` [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT " Yixun Lan
@ 2024-08-26  8:16   ` Inochi Amaoto
  2024-08-27  3:30     ` Yixun Lan
  0 siblings, 1 reply; 19+ messages in thread
From: Inochi Amaoto @ 2024-08-26  8:16 UTC (permalink / raw)
  To: Yixun Lan, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Conor Dooley
  Cc: Yangyu Chen, Jesse Taube, Jisheng Zhang, Inochi Amaoto,
	Icenowy Zheng, Meng Zhang, Meng Zhang, devicetree, linux-riscv,
	linux-kernel, linux-gpio

On Sun, Aug 25, 2024 at 01:10:03PM GMT, Yixun Lan wrote:
> SpacemiT's K1 SoC has a pinctrl controller which use single register
> to describe all functions, which include bias pull up/down, drive
> strength, schmitter trigger, slew rate, strong pull up, mux mode.
> 
> Signed-off-by: Yixun Lan <dlan@gentoo.org>
> ---
>  drivers/pinctrl/Kconfig               |    1 +
>  drivers/pinctrl/Makefile              |    1 +
>  drivers/pinctrl/spacemit/Kconfig      |   17 +
>  drivers/pinctrl/spacemit/Makefile     |    3 +
>  drivers/pinctrl/spacemit/pinctrl-k1.c | 1012 +++++++++++++++++++++++++++++++++
>  drivers/pinctrl/spacemit/pinctrl-k1.h |   36 ++
>  6 files changed, 1070 insertions(+)
> 
> diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> index 7e4f93a3bc7ac..8358da47cd00d 100644
> --- a/drivers/pinctrl/Kconfig
> +++ b/drivers/pinctrl/Kconfig
> @@ -583,6 +583,7 @@ source "drivers/pinctrl/qcom/Kconfig"
>  source "drivers/pinctrl/realtek/Kconfig"
>  source "drivers/pinctrl/renesas/Kconfig"
>  source "drivers/pinctrl/samsung/Kconfig"
> +source "drivers/pinctrl/spacemit/Kconfig"
>  source "drivers/pinctrl/spear/Kconfig"
>  source "drivers/pinctrl/sprd/Kconfig"
>  source "drivers/pinctrl/starfive/Kconfig"
> diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> index cc809669405ab..553beead7ffa0 100644
> --- a/drivers/pinctrl/Makefile
> +++ b/drivers/pinctrl/Makefile
> @@ -73,6 +73,7 @@ obj-y				+= qcom/
>  obj-$(CONFIG_ARCH_REALTEK)      += realtek/
>  obj-$(CONFIG_PINCTRL_RENESAS)	+= renesas/
>  obj-$(CONFIG_PINCTRL_SAMSUNG)	+= samsung/
> +obj-y				+= spacemit/
>  obj-$(CONFIG_PINCTRL_SPEAR)	+= spear/
>  obj-y				+= sprd/
>  obj-$(CONFIG_SOC_STARFIVE)	+= starfive/
> diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
> new file mode 100644
> index 0000000000000..168f8a5ffbb95
> --- /dev/null
> +++ b/drivers/pinctrl/spacemit/Kconfig
> @@ -0,0 +1,17 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Sophgo SoC PINCTRL drivers
> +#
> +
> +config PINCTRL_SPACEMIT_K1
> +	tristate "SpacemiT K1 SoC Pinctrl driver"
> +	depends on ARCH_SPACEMIT || COMPILE_TEST
> +	depends on OF
> +	select GENERIC_PINCTRL_GROUPS
> +	select GENERIC_PINMUX_FUNCTIONS
> +	select GENERIC_PINCONF
> +	help
> +	  Say Y to select the pinctrl driver for K1 SoC.
> +	  This pin controller allows selecting the mux function for
> +	  each pin. This driver can also be built as a module called
> +	  pinctrl-k1.
> diff --git a/drivers/pinctrl/spacemit/Makefile b/drivers/pinctrl/spacemit/Makefile
> new file mode 100644
> index 0000000000000..fede1e80fe0ff
> --- /dev/null
> +++ b/drivers/pinctrl/spacemit/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_PINCTRL_SPACEMIT_K1)	+= pinctrl-k1.o
> diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> new file mode 100644
> index 0000000000000..9ed9530c78150
> --- /dev/null
> +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> @@ -0,0 +1,1012 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
> +
> +#include <linux/bitfield.h>
> +#include <linux/export.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/seq_file.h>
> +#include <linux/spinlock.h>
> +#include <linux/module.h>
> +
> +#include <linux/pinctrl/consumer.h>
> +#include <linux/pinctrl/machine.h>
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinconf.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinmux.h>
> +
> +#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> +
> +#include "../core.h"
> +#include "../pinctrl-utils.h"
> +#include "../pinconf.h"
> +#include "../pinmux.h"
> +#include "pinctrl-k1.h"
> +
> +/*
> + * +---------+----------+-----------+--------+--------+----------+--------+
> + * |   pull  |   drive  | schmitter |  slew  |  edge  |  strong  |   mux  |
> + * | up/down | strength |  trigger  |  rate  | detect |   pull   |  mode  |
> + * +---------+----------+-----------+--------+--------+----------+--------+
> + *   3 bits     3 bits     2 bits     1 bit    3 bits     1 bit    3 bits
> + */
> +
> +#define PAD_MUX			GENMASK(2, 0)
> +#define PAD_STRONG_PULL		BIT(3)
> +#define PAD_EDGE_RISE		BIT(4)
> +#define PAD_EDGE_FALL		BIT(5)
> +#define PAD_EDGE_CLEAR		BIT(6)
> +#define PAD_SLEW_RATE		GENMASK(12, 11)
> +#define PAD_SLEW_RATE_EN	BIT(7)
> +#define PAD_SCHMITT		GENMASK(9, 8)
> +#define PAD_DRIVE		GENMASK(12, 10)
> +#define PAD_PULLDOWN		BIT(13)
> +#define PAD_PULLUP		BIT(14)
> +#define PAD_PULL_EN		BIT(15)
> +
> +#define spacemit_pin_to_reg(pctrl, pin)		((pctrl)->regs + (pin << 2))
> +
> +struct spacemit_pin {
> +	u16				pin;
> +	u16				flags;
> +};
> +
> +struct spacemit_pinctrl {
> +	struct device				*dev;
> +	struct pinctrl_dev			*pctl_dev;
> +	const struct spacemit_pinctrl_data	*data;
> +	struct pinctrl_desc			pdesc;
> +
> +	struct list_head			gpiofuncs;
> +
> +	struct mutex				mutex;
> +	raw_spinlock_t				lock;
> +
> +	void __iomem				*regs;
> +};
> +
> +struct spacemit_pinctrl_data {
> +	const struct pinctrl_pin_desc   *pins;
> +	const struct spacemit_pin	*data;
> +	u16				npins;
> +};
> +
> +struct spacemit_gpiofunc_range {
> +	u32		offset;
> +	u32		npins;
> +	u32		gpiofunc;
> +	struct		list_head node;
> +};
> +
> +struct spacemit_pin_mux_config {
> +	const struct spacemit_pin	*pin;
> +	u32		config;
> +};
> +
> +struct spacemit_pin_drv_strength {
> +	u8		val;
> +	u32		mA;
> +};
> +
> +static unsigned int spacemit_dt_get_pin(u32 value)
> +{
> +	return (value >> 16) & GENMASK(15, 0);
> +}
> +
> +static unsigned int spacemit_dt_get_pin_mux(u32 value)
> +{
> +	return value & GENMASK(15, 0);
> +}
> +
> +static const struct spacemit_pin *spacemit_get_pin(struct spacemit_pinctrl *pctrl,
> +						   unsigned long pin)
> +{
> +	const struct spacemit_pin *pdata = pctrl->data->data;
> +	int i;
> +
> +	for (i = 0; i < pctrl->data->npins; i++) {
> +		if (pin == pdata[i].pin)
> +			return &pdata[i];
> +	}
> +
> +	return NULL;
> +}
> +
> +static inline enum spacemit_pin_io_type spacemit_to_pin_io_type(
> +	const struct spacemit_pin *pin)
> +{
> +	return K1_PIN_GET_IO_TYPE(pin->flags);
> +}
> +
> +/* External: IO voltage via external source, can be 1.8V or 3.3V */
> +static const char * const io_type_desc[] = {
> +	"None",
> +	"Fixed/1V8",
> +	"Fixed/3V3",
> +	"External",
> +};
> +
> +static void spacemit_pctrl_dbg_show(struct pinctrl_dev *pctldev,
> +				    struct seq_file *seq, unsigned int pin)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> +	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
> +	void __iomem *reg;
> +	u32 value;
> +
> +	seq_printf(seq, "pin: %u ", spin->pin);
> +
> +	seq_printf(seq, "type: %s ", io_type_desc[type]);
> +
> +	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> +	value = readl(reg);
> +	seq_printf(seq, "mux: 0x%08x ", value);
> +}
> +

> +/* use IO high level output current as the table */
> +static struct spacemit_pin_drv_strength spacemit_ds_1v8_tbl[4] = {
> +	{ (0 << 1), 11 },
> +	{ (1 << 1), 21 },
> +	{ (2 << 1), 32 },
> +	{ (3 << 1), 42 },
> +};
> +
> +static struct spacemit_pin_drv_strength spacemit_ds_3v3_tbl[8] = {
> +	{ 0,  7 },
> +	{ 2, 10 },
> +	{ 4, 13 },
> +	{ 6, 16 },
> +	{ 1, 19 },
> +	{ 3, 23 },
> +	{ 5, 26 },
> +	{ 7, 29 },
> +};

Why these two use different format?

> +
> +static inline u8 spacemit_get_ds_value(struct spacemit_pin_drv_strength *tbl,
> +				       u32 num, u32 mA)
> +{
> +	int i;
> +
> +	for (i = 0; i < num; i++)
> +		if (mA <= tbl[i].mA)
> +			return tbl[i].val;
> +
> +	return tbl[num - 1].val;
> +}
> +
> +static inline u8 spacemit_get_driver_strength(enum spacemit_pin_io_type type,
> +					      u32 mA)
> +{
> +	switch (type) {
> +	case IO_TYPE_1V8:
> +		return spacemit_get_ds_value(spacemit_ds_1v8_tbl,
> +					     ARRAY_SIZE(spacemit_ds_1v8_tbl),
> +					     mA);
> +	case IO_TYPE_3V3:
> +		return spacemit_get_ds_value(spacemit_ds_3v3_tbl,
> +					     ARRAY_SIZE(spacemit_ds_3v3_tbl),
> +					     mA);
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int spacemit_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> +					 struct device_node *np,
> +					 struct pinctrl_map **maps,
> +					 unsigned int *num_maps)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	struct device *dev = pctrl->dev;
> +	struct device_node *child;
> +	struct pinctrl_map *map;
> +	const char **grpnames;
> +	const char *grpname;
> +	int ngroups = 0;
> +	int nmaps = 0;
> +	int ret;
> +
> +	for_each_available_child_of_node(np, child)
> +		ngroups += 1;
> +
> +	grpnames = devm_kcalloc(dev, ngroups, sizeof(*grpnames), GFP_KERNEL);
> +	if (!grpnames)
> +		return -ENOMEM;
> +
> +	map = devm_kcalloc(dev, ngroups * 2, sizeof(*map), GFP_KERNEL);
> +	if (!map)
> +		return -ENOMEM;
> +
> +	ngroups = 0;
> +	mutex_lock(&pctrl->mutex);
> +	for_each_available_child_of_node(np, child) {
> +		int npins = of_property_count_u32_elems(child, "pinmux");
> +		unsigned int *pins;
> +		struct spacemit_pin_mux_config *pinmuxs;
> +		u32 config;
> +		int i;
> +
> +		if (npins < 1) {
> +			dev_err(dev, "invalid pinctrl group %pOFn.%pOFn\n",
> +				np, child);
> +			ret = -EINVAL;
> +			goto dt_failed;
> +		}
> +
> +		grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn",
> +					 np, child);
> +		if (!grpname) {
> +			ret = -ENOMEM;
> +			goto dt_failed;
> +		}
> +
> +		grpnames[ngroups++] = grpname;
> +
> +		pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> +		if (!pins) {
> +			ret = -ENOMEM;
> +			goto dt_failed;
> +		}
> +
> +		pinmuxs = devm_kcalloc(dev, npins, sizeof(*pinmuxs), GFP_KERNEL);
> +		if (!pinmuxs) {
> +			ret = -ENOMEM;
> +			goto dt_failed;
> +		}
> +
> +		for (i = 0; i < npins; i++) {
> +			ret = of_property_read_u32_index(child, "pinmux",
> +							 i, &config);
> +
> +			if (ret)
> +				goto dt_failed;
> +
> +			pins[i] = spacemit_dt_get_pin(config);
> +			pinmuxs[i].config = config;
> +			pinmuxs[i].pin = spacemit_get_pin(pctrl, pins[i]);
> +
> +			if (!pinmuxs[i].pin) {
> +				dev_err(dev, "failed to get pin %d\n", pins[i]);
> +				ret = -ENODEV;
> +				goto dt_failed;
> +			}
> +		}
> +

It will be better to check the power-source before inserting a map.

> +		map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
> +		map[nmaps].data.mux.function = np->name;
> +		map[nmaps].data.mux.group = grpname;
> +		nmaps += 1;
> +
> +		ret = pinctrl_generic_add_group(pctldev, grpname,
> +						pins, npins, pinmuxs);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to add group %s: %d\n", grpname, ret);
> +			goto dt_failed;
> +		}
> +
> +		ret = pinconf_generic_parse_dt_config(child, pctldev,
> +						      &map[nmaps].data.configs.configs,
> +						      &map[nmaps].data.configs.num_configs);
> +		if (ret) {
> +			dev_err(dev, "failed to parse pin config of group %s: %d\n",
> +				grpname, ret);
> +			goto dt_failed;
> +		}
> +
> +		if (map[nmaps].data.configs.num_configs == 0)
> +			continue;
> +
> +		map[nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> +		map[nmaps].data.configs.group_or_pin = grpname;
> +		nmaps += 1;
> +	}
> +
> +	ret = pinmux_generic_add_function(pctldev, np->name,
> +					  grpnames, ngroups, NULL);
> +	if (ret < 0) {
> +		dev_err(dev, "error adding function %s: %d\n", np->name, ret);
> +		goto function_failed;
> +	}
> +
> +	*maps = map;
> +	*num_maps = nmaps;
> +	mutex_unlock(&pctrl->mutex);
> +
> +	return 0;
> +
> +dt_failed:
> +	of_node_put(child);
> +function_failed:
> +	pinctrl_utils_free_map(pctldev, map, nmaps);
> +	mutex_unlock(&pctrl->mutex);
> +	return ret;
> +}
> +
> +static const struct pinctrl_ops spacemit_pctrl_ops = {
> +	.get_groups_count	= pinctrl_generic_get_group_count,
> +	.get_group_name		= pinctrl_generic_get_group_name,
> +	.get_group_pins		= pinctrl_generic_get_group_pins,
> +	.pin_dbg_show		= spacemit_pctrl_dbg_show,
> +	.dt_node_to_map		= spacemit_pctrl_dt_node_to_map,
> +	.dt_free_map		= pinctrl_utils_free_map,
> +};
> +
> +static int spacemit_pmx_set_mux(struct pinctrl_dev *pctldev,
> +			      unsigned int fsel, unsigned int gsel)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct group_desc *group;
> +	const struct spacemit_pin_mux_config *configs;
> +	unsigned int i, mux;
> +	void __iomem *reg;
> +
> +	group = pinctrl_generic_get_group(pctldev, gsel);
> +	if (!group)
> +		return -EINVAL;
> +
> +	configs = group->data;
> +
> +	for (i = 0; i < group->grp.npins; i++) {
> +		const struct spacemit_pin *spin = configs[i].pin;
> +		u32 value = configs[i].config;
> +		unsigned long flags;
> +
> +		reg = spacemit_pin_to_reg(pctrl, spin->pin);
> +		mux = spacemit_dt_get_pin_mux(value);
> +
> +		raw_spin_lock_irqsave(&pctrl->lock, flags);
> +		value = readl_relaxed(reg) & ~PAD_MUX;
> +		writel_relaxed(mux | value, reg);
> +		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +	}
> +
> +	return 0;
> +}
> +
> +static int spacemit_request_gpio(struct pinctrl_dev *pctldev,
> +				 struct pinctrl_gpio_range *range,
> +				 unsigned int pin)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	struct spacemit_gpiofunc_range *frange = NULL;
> +	struct list_head *pos, *tmp;
> +	void __iomem *reg;
> +
> +	list_for_each_safe(pos, tmp, &pctrl->gpiofuncs) {
> +		frange = list_entry(pos, struct spacemit_gpiofunc_range, node);
> +		if (pin >= frange->offset + frange->npins
> +			|| pin < frange->offset)
> +			continue;
> +
> +		reg = spacemit_pin_to_reg(pctrl, pin);
> +		writel(frange->gpiofunc | PAD_EDGE_CLEAR, reg);
> +
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct pinmux_ops spacemit_pmx_ops = {
> +	.get_functions_count	= pinmux_generic_get_function_count,
> +	.get_function_name	= pinmux_generic_get_function_name,
> +	.get_function_groups	= pinmux_generic_get_function_groups,
> +	.set_mux		= spacemit_pmx_set_mux,
> +	.gpio_request_enable	= spacemit_request_gpio,
> +};
> +
> +#define PIN_CONFIG_K1_SLEW_RATE_ENABLE	(PIN_CONFIG_END + 1)
> +#define PIN_CONFIG_K1_STRONG_PULL_UP	(PIN_CONFIG_END + 2)
> +
> +static const struct pinconf_generic_params spacemit_pinconf_custom_params[] = {
> +	{ "spacemit,slew-rate-disable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 0 },
> +	{ "spacemit,slew-rate-enable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 1 },

> +	{ "spacemit,strong-pull-up",	PIN_CONFIG_K1_STRONG_PULL_UP,	 1 },

default enable?

> +};
> +
> +static int spacemit_add_gpio_func(struct device_node *node, struct spacemit_pinctrl *pctrl)
> +{
> +	const char *propname = "spacemit,gpio-range";
> +	const char *cellname = "#spacemit,gpio-range-cells";
> +	struct of_phandle_args gpiospec;
> +	struct spacemit_gpiofunc_range *range;
> +	int ret, i;
> +
> +	for (i = 0; ; i++) {
> +		ret = of_parse_phandle_with_args(node, propname, cellname,
> +						 i, &gpiospec);
> +		/* Do not treat it as error. Only treat it as end condition. */
> +		if (ret) {
> +			ret = 0;
> +			break;
> +		}
> +		range = devm_kzalloc(pctrl->dev, sizeof(*range), GFP_KERNEL);
> +		if (!range) {
> +			ret = -ENOMEM;
> +			break;
> +		}
> +		range->offset = gpiospec.args[0];
> +		range->npins = gpiospec.args[1];
> +		range->gpiofunc = gpiospec.args[2];
> +		mutex_lock(&pctrl->mutex);
> +		list_add_tail(&range->node, &pctrl->gpiofuncs);
> +		mutex_unlock(&pctrl->mutex);
> +	}
> +
> +	return ret;
> +}
> +
> +static int spacemit_pinconf_get(struct pinctrl_dev *pctldev,
> +				unsigned int pin, unsigned long *config)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	int param = pinconf_to_config_param(*config);
> +	u32 value, arg;
> +
> +	if (!pin)
> +		return -EINVAL;
> +
> +	value = readl(spacemit_pin_to_reg(pctrl, pin));
> +
> +	switch (param) {
> +	case PIN_CONFIG_BIAS_DISABLE:
> +		arg = !FIELD_GET(PAD_PULL_EN, value);
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_DOWN:
> +		arg = FIELD_GET(PAD_PULLDOWN, value);
> +		break;
> +	case PIN_CONFIG_BIAS_PULL_UP:
> +		arg = FIELD_GET(PAD_PULLUP, value);
> +		break;
> +	case PIN_CONFIG_DRIVE_STRENGTH_UA:
> +		arg = FIELD_GET(PAD_DRIVE, value);
> +		break;
> +	case PIN_CONFIG_INPUT_SCHMITT:
> +		arg = FIELD_GET(PAD_SCHMITT, value);
> +		break;
> +	case PIN_CONFIG_SLEW_RATE:
> +		arg = FIELD_GET(PAD_SLEW_RATE, value);
> +		break;
> +	case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
> +		arg = FIELD_GET(PAD_SLEW_RATE_EN, value);
> +		break;
> +	case PIN_CONFIG_K1_STRONG_PULL_UP:
> +		arg = FIELD_GET(PAD_STRONG_PULL, value);
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*config = pinconf_to_config_packed(param, arg);
> +
> +	return 0;
> +}
> +
> +static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
> +					    unsigned long *configs,
> +					    unsigned int num_configs,
> +					    u32 *value)
> +{
> +	enum spacemit_pin_io_type type;
> +	int i, param;
> +	u32 v = 0, voltage = 0, arg, val;

> +#define ENABLE_DRV_STRENGTH	BIT(1)
> +#define ENABLE_SLEW_RATE	BIT(2)

move this before the function.

> +	u32 flag = 0, drv_strength, slew_rate;
> +
> +	if (!spin)
> +		return -EINVAL;
> +
> +	for (i = 0; i < num_configs; i++) {
> +		param = pinconf_to_config_param(configs[i]);
> +		arg = pinconf_to_config_argument(configs[i]);
> +
> +		switch (param) {
> +		case PIN_CONFIG_BIAS_DISABLE:
> +			v &= ~(PAD_PULL_EN | PAD_PULLDOWN | PAD_PULLUP);
> +			break;
> +		case PIN_CONFIG_BIAS_PULL_DOWN:
> +			v &= ~PAD_PULLDOWN;
> +			v |= PAD_PULL_EN;
> +			v |= FIELD_PREP(PAD_PULLDOWN, arg);
> +			break;
> +		case PIN_CONFIG_BIAS_PULL_UP:
> +			v &= ~PAD_PULLUP;
> +			v |= PAD_PULL_EN;
> +			v |= FIELD_PREP(PAD_PULLUP, arg);
> +			break;
> +		case PIN_CONFIG_DRIVE_STRENGTH_UA:
> +			flag |= ENABLE_DRV_STRENGTH;
> +			drv_strength = arg;
> +			break;
> +		case PIN_CONFIG_INPUT_SCHMITT:
> +			slew_rate = arg;
> +			break;
> +		case PIN_CONFIG_POWER_SOURCE:
> +			voltage = arg;
> +			break;
> +		case PIN_CONFIG_SLEW_RATE:
> +			flag |= ENABLE_SLEW_RATE;
> +			break;
> +		case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
> +			if (arg)
> +				v |= PAD_SLEW_RATE_EN;
> +			else
> +				v &= ~PAD_SLEW_RATE_EN;
> +			break;
> +		case PIN_CONFIG_K1_STRONG_PULL_UP:
> +			v &= ~PAD_STRONG_PULL;
> +			v |= FIELD_PREP(PAD_STRONG_PULL, arg);
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (flag & ENABLE_DRV_STRENGTH) {
> +		type = spacemit_to_pin_io_type(spin);
> +
> +		/* fix external io type */
> +		if (type == IO_TYPE_EXTERNAL) {
> +			switch (voltage) {
> +			case 1800:
> +				type = IO_TYPE_1V8;
> +				break;
> +			case 3300:
> +				type = IO_TYPE_3V3;
> +				break;
> +			default:
> +				pr_err("pin(%d) lack power source\n", spin->pin);
> +				return -EINVAL;
> +			}
> +		}
> +
> +		val = spacemit_get_driver_strength(type, drv_strength);
> +
> +		v &= ~PAD_DRIVE;
> +		v |= FIELD_PREP(PAD_DRIVE, val);
> +	}
> +

> +	if (flag & ENABLE_SLEW_RATE) {
> +		v &= ~PAD_SLEW_RATE;
> +		v |= FIELD_PREP(PAD_SLEW_RATE, arg);
> +		/* check, driver strength & slew rate */
> +		if (flag & ENABLE_DRV_STRENGTH) {
> +			if (slew_rate != FIELD_GET(PAD_SLEW_RATE, v))
> +				pr_warn("%s: slew rate conflict with driver strength\n", __func__);

Should this return -EINVAL?

> +		} else {
> +			v &= ~PAD_SCHMITT;
> +			v |= FIELD_PREP(PAD_SCHMITT, slew_rate);
> +		}
> +	}
> +
> +	*value = v;
> +
> +	return 0;
> +}
> +
> +static int spacemit_pin_set_config(struct spacemit_pinctrl *pctrl,
> +				 unsigned int pin,
> +				 u32 value)
> +{
> +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> +	void __iomem *reg;
> +	unsigned long flags;
> +	unsigned int mux;
> +
> +	if (!pin)
> +		return -EINVAL;
> +
> +	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> +
> +	raw_spin_lock_irqsave(&pctrl->lock, flags);
> +	mux = readl_relaxed(reg) & PAD_MUX;
> +	writel_relaxed(mux | value, reg);
> +	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int spacemit_pinconf_set(struct pinctrl_dev *pctldev,
> +				unsigned int pin, unsigned long *configs,
> +				unsigned int num_configs)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> +	u32 value;
> +
> +	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
> +		return -EINVAL;
> +
> +	return spacemit_pin_set_config(pctrl, pin, value);
> +}
> +
> +static int spacemit_pinconf_group_set(struct pinctrl_dev *pctldev,
> +				      unsigned int gsel,
> +				      unsigned long *configs,
> +				      unsigned int num_configs)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct spacemit_pin *spin;
> +	const struct group_desc *group;
> +	u32 value;
> +	int i;
> +
> +	group = pinctrl_generic_get_group(pctldev, gsel);
> +	if (!group)
> +		return -EINVAL;
> +
> +	spin = spacemit_get_pin(pctrl, group->grp.pins[0]);
> +	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
> +		return -EINVAL;
> +
> +	for (i = 0; i < group->grp.npins; i++)
> +		spacemit_pin_set_config(pctrl, group->grp.pins[i], value);
> +
> +	return 0;
> +}
> +

> +static void spacemit_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> +				      struct seq_file *seq, unsigned int pin)
> +{
> +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> +	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
> +	void __iomem *reg;
> +
> +	reg = spacemit_pin_to_reg(pctrl, pin);
> +	seq_printf(seq, " reg (0x%x), io (%d)\n", readl(reg), type);
> +}
> +

I think this is a duplicate of "spacemit_pctrl_dbg_show".

> +static const struct pinconf_ops spacemit_pinconf_ops = {
> +	.pin_config_get			= spacemit_pinconf_get,
> +	.pin_config_set			= spacemit_pinconf_set,
> +	.pin_config_group_set		= spacemit_pinconf_group_set,
> +	.pin_config_dbg_show		= spacemit_pinconf_dbg_show,
> +	.is_generic			= true,
> +};
> +
> +static int spacemit_pinctrl_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device *dev = &pdev->dev;
> +	struct spacemit_pinctrl *pctrl;
> +	const struct spacemit_pinctrl_data *pctrl_data;
> +	int ret;
> +
> +	pctrl_data = device_get_match_data(dev);
> +	if (!pctrl_data)
> +		return -ENODEV;
> +
> +	if (pctrl_data->npins == 0)
> +		return dev_err_probe(dev, -EINVAL, "invalid pin data\n");
> +
> +	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
> +	if (!pctrl)
> +		return -ENOMEM;
> +
> +	pctrl->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pctrl->regs))
> +		return PTR_ERR(pctrl->regs);
> +
> +	pctrl->pdesc.name = dev_name(dev);
> +	pctrl->pdesc.pins = pctrl_data->pins;
> +	pctrl->pdesc.npins = pctrl_data->npins;
> +	pctrl->pdesc.pctlops = &spacemit_pctrl_ops;
> +	pctrl->pdesc.pmxops = &spacemit_pmx_ops;
> +	pctrl->pdesc.confops = &spacemit_pinconf_ops;
> +	pctrl->pdesc.num_custom_params = ARRAY_SIZE(spacemit_pinconf_custom_params);
> +	pctrl->pdesc.custom_params = spacemit_pinconf_custom_params;
> +	pctrl->pdesc.owner = THIS_MODULE;
> +
> +	pctrl->data = pctrl_data;
> +	pctrl->dev = dev;
> +	raw_spin_lock_init(&pctrl->lock);
> +	INIT_LIST_HEAD(&pctrl->gpiofuncs);
> +	mutex_init(&pctrl->mutex);
> +
> +	platform_set_drvdata(pdev, pctrl);
> +
> +	ret = devm_pinctrl_register_and_init(dev, &pctrl->pdesc,
> +					     pctrl, &pctrl->pctl_dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "fail to register pinctrl driver\n");
> +	ret = spacemit_add_gpio_func(np, pctrl);
> +	if (ret < 0)
> +		return dev_err_probe(dev, ret,
> +				     "fail to get gpio function\n");
> +
> +	return pinctrl_enable(pctrl->pctl_dev);
> +}
> +
> +static const struct pinctrl_pin_desc k1_pin_desc[] = {
> +	PINCTRL_PIN(GPIO_00, "GPIO_00"),
> +	PINCTRL_PIN(GPIO_01, "GPIO_01"),
> +	PINCTRL_PIN(GPIO_02, "GPIO_02"),
> +	PINCTRL_PIN(GPIO_03, "GPIO_03"),
> +	PINCTRL_PIN(GPIO_04, "GPIO_04"),
> +	PINCTRL_PIN(GPIO_05, "GPIO_05"),
> +	PINCTRL_PIN(GPIO_06, "GPIO_06"),
> +	PINCTRL_PIN(GPIO_07, "GPIO_07"),
> +	PINCTRL_PIN(GPIO_08, "GPIO_08"),
> +	PINCTRL_PIN(GPIO_09, "GPIO_09"),
> +	PINCTRL_PIN(GPIO_10, "GPIO_10"),
> +	PINCTRL_PIN(GPIO_11, "GPIO_11"),
> +	PINCTRL_PIN(GPIO_12, "GPIO_12"),
> +	PINCTRL_PIN(GPIO_13, "GPIO_13"),
> +	PINCTRL_PIN(GPIO_14, "GPIO_14"),
> +	PINCTRL_PIN(GPIO_15, "GPIO_15"),
> +	PINCTRL_PIN(GPIO_16, "GPIO_16"),
> +	PINCTRL_PIN(GPIO_17, "GPIO_17"),
> +	PINCTRL_PIN(GPIO_18, "GPIO_18"),
> +	PINCTRL_PIN(GPIO_19, "GPIO_19"),
> +	PINCTRL_PIN(GPIO_20, "GPIO_20"),
> +	PINCTRL_PIN(GPIO_21, "GPIO_21"),
> +	PINCTRL_PIN(GPIO_22, "GPIO_22"),
> +	PINCTRL_PIN(GPIO_23, "GPIO_23"),
> +	PINCTRL_PIN(GPIO_24, "GPIO_24"),
> +	PINCTRL_PIN(GPIO_25, "GPIO_25"),
> +	PINCTRL_PIN(GPIO_26, "GPIO_26"),
> +	PINCTRL_PIN(GPIO_27, "GPIO_27"),
> +	PINCTRL_PIN(GPIO_28, "GPIO_28"),
> +	PINCTRL_PIN(GPIO_29, "GPIO_29"),
> +	PINCTRL_PIN(GPIO_30, "GPIO_30"),
> +	PINCTRL_PIN(GPIO_31, "GPIO_31"),
> +	PINCTRL_PIN(GPIO_32, "GPIO_32"),
> +	PINCTRL_PIN(GPIO_33, "GPIO_33"),
> +	PINCTRL_PIN(GPIO_34, "GPIO_34"),
> +	PINCTRL_PIN(GPIO_35, "GPIO_35"),
> +	PINCTRL_PIN(GPIO_36, "GPIO_36"),
> +	PINCTRL_PIN(GPIO_37, "GPIO_37"),
> +	PINCTRL_PIN(GPIO_38, "GPIO_38"),
> +	PINCTRL_PIN(GPIO_39, "GPIO_39"),
> +	PINCTRL_PIN(GPIO_40, "GPIO_40"),
> +	PINCTRL_PIN(GPIO_41, "GPIO_41"),
> +	PINCTRL_PIN(GPIO_42, "GPIO_42"),
> +	PINCTRL_PIN(GPIO_43, "GPIO_43"),
> +	PINCTRL_PIN(GPIO_44, "GPIO_44"),
> +	PINCTRL_PIN(GPIO_45, "GPIO_45"),
> +	PINCTRL_PIN(GPIO_46, "GPIO_46"),
> +	PINCTRL_PIN(GPIO_47, "GPIO_47"),
> +	PINCTRL_PIN(GPIO_48, "GPIO_48"),
> +	PINCTRL_PIN(GPIO_49, "GPIO_49"),
> +	PINCTRL_PIN(GPIO_50, "GPIO_50"),
> +	PINCTRL_PIN(GPIO_51, "GPIO_51"),
> +	PINCTRL_PIN(GPIO_52, "GPIO_52"),
> +	PINCTRL_PIN(GPIO_53, "GPIO_53"),
> +	PINCTRL_PIN(GPIO_54, "GPIO_54"),
> +	PINCTRL_PIN(GPIO_55, "GPIO_55"),
> +	PINCTRL_PIN(GPIO_56, "GPIO_56"),
> +	PINCTRL_PIN(GPIO_57, "GPIO_57"),
> +	PINCTRL_PIN(GPIO_58, "GPIO_58"),
> +	PINCTRL_PIN(GPIO_59, "GPIO_59"),
> +	PINCTRL_PIN(GPIO_60, "GPIO_60"),
> +	PINCTRL_PIN(GPIO_61, "GPIO_61"),
> +	PINCTRL_PIN(GPIO_62, "GPIO_62"),
> +	PINCTRL_PIN(GPIO_63, "GPIO_63"),
> +	PINCTRL_PIN(GPIO_64, "GPIO_64"),
> +	PINCTRL_PIN(GPIO_65, "GPIO_65"),
> +	PINCTRL_PIN(GPIO_66, "GPIO_66"),
> +	PINCTRL_PIN(GPIO_67, "GPIO_67"),
> +	PINCTRL_PIN(GPIO_68, "GPIO_68"),
> +	PINCTRL_PIN(GPIO_69, "GPIO_69"),
> +	PINCTRL_PIN(GPIO_70, "GPIO_70/PRI_DTI"),
> +	PINCTRL_PIN(GPIO_71, "GPIO_71/PRI_TMS"),
> +	PINCTRL_PIN(GPIO_72, "GPIO_72/PRI_TCK"),
> +	PINCTRL_PIN(GPIO_73, "GPIO_73/PRI_TDO"),
> +	PINCTRL_PIN(GPIO_74, "GPIO_74"),
> +	PINCTRL_PIN(GPIO_75, "GPIO_75"),
> +	PINCTRL_PIN(GPIO_76, "GPIO_76"),
> +	PINCTRL_PIN(GPIO_77, "GPIO_77"),
> +	PINCTRL_PIN(GPIO_78, "GPIO_78"),
> +	PINCTRL_PIN(GPIO_79, "GPIO_79"),
> +	PINCTRL_PIN(GPIO_80, "GPIO_80"),
> +	PINCTRL_PIN(GPIO_81, "GPIO_81"),
> +	PINCTRL_PIN(GPIO_82, "GPIO_82"),
> +	PINCTRL_PIN(GPIO_83, "GPIO_83"),
> +	PINCTRL_PIN(GPIO_84, "GPIO_84"),
> +	PINCTRL_PIN(GPIO_85, "GPIO_85"),
> +	PINCTRL_PIN(GPIO_86, "GPIO_86"),
> +	PINCTRL_PIN(GPIO_87, "GPIO_87"),
> +	PINCTRL_PIN(GPIO_88, "GPIO_88"),
> +	PINCTRL_PIN(GPIO_89, "GPIO_89"),
> +	PINCTRL_PIN(GPIO_90, "GPIO_90"),
> +	PINCTRL_PIN(GPIO_91, "GPIO_91"),
> +	PINCTRL_PIN(GPIO_92, "GPIO_92"),
> +	PINCTRL_PIN(GPIO_93, "GPIO_93/PWR_SCL"),
> +	PINCTRL_PIN(GPIO_94, "GPIO_94/PWR_SDA"),
> +	PINCTRL_PIN(GPIO_95, "GPIO_95/VCX0_EN"),
> +	PINCTRL_PIN(GPIO_96, "GPIO_96/DVL0"),
> +	PINCTRL_PIN(GPIO_97, "GPIO_97/DVL1"),
> +	PINCTRL_PIN(GPIO_98,  "GPIO_98/QSPI_DAT3"),
> +	PINCTRL_PIN(GPIO_99,  "GPIO_99/QSPI_DAT2"),
> +	PINCTRL_PIN(GPIO_100, "GPIO_100/QSPI_DAT1"),
> +	PINCTRL_PIN(GPIO_101, "GPIO_101/QSPI_DAT0"),
> +	PINCTRL_PIN(GPIO_102, "GPIO_102/QSPI_CLK"),
> +	PINCTRL_PIN(GPIO_103, "GPIO_103/QSPI_CSI"),
> +	PINCTRL_PIN(GPIO_104, "GPIO_104/MMC1_DAT3"),
> +	PINCTRL_PIN(GPIO_105, "GPIO_105/MMC1_DAT2"),
> +	PINCTRL_PIN(GPIO_106, "GPIO_106/MMC1_DAT1"),
> +	PINCTRL_PIN(GPIO_107, "GPIO_107/MMC1_DAT0"),
> +	PINCTRL_PIN(GPIO_108, "GPIO_108/MMC1_CMD"),
> +	PINCTRL_PIN(GPIO_109, "GPIO_109/MMC1_CLK"),
> +	PINCTRL_PIN(GPIO_110, "GPIO_110"),
> +	PINCTRL_PIN(GPIO_111, "GPIO_111"),
> +	PINCTRL_PIN(GPIO_112, "GPIO_112"),
> +	PINCTRL_PIN(GPIO_113, "GPIO_113"),
> +	PINCTRL_PIN(GPIO_114, "GPIO_114"),
> +	PINCTRL_PIN(GPIO_115, "GPIO_115"),
> +	PINCTRL_PIN(GPIO_116, "GPIO_116"),
> +	PINCTRL_PIN(GPIO_117, "GPIO_117"),
> +	PINCTRL_PIN(GPIO_118, "GPIO_118"),
> +	PINCTRL_PIN(GPIO_119, "GPIO_119"),
> +	PINCTRL_PIN(GPIO_120, "GPIO_120"),
> +	PINCTRL_PIN(GPIO_121, "GPIO_121"),
> +	PINCTRL_PIN(GPIO_122, "GPIO_122"),
> +	PINCTRL_PIN(GPIO_123, "GPIO_123"),
> +	PINCTRL_PIN(GPIO_124, "GPIO_124"),
> +	PINCTRL_PIN(GPIO_125, "GPIO_125"),
> +	PINCTRL_PIN(GPIO_126, "GPIO_126"),
> +	PINCTRL_PIN(GPIO_127, "GPIO_127"),
> +};
> +
> +static const struct spacemit_pin k1_pin_data[ARRAY_SIZE(k1_pin_desc)] = {
> +	K1_FUNC_PIN(GPIO_00, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_01, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_02, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_03, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_04, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_05, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_06, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_07, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_08, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_09, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_10, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_11, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_12, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_13, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_14, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_15, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_16, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_17, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_18, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_19, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_20, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_21, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_22, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_23, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_24, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_25, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_26, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_27, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_28, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_29, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_30, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_31, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_32, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_33, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_34, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_35, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_36, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_37, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_38, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_39, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_40, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_41, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_42, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_43, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_44, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_45, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_46, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_47, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_48, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_49, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_50, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_51, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_52, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_53, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_54, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_55, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_56, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_57, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_58, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_59, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_60, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_61, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_62, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_63, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_64, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_65, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_66, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_67, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_68, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_69, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_70, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_71, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_72, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_73, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_74, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_75, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_76, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_77, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_78, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_79, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_80, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_81, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_82, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_83, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_84, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_85, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_86, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_87, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_88, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_89, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_90, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_91, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_92, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_93, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_94, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_95, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_96, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_97, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_98, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_99, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_100, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_101, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_102, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_103, IO_TYPE_EXTERNAL),
> +	K1_FUNC_PIN(GPIO_104, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_105, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_106, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_107, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_108, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_109, IO_TYPE_3V3),
> +	K1_FUNC_PIN(GPIO_110, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_111, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_112, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_113, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_114, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_115, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_116, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_117, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_118, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_119, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_120, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_121, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_122, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_123, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_124, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_125, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_126, IO_TYPE_1V8),
> +	K1_FUNC_PIN(GPIO_127, IO_TYPE_1V8),
> +};
> +
> +static const struct spacemit_pinctrl_data k1_pinctrl_data = {
> +	.pins = k1_pin_desc,
> +	.data = k1_pin_data,
> +	.npins = ARRAY_SIZE(k1_pin_desc),
> +};
> +
> +static const struct of_device_id k1_pinctrl_ids[] = {
> +	{ .compatible = "spacemit,k1-pinctrl", .data = &k1_pinctrl_data },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, k1_pinctrl_ids);
> +
> +static struct platform_driver k1_pinctrl_driver = {
> +	.probe	= spacemit_pinctrl_probe,
> +	.driver	= {
> +		.name			= "k1-pinctrl",
> +		.suppress_bind_attrs	= true,
> +		.of_match_table		= k1_pinctrl_ids,
> +	},
> +};
> +module_platform_driver(k1_pinctrl_driver);
> +
> +MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
> +MODULE_DESCRIPTION("Pinctrl driver for the SpacemiT K1 SoC");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.h b/drivers/pinctrl/spacemit/pinctrl-k1.h
> new file mode 100644
> index 0000000000000..9d6589b626e9d
> --- /dev/null
> +++ b/drivers/pinctrl/spacemit/pinctrl-k1.h
> @@ -0,0 +1,36 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
> +
> +#ifndef _PINCTRL_SPACEMIT_K1_H
> +#define _PINCTRL_SPACEMIT_K1_H
> +
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/mutex.h>
> +#include <linux/spinlock.h>
> +#include <linux/platform_device.h>
> +#include <linux/pinctrl/pinctrl.h>
> +#include <linux/pinctrl/pinconf.h>
> +
> +enum spacemit_pin_io_type {
> +	IO_TYPE_NONE = 0,
> +	IO_TYPE_1V8,
> +	IO_TYPE_3V3,
> +	IO_TYPE_EXTERNAL,
> +};
> +
> +#define K1_PIN_IO_TYPE		GENMASK(2, 1)
> +
> +#define K1_PIN_CAP_IO_TYPE(type)		\
> +	FIELD_PREP_CONST(K1_PIN_IO_TYPE, type)
> +#define K1_PIN_GET_IO_TYPE(val)			\
> +	FIELD_GET(K1_PIN_IO_TYPE, val)
> +
> +#define K1_FUNC_PIN(_id, _io)				\
> +	{							\
> +		.pin = (_id),					\
> +		.flags = (K1_PIN_CAP_IO_TYPE(_io))		\
> +	}
> +
> +#endif /* _PINCTRL_SPACEMIT_K1_H */
> 
> -- 
> 2.45.2
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
       [not found]   ` <66cbf3bb.050a0220.2632ed.b191SMTPIN_ADDED_BROKEN@mx.google.com>
@ 2024-08-26 16:22     ` Conor Dooley
  2024-08-27  2:23       ` Yixun Lan
       [not found]       ` <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com>
  0 siblings, 2 replies; 19+ messages in thread
From: Conor Dooley @ 2024-08-26 16:22 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Yangyu Chen,
	Jesse Taube, Jisheng Zhang, Inochi Amaoto, Icenowy Zheng,
	Meng Zhang, Meng Zhang, devicetree, linux-riscv, linux-kernel,
	linux-gpio

[-- Attachment #1: Type: text/plain, Size: 2124 bytes --]

On Mon, Aug 26, 2024 at 03:09:39AM +0000, Yixun Lan wrote:
> 
> On 13:10 Sun 25 Aug     , Yixun Lan wrote:
> > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> > 
> > Two vendor specific properties are introduced here, As the pinctrl
> > has dedicated slew rate enable control - bit[7], so we have
> > spacemit,slew-rate-{enable,disable} for this. For the same reason,
> > creating spacemit,strong-pull-up for the strong pull up control.
> > 
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>

I got this mail, and one of your other ones, 5 times. What's going wrong
with your mail setup? 

| 250   T Aug 25 Yixun Lan       (6.0K) ┌─>[PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3
| 251 N T Aug 26 Inochi Amaoto   ( 12K) │ ┌─>
| 252   T Aug 25 Yixun Lan       (6.8K) ├─>[PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
| 253 N T Aug 26 Inochi Amaoto   ( 46K) │ ┌─>
| 254   T Aug 25 Yixun Lan       ( 38K) ├─>[PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
| 255 N T Aug 26 Inochi Amaoto   ( 22K) │ ┌─>
| 256   T Aug 26 Yixun Lan       ( 333) │ ├─>
| 257   T Aug 26 Yixun Lan       ( 338) │ ├─>
| 258   T Aug 26 Yixun Lan       ( 334) │ ├─>
| 259   T Aug 26 Yixun Lan       ( 334) │ ├─>
| 260   T Aug 26 Yixun Lan       ( 333) │ ├─>
| 261   C Aug 25 Rob Herring (Ar (9.0K) │ ├─>
| 262 N C Aug 26 Krzysztof Kozlo ( 14K) │ │ ┌─>
| 263 N C Aug 26 Inochi Amaoto   ( 19K) │ │ ├─>
| 264   C Aug 26 Yixun Lan       ( 285) │ │ ├─>
| 265   C Aug 26 Yixun Lan       ( 281) │ │ ├─>
| 266 N C Aug 26 Yixun Lan       ( 14K) │ │ ├─>
| 267 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
| 268 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
| 269   T Aug 25 Krzysztof Kozlo ( 13K) │ ├─>
| 270   T Aug 25 Yixun Lan       ( 14K) ├─>[PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
| 271   T Aug 25 Yixun Lan       (8.5K) [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
  2024-08-26 16:22     ` Conor Dooley
@ 2024-08-27  2:23       ` Yixun Lan
       [not found]       ` <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com>
  1 sibling, 0 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-27  2:23 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Yangyu Chen, Jesse Taube,
	Jisheng Zhang, Inochi Amaoto, Icenowy Zheng, Meng Zhang,
	Meng Zhang, devicetree, linux-riscv, linux-kernel, linux-gpio

Hi Conor:

On 17:22 Mon 26 Aug     , Conor Dooley wrote:
> On Mon, Aug 26, 2024 at 03:09:39AM +0000, Yixun Lan wrote:
> > 
> > On 13:10 Sun 25 Aug     , Yixun Lan wrote:
> > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> > > 
> > > Two vendor specific properties are introduced here, As the pinctrl
> > > has dedicated slew rate enable control - bit[7], so we have
> > > spacemit,slew-rate-{enable,disable} for this. For the same reason,
> > > creating spacemit,strong-pull-up for the strong pull up control.
> > > 
> > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> 
> I got this mail, and one of your other ones, 5 times. What's going wrong
> with your mail setup? 
> 
Oops, sorry for this, it's the second time you complain this..
TBO, I have no idea what's happened, asked Yangyu, he didn't have this problem

I'm using mutt+msmtp to reply, while using b4 to send the patch series,
for all the mails you received, do they all have same message-id?
I have a local filter for duplicated mails myself, could this help?

(I leave only one address of your mail in this reply, see if still have problem?)

> | 250   T Aug 25 Yixun Lan       (6.0K) ┌─>[PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3
> | 251 N T Aug 26 Inochi Amaoto   ( 12K) │ ┌─>
> | 252   T Aug 25 Yixun Lan       (6.8K) ├─>[PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
> | 253 N T Aug 26 Inochi Amaoto   ( 46K) │ ┌─>
> | 254   T Aug 25 Yixun Lan       ( 38K) ├─>[PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
> | 255 N T Aug 26 Inochi Amaoto   ( 22K) │ ┌─>
> | 256   T Aug 26 Yixun Lan       ( 333) │ ├─>
> | 257   T Aug 26 Yixun Lan       ( 338) │ ├─>
> | 258   T Aug 26 Yixun Lan       ( 334) │ ├─>
> | 259   T Aug 26 Yixun Lan       ( 334) │ ├─>
> | 260   T Aug 26 Yixun Lan       ( 333) │ ├─>
> | 261   C Aug 25 Rob Herring (Ar (9.0K) │ ├─>
> | 262 N C Aug 26 Krzysztof Kozlo ( 14K) │ │ ┌─>
> | 263 N C Aug 26 Inochi Amaoto   ( 19K) │ │ ├─>
> | 264   C Aug 26 Yixun Lan       ( 285) │ │ ├─>
> | 265   C Aug 26 Yixun Lan       ( 281) │ │ ├─>
> | 266 N C Aug 26 Yixun Lan       ( 14K) │ │ ├─>
> | 267 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
> | 268 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
> | 269   T Aug 25 Krzysztof Kozlo ( 13K) │ ├─>
> | 270   T Aug 25 Yixun Lan       ( 14K) ├─>[PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
> | 271   T Aug 25 Yixun Lan       (8.5K) [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC
> 


-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
  2024-08-26  7:55   ` Inochi Amaoto
@ 2024-08-27  3:03     ` Yixun Lan
  0 siblings, 0 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-27  3:03 UTC (permalink / raw)
  To: Inochi Amaoto
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
	Yangyu Chen, Jesse Taube, Jisheng Zhang, Icenowy Zheng,
	Meng Zhang, Meng Zhang, devicetree, linux-riscv, linux-kernel,
	linux-gpio

Hi Inochi:

On 15:55 Mon 26 Aug     , Inochi Amaoto wrote:
> On Sun, Aug 25, 2024 at 01:10:04PM GMT, Yixun Lan wrote:
> > Add pinctrl device tree data to SpacemiT's K1 SoC.
> > 
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > ---
> > Note, only minimal device tree data added in this series,
> > which just try to demonstrate this pinctrl driver, but
> > more dt data can be added later, in separate patches.
> > ---
> >  arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 19 +++++++++++++++++++
> >  arch/riscv/boot/dts/spacemit/k1.dtsi         |  5 +++++
> >  2 files changed, 24 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > new file mode 100644
> > index 0000000000000..38ccaad1209f5
> > --- /dev/null
> > +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> > @@ -0,0 +1,19 @@
> > +// SPDX-License-Identifier: GPL-2.0 OR MIT
> > +/*
> > + * Copyright (c) 2024 Yixun Lan <dlan@gentoo.org>
> > + */
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> > +
> 
> 
> > +&pinctrl {
> > +	uart0_2_cfg: uart0-2-cfg {
> > +		uart0-2-pins {
> > +			pinmux = <K1_PADCONF(GPIO_68, 2)>,
> > +				 <K1_PADCONF(GPIO_69, 2)>;
> > +
> > +			bias-pull-up;
> > +			drive-strength-microamp = <32>;
> > +		};
> > +	};
> > +};
> 
> No common file is needed at least for now. You can put it
> in the board dts. Also, squash this into the next patch as
> it is more related to the uart.
> 
given that there are many K1 SoC boards on the market, having
a separated pinctrl dts file makes more sense, this will maximize
data sharing.

the problem here that I haven't populated too many pinctrl
cfgs in this file, so it looks quite slim.. but the plan was to
put all pinctrl meta data here even if not used by particular board.

so, I would like to keep current struture unless I'm wrong here.

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
  2024-08-26  8:16   ` Inochi Amaoto
@ 2024-08-27  3:30     ` Yixun Lan
  0 siblings, 0 replies; 19+ messages in thread
From: Yixun Lan @ 2024-08-27  3:30 UTC (permalink / raw)
  To: Inochi Amaoto
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Paul Walmsley, Palmer Dabbelt, Albert Ou, Conor Dooley,
	Yangyu Chen, Jesse Taube, Jisheng Zhang, Icenowy Zheng,
	Meng Zhang, Meng Zhang, devicetree, linux-riscv, linux-kernel,
	linux-gpio

Hi Inochi:

On 16:16 Mon 26 Aug     , Inochi Amaoto wrote:
> On Sun, Aug 25, 2024 at 01:10:03PM GMT, Yixun Lan wrote:
> > SpacemiT's K1 SoC has a pinctrl controller which use single register
> > to describe all functions, which include bias pull up/down, drive
> > strength, schmitter trigger, slew rate, strong pull up, mux mode.
> > 
> > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > ---
> >  drivers/pinctrl/Kconfig               |    1 +
> >  drivers/pinctrl/Makefile              |    1 +
> >  drivers/pinctrl/spacemit/Kconfig      |   17 +
> >  drivers/pinctrl/spacemit/Makefile     |    3 +
> >  drivers/pinctrl/spacemit/pinctrl-k1.c | 1012 +++++++++++++++++++++++++++++++++
> >  drivers/pinctrl/spacemit/pinctrl-k1.h |   36 ++
> >  6 files changed, 1070 insertions(+)
> > 
> > diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
> > index 7e4f93a3bc7ac..8358da47cd00d 100644
> > --- a/drivers/pinctrl/Kconfig
> > +++ b/drivers/pinctrl/Kconfig
> > @@ -583,6 +583,7 @@ source "drivers/pinctrl/qcom/Kconfig"
> >  source "drivers/pinctrl/realtek/Kconfig"
> >  source "drivers/pinctrl/renesas/Kconfig"
> >  source "drivers/pinctrl/samsung/Kconfig"
> > +source "drivers/pinctrl/spacemit/Kconfig"
> >  source "drivers/pinctrl/spear/Kconfig"
> >  source "drivers/pinctrl/sprd/Kconfig"
> >  source "drivers/pinctrl/starfive/Kconfig"
> > diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
> > index cc809669405ab..553beead7ffa0 100644
> > --- a/drivers/pinctrl/Makefile
> > +++ b/drivers/pinctrl/Makefile
> > @@ -73,6 +73,7 @@ obj-y				+= qcom/
> >  obj-$(CONFIG_ARCH_REALTEK)      += realtek/
> >  obj-$(CONFIG_PINCTRL_RENESAS)	+= renesas/
> >  obj-$(CONFIG_PINCTRL_SAMSUNG)	+= samsung/
> > +obj-y				+= spacemit/
> >  obj-$(CONFIG_PINCTRL_SPEAR)	+= spear/
> >  obj-y				+= sprd/
> >  obj-$(CONFIG_SOC_STARFIVE)	+= starfive/
> > diff --git a/drivers/pinctrl/spacemit/Kconfig b/drivers/pinctrl/spacemit/Kconfig
> > new file mode 100644
> > index 0000000000000..168f8a5ffbb95
> > --- /dev/null
> > +++ b/drivers/pinctrl/spacemit/Kconfig
> > @@ -0,0 +1,17 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +#
> > +# Sophgo SoC PINCTRL drivers
> > +#
> > +
> > +config PINCTRL_SPACEMIT_K1
> > +	tristate "SpacemiT K1 SoC Pinctrl driver"
> > +	depends on ARCH_SPACEMIT || COMPILE_TEST
> > +	depends on OF
> > +	select GENERIC_PINCTRL_GROUPS
> > +	select GENERIC_PINMUX_FUNCTIONS
> > +	select GENERIC_PINCONF
> > +	help
> > +	  Say Y to select the pinctrl driver for K1 SoC.
> > +	  This pin controller allows selecting the mux function for
> > +	  each pin. This driver can also be built as a module called
> > +	  pinctrl-k1.
> > diff --git a/drivers/pinctrl/spacemit/Makefile b/drivers/pinctrl/spacemit/Makefile
> > new file mode 100644
> > index 0000000000000..fede1e80fe0ff
> > --- /dev/null
> > +++ b/drivers/pinctrl/spacemit/Makefile
> > @@ -0,0 +1,3 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +
> > +obj-$(CONFIG_PINCTRL_SPACEMIT_K1)	+= pinctrl-k1.o
> > diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.c b/drivers/pinctrl/spacemit/pinctrl-k1.c
> > new file mode 100644
> > index 0000000000000..9ed9530c78150
> > --- /dev/null
> > +++ b/drivers/pinctrl/spacemit/pinctrl-k1.c
> > @@ -0,0 +1,1012 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/export.h>
> > +#include <linux/io.h>
> > +#include <linux/of.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/module.h>
> > +
> > +#include <linux/pinctrl/consumer.h>
> > +#include <linux/pinctrl/machine.h>
> > +#include <linux/pinctrl/pinconf-generic.h>
> > +#include <linux/pinctrl/pinconf.h>
> > +#include <linux/pinctrl/pinctrl.h>
> > +#include <linux/pinctrl/pinmux.h>
> > +
> > +#include <dt-bindings/pinctrl/spacemit,k1-pinctrl.h>
> > +
> > +#include "../core.h"
> > +#include "../pinctrl-utils.h"
> > +#include "../pinconf.h"
> > +#include "../pinmux.h"
> > +#include "pinctrl-k1.h"
> > +
> > +/*
> > + * +---------+----------+-----------+--------+--------+----------+--------+
> > + * |   pull  |   drive  | schmitter |  slew  |  edge  |  strong  |   mux  |
> > + * | up/down | strength |  trigger  |  rate  | detect |   pull   |  mode  |
> > + * +---------+----------+-----------+--------+--------+----------+--------+
> > + *   3 bits     3 bits     2 bits     1 bit    3 bits     1 bit    3 bits
> > + */
> > +
> > +#define PAD_MUX			GENMASK(2, 0)
> > +#define PAD_STRONG_PULL		BIT(3)
> > +#define PAD_EDGE_RISE		BIT(4)
> > +#define PAD_EDGE_FALL		BIT(5)
> > +#define PAD_EDGE_CLEAR		BIT(6)
> > +#define PAD_SLEW_RATE		GENMASK(12, 11)
> > +#define PAD_SLEW_RATE_EN	BIT(7)
> > +#define PAD_SCHMITT		GENMASK(9, 8)
> > +#define PAD_DRIVE		GENMASK(12, 10)
> > +#define PAD_PULLDOWN		BIT(13)
> > +#define PAD_PULLUP		BIT(14)
> > +#define PAD_PULL_EN		BIT(15)
> > +
> > +#define spacemit_pin_to_reg(pctrl, pin)		((pctrl)->regs + (pin << 2))
> > +
> > +struct spacemit_pin {
> > +	u16				pin;
> > +	u16				flags;
> > +};
> > +
> > +struct spacemit_pinctrl {
> > +	struct device				*dev;
> > +	struct pinctrl_dev			*pctl_dev;
> > +	const struct spacemit_pinctrl_data	*data;
> > +	struct pinctrl_desc			pdesc;
> > +
> > +	struct list_head			gpiofuncs;
> > +
> > +	struct mutex				mutex;
> > +	raw_spinlock_t				lock;
> > +
> > +	void __iomem				*regs;
> > +};
> > +
> > +struct spacemit_pinctrl_data {
> > +	const struct pinctrl_pin_desc   *pins;
> > +	const struct spacemit_pin	*data;
> > +	u16				npins;
> > +};
> > +
> > +struct spacemit_gpiofunc_range {
> > +	u32		offset;
> > +	u32		npins;
> > +	u32		gpiofunc;
> > +	struct		list_head node;
> > +};
> > +
> > +struct spacemit_pin_mux_config {
> > +	const struct spacemit_pin	*pin;
> > +	u32		config;
> > +};
> > +
> > +struct spacemit_pin_drv_strength {
> > +	u8		val;
> > +	u32		mA;
> > +};
> > +
> > +static unsigned int spacemit_dt_get_pin(u32 value)
> > +{
> > +	return (value >> 16) & GENMASK(15, 0);
> > +}
> > +
> > +static unsigned int spacemit_dt_get_pin_mux(u32 value)
> > +{
> > +	return value & GENMASK(15, 0);
> > +}
> > +
> > +static const struct spacemit_pin *spacemit_get_pin(struct spacemit_pinctrl *pctrl,
> > +						   unsigned long pin)
> > +{
> > +	const struct spacemit_pin *pdata = pctrl->data->data;
> > +	int i;
> > +
> > +	for (i = 0; i < pctrl->data->npins; i++) {
> > +		if (pin == pdata[i].pin)
> > +			return &pdata[i];
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static inline enum spacemit_pin_io_type spacemit_to_pin_io_type(
> > +	const struct spacemit_pin *pin)
> > +{
> > +	return K1_PIN_GET_IO_TYPE(pin->flags);
> > +}
> > +
> > +/* External: IO voltage via external source, can be 1.8V or 3.3V */
> > +static const char * const io_type_desc[] = {
> > +	"None",
> > +	"Fixed/1V8",
> > +	"Fixed/3V3",
> > +	"External",
> > +};
> > +
> > +static void spacemit_pctrl_dbg_show(struct pinctrl_dev *pctldev,
> > +				    struct seq_file *seq, unsigned int pin)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> > +	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
> > +	void __iomem *reg;
> > +	u32 value;
> > +
> > +	seq_printf(seq, "pin: %u ", spin->pin);
> > +
> > +	seq_printf(seq, "type: %s ", io_type_desc[type]);
> > +
> > +	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> > +	value = readl(reg);
> > +	seq_printf(seq, "mux: 0x%08x ", value);
> > +}
> > +
> 
> > +/* use IO high level output current as the table */
> > +static struct spacemit_pin_drv_strength spacemit_ds_1v8_tbl[4] = {
> > +	{ (0 << 1), 11 },
> > +	{ (1 << 1), 21 },
> > +	{ (2 << 1), 32 },
> > +	{ (3 << 1), 42 },
> > +};
> > +
> > +static struct spacemit_pin_drv_strength spacemit_ds_3v3_tbl[8] = {
> > +	{ 0,  7 },
> > +	{ 2, 10 },
> > +	{ 4, 13 },
> > +	{ 6, 16 },
> > +	{ 1, 19 },
> > +	{ 3, 23 },
> > +	{ 5, 26 },
> > +	{ 7, 29 },
> > +};
> 
> Why these two use different format?
> 
I can adjust it, but personally no preference here, it's not much different

the underlying logic is for 1v8 table it's using bit[12:11], while 3v3 table 
using bit[12:10], with MASK bit[12:10], there is one bit shift for 1v8

> > +
> > +static inline u8 spacemit_get_ds_value(struct spacemit_pin_drv_strength *tbl,
> > +				       u32 num, u32 mA)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < num; i++)
> > +		if (mA <= tbl[i].mA)
> > +			return tbl[i].val;
> > +
> > +	return tbl[num - 1].val;
> > +}
> > +
> > +static inline u8 spacemit_get_driver_strength(enum spacemit_pin_io_type type,
> > +					      u32 mA)
> > +{
> > +	switch (type) {
> > +	case IO_TYPE_1V8:
> > +		return spacemit_get_ds_value(spacemit_ds_1v8_tbl,
> > +					     ARRAY_SIZE(spacemit_ds_1v8_tbl),
> > +					     mA);
> > +	case IO_TYPE_3V3:
> > +		return spacemit_get_ds_value(spacemit_ds_3v3_tbl,
> > +					     ARRAY_SIZE(spacemit_ds_3v3_tbl),
> > +					     mA);
> > +	default:
> > +		return 0;
> > +	}
> > +}
> > +
> > +static int spacemit_pctrl_dt_node_to_map(struct pinctrl_dev *pctldev,
> > +					 struct device_node *np,
> > +					 struct pinctrl_map **maps,
> > +					 unsigned int *num_maps)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	struct device *dev = pctrl->dev;
> > +	struct device_node *child;
> > +	struct pinctrl_map *map;
> > +	const char **grpnames;
> > +	const char *grpname;
> > +	int ngroups = 0;
> > +	int nmaps = 0;
> > +	int ret;
> > +
> > +	for_each_available_child_of_node(np, child)
> > +		ngroups += 1;
> > +
> > +	grpnames = devm_kcalloc(dev, ngroups, sizeof(*grpnames), GFP_KERNEL);
> > +	if (!grpnames)
> > +		return -ENOMEM;
> > +
> > +	map = devm_kcalloc(dev, ngroups * 2, sizeof(*map), GFP_KERNEL);
> > +	if (!map)
> > +		return -ENOMEM;
> > +
> > +	ngroups = 0;
> > +	mutex_lock(&pctrl->mutex);
> > +	for_each_available_child_of_node(np, child) {
> > +		int npins = of_property_count_u32_elems(child, "pinmux");
> > +		unsigned int *pins;
> > +		struct spacemit_pin_mux_config *pinmuxs;
> > +		u32 config;
> > +		int i;
> > +
> > +		if (npins < 1) {
> > +			dev_err(dev, "invalid pinctrl group %pOFn.%pOFn\n",
> > +				np, child);
> > +			ret = -EINVAL;
> > +			goto dt_failed;
> > +		}
> > +
> > +		grpname = devm_kasprintf(dev, GFP_KERNEL, "%pOFn.%pOFn",
> > +					 np, child);
> > +		if (!grpname) {
> > +			ret = -ENOMEM;
> > +			goto dt_failed;
> > +		}
> > +
> > +		grpnames[ngroups++] = grpname;
> > +
> > +		pins = devm_kcalloc(dev, npins, sizeof(*pins), GFP_KERNEL);
> > +		if (!pins) {
> > +			ret = -ENOMEM;
> > +			goto dt_failed;
> > +		}
> > +
> > +		pinmuxs = devm_kcalloc(dev, npins, sizeof(*pinmuxs), GFP_KERNEL);
> > +		if (!pinmuxs) {
> > +			ret = -ENOMEM;
> > +			goto dt_failed;
> > +		}
> > +
> > +		for (i = 0; i < npins; i++) {
> > +			ret = of_property_read_u32_index(child, "pinmux",
> > +							 i, &config);
> > +
> > +			if (ret)
> > +				goto dt_failed;
> > +
> > +			pins[i] = spacemit_dt_get_pin(config);
> > +			pinmuxs[i].config = config;
> > +			pinmuxs[i].pin = spacemit_get_pin(pctrl, pins[i]);
> > +
> > +			if (!pinmuxs[i].pin) {
> > +				dev_err(dev, "failed to get pin %d\n", pins[i]);
> > +				ret = -ENODEV;
> > +				goto dt_failed;
> > +			}
> > +		}
> > +
> 
> It will be better to check the power-source before inserting a map.
> 

Ok, I will think about this, maybe a more generic sanity checking function

> > +		map[nmaps].type = PIN_MAP_TYPE_MUX_GROUP;
> > +		map[nmaps].data.mux.function = np->name;
> > +		map[nmaps].data.mux.group = grpname;
> > +		nmaps += 1;
> > +
> > +		ret = pinctrl_generic_add_group(pctldev, grpname,
> > +						pins, npins, pinmuxs);
> > +		if (ret < 0) {
> > +			dev_err(dev, "failed to add group %s: %d\n", grpname, ret);
> > +			goto dt_failed;
> > +		}
> > +
> > +		ret = pinconf_generic_parse_dt_config(child, pctldev,
> > +						      &map[nmaps].data.configs.configs,
> > +						      &map[nmaps].data.configs.num_configs);
> > +		if (ret) {
> > +			dev_err(dev, "failed to parse pin config of group %s: %d\n",
> > +				grpname, ret);
> > +			goto dt_failed;
> > +		}
> > +
> > +		if (map[nmaps].data.configs.num_configs == 0)
> > +			continue;
> > +
> > +		map[nmaps].type = PIN_MAP_TYPE_CONFIGS_GROUP;
> > +		map[nmaps].data.configs.group_or_pin = grpname;
> > +		nmaps += 1;
> > +	}
> > +
> > +	ret = pinmux_generic_add_function(pctldev, np->name,
> > +					  grpnames, ngroups, NULL);
> > +	if (ret < 0) {
> > +		dev_err(dev, "error adding function %s: %d\n", np->name, ret);
> > +		goto function_failed;
> > +	}
> > +
> > +	*maps = map;
> > +	*num_maps = nmaps;
> > +	mutex_unlock(&pctrl->mutex);
> > +
> > +	return 0;
> > +
> > +dt_failed:
> > +	of_node_put(child);
> > +function_failed:
> > +	pinctrl_utils_free_map(pctldev, map, nmaps);
> > +	mutex_unlock(&pctrl->mutex);
> > +	return ret;
> > +}
> > +
> > +static const struct pinctrl_ops spacemit_pctrl_ops = {
> > +	.get_groups_count	= pinctrl_generic_get_group_count,
> > +	.get_group_name		= pinctrl_generic_get_group_name,
> > +	.get_group_pins		= pinctrl_generic_get_group_pins,
> > +	.pin_dbg_show		= spacemit_pctrl_dbg_show,
> > +	.dt_node_to_map		= spacemit_pctrl_dt_node_to_map,
> > +	.dt_free_map		= pinctrl_utils_free_map,
> > +};
> > +
> > +static int spacemit_pmx_set_mux(struct pinctrl_dev *pctldev,
> > +			      unsigned int fsel, unsigned int gsel)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct group_desc *group;
> > +	const struct spacemit_pin_mux_config *configs;
> > +	unsigned int i, mux;
> > +	void __iomem *reg;
> > +
> > +	group = pinctrl_generic_get_group(pctldev, gsel);
> > +	if (!group)
> > +		return -EINVAL;
> > +
> > +	configs = group->data;
> > +
> > +	for (i = 0; i < group->grp.npins; i++) {
> > +		const struct spacemit_pin *spin = configs[i].pin;
> > +		u32 value = configs[i].config;
> > +		unsigned long flags;
> > +
> > +		reg = spacemit_pin_to_reg(pctrl, spin->pin);
> > +		mux = spacemit_dt_get_pin_mux(value);
> > +
> > +		raw_spin_lock_irqsave(&pctrl->lock, flags);
> > +		value = readl_relaxed(reg) & ~PAD_MUX;
> > +		writel_relaxed(mux | value, reg);
> > +		raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int spacemit_request_gpio(struct pinctrl_dev *pctldev,
> > +				 struct pinctrl_gpio_range *range,
> > +				 unsigned int pin)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	struct spacemit_gpiofunc_range *frange = NULL;
> > +	struct list_head *pos, *tmp;
> > +	void __iomem *reg;
> > +
> > +	list_for_each_safe(pos, tmp, &pctrl->gpiofuncs) {
> > +		frange = list_entry(pos, struct spacemit_gpiofunc_range, node);
> > +		if (pin >= frange->offset + frange->npins
> > +			|| pin < frange->offset)
> > +			continue;
> > +
> > +		reg = spacemit_pin_to_reg(pctrl, pin);
> > +		writel(frange->gpiofunc | PAD_EDGE_CLEAR, reg);
> > +
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct pinmux_ops spacemit_pmx_ops = {
> > +	.get_functions_count	= pinmux_generic_get_function_count,
> > +	.get_function_name	= pinmux_generic_get_function_name,
> > +	.get_function_groups	= pinmux_generic_get_function_groups,
> > +	.set_mux		= spacemit_pmx_set_mux,
> > +	.gpio_request_enable	= spacemit_request_gpio,
> > +};
> > +
> > +#define PIN_CONFIG_K1_SLEW_RATE_ENABLE	(PIN_CONFIG_END + 1)
> > +#define PIN_CONFIG_K1_STRONG_PULL_UP	(PIN_CONFIG_END + 2)
> > +
> > +static const struct pinconf_generic_params spacemit_pinconf_custom_params[] = {
> > +	{ "spacemit,slew-rate-disable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 0 },
> > +	{ "spacemit,slew-rate-enable",	PIN_CONFIG_K1_SLEW_RATE_ENABLE,	 1 },
> 
> > +	{ "spacemit,strong-pull-up",	PIN_CONFIG_K1_STRONG_PULL_UP,	 1 },
> 
> default enable?
> 
default value is 0 if this property not added, otherwise 1

but, as suggested by Krzysztof, will drop this property and fold it into bias-pull-up

> > +};
> > +
> > +static int spacemit_add_gpio_func(struct device_node *node, struct spacemit_pinctrl *pctrl)
> > +{
> > +	const char *propname = "spacemit,gpio-range";
> > +	const char *cellname = "#spacemit,gpio-range-cells";
> > +	struct of_phandle_args gpiospec;
> > +	struct spacemit_gpiofunc_range *range;
> > +	int ret, i;
> > +
> > +	for (i = 0; ; i++) {
> > +		ret = of_parse_phandle_with_args(node, propname, cellname,
> > +						 i, &gpiospec);
> > +		/* Do not treat it as error. Only treat it as end condition. */
> > +		if (ret) {
> > +			ret = 0;
> > +			break;
> > +		}
> > +		range = devm_kzalloc(pctrl->dev, sizeof(*range), GFP_KERNEL);
> > +		if (!range) {
> > +			ret = -ENOMEM;
> > +			break;
> > +		}
> > +		range->offset = gpiospec.args[0];
> > +		range->npins = gpiospec.args[1];
> > +		range->gpiofunc = gpiospec.args[2];
> > +		mutex_lock(&pctrl->mutex);
> > +		list_add_tail(&range->node, &pctrl->gpiofuncs);
> > +		mutex_unlock(&pctrl->mutex);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static int spacemit_pinconf_get(struct pinctrl_dev *pctldev,
> > +				unsigned int pin, unsigned long *config)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	int param = pinconf_to_config_param(*config);
> > +	u32 value, arg;
> > +
> > +	if (!pin)
> > +		return -EINVAL;
> > +
> > +	value = readl(spacemit_pin_to_reg(pctrl, pin));
> > +
> > +	switch (param) {
> > +	case PIN_CONFIG_BIAS_DISABLE:
> > +		arg = !FIELD_GET(PAD_PULL_EN, value);
> > +		break;
> > +	case PIN_CONFIG_BIAS_PULL_DOWN:
> > +		arg = FIELD_GET(PAD_PULLDOWN, value);
> > +		break;
> > +	case PIN_CONFIG_BIAS_PULL_UP:
> > +		arg = FIELD_GET(PAD_PULLUP, value);
> > +		break;
> > +	case PIN_CONFIG_DRIVE_STRENGTH_UA:
> > +		arg = FIELD_GET(PAD_DRIVE, value);
> > +		break;
> > +	case PIN_CONFIG_INPUT_SCHMITT:
> > +		arg = FIELD_GET(PAD_SCHMITT, value);
> > +		break;
> > +	case PIN_CONFIG_SLEW_RATE:
> > +		arg = FIELD_GET(PAD_SLEW_RATE, value);
> > +		break;
> > +	case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
> > +		arg = FIELD_GET(PAD_SLEW_RATE_EN, value);
> > +		break;
> > +	case PIN_CONFIG_K1_STRONG_PULL_UP:
> > +		arg = FIELD_GET(PAD_STRONG_PULL, value);
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	*config = pinconf_to_config_packed(param, arg);
> > +
> > +	return 0;
> > +}
> > +
> > +static int spacemit_pinconf_generate_config(const struct spacemit_pin *spin,
> > +					    unsigned long *configs,
> > +					    unsigned int num_configs,
> > +					    u32 *value)
> > +{
> > +	enum spacemit_pin_io_type type;
> > +	int i, param;
> > +	u32 v = 0, voltage = 0, arg, val;
> 
> > +#define ENABLE_DRV_STRENGTH	BIT(1)
> > +#define ENABLE_SLEW_RATE	BIT(2)
> 
> move this before the function.
> 
Ok
> > +	u32 flag = 0, drv_strength, slew_rate;
> > +
> > +	if (!spin)
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < num_configs; i++) {
> > +		param = pinconf_to_config_param(configs[i]);
> > +		arg = pinconf_to_config_argument(configs[i]);
> > +
> > +		switch (param) {
> > +		case PIN_CONFIG_BIAS_DISABLE:
> > +			v &= ~(PAD_PULL_EN | PAD_PULLDOWN | PAD_PULLUP);
> > +			break;
> > +		case PIN_CONFIG_BIAS_PULL_DOWN:
> > +			v &= ~PAD_PULLDOWN;
> > +			v |= PAD_PULL_EN;
> > +			v |= FIELD_PREP(PAD_PULLDOWN, arg);
> > +			break;
> > +		case PIN_CONFIG_BIAS_PULL_UP:
> > +			v &= ~PAD_PULLUP;
> > +			v |= PAD_PULL_EN;
> > +			v |= FIELD_PREP(PAD_PULLUP, arg);
> > +			break;
> > +		case PIN_CONFIG_DRIVE_STRENGTH_UA:
> > +			flag |= ENABLE_DRV_STRENGTH;
> > +			drv_strength = arg;
> > +			break;
> > +		case PIN_CONFIG_INPUT_SCHMITT:
> > +			slew_rate = arg;
> > +			break;
> > +		case PIN_CONFIG_POWER_SOURCE:
> > +			voltage = arg;
> > +			break;
> > +		case PIN_CONFIG_SLEW_RATE:
> > +			flag |= ENABLE_SLEW_RATE;
> > +			break;
> > +		case PIN_CONFIG_K1_SLEW_RATE_ENABLE:
> > +			if (arg)
> > +				v |= PAD_SLEW_RATE_EN;
> > +			else
> > +				v &= ~PAD_SLEW_RATE_EN;
> > +			break;
> > +		case PIN_CONFIG_K1_STRONG_PULL_UP:
> > +			v &= ~PAD_STRONG_PULL;
> > +			v |= FIELD_PREP(PAD_STRONG_PULL, arg);
> > +			break;
> > +		default:
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> > +	if (flag & ENABLE_DRV_STRENGTH) {
> > +		type = spacemit_to_pin_io_type(spin);
> > +
> > +		/* fix external io type */
> > +		if (type == IO_TYPE_EXTERNAL) {
> > +			switch (voltage) {
> > +			case 1800:
> > +				type = IO_TYPE_1V8;
> > +				break;
> > +			case 3300:
> > +				type = IO_TYPE_3V3;
> > +				break;
> > +			default:
> > +				pr_err("pin(%d) lack power source\n", spin->pin);
> > +				return -EINVAL;
> > +			}
> > +		}
> > +
> > +		val = spacemit_get_driver_strength(type, drv_strength);
> > +
> > +		v &= ~PAD_DRIVE;
> > +		v |= FIELD_PREP(PAD_DRIVE, val);
> > +	}
> > +
> 
> > +	if (flag & ENABLE_SLEW_RATE) {
> > +		v &= ~PAD_SLEW_RATE;
> > +		v |= FIELD_PREP(PAD_SLEW_RATE, arg);
> > +		/* check, driver strength & slew rate */
> > +		if (flag & ENABLE_DRV_STRENGTH) {
> > +			if (slew_rate != FIELD_GET(PAD_SLEW_RATE, v))
> > +				pr_warn("%s: slew rate conflict with driver strength\n", __func__);
> 
> Should this return -EINVAL?
> 
due to slew rate share same register bits with drive strength (bit[12:11]),
there is always value of slew rate setting implicitly, so I would like to
just give a warning instead of failure if there is value conflicts, which mean 
the drive favor settings from drive strength.. and the design logic from hw is
that higher drive strength lead to more fast slew rate.

> > +		} else {
> > +			v &= ~PAD_SCHMITT;
> > +			v |= FIELD_PREP(PAD_SCHMITT, slew_rate);
I have some typo here while doing copy & paste, will fix it

> > +		}
> > +	}
> > +
> > +	*value = v;
> > +
> > +	return 0;
> > +}
> > +
> > +static int spacemit_pin_set_config(struct spacemit_pinctrl *pctrl,
> > +				 unsigned int pin,
> > +				 u32 value)
> > +{
> > +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> > +	void __iomem *reg;
> > +	unsigned long flags;
> > +	unsigned int mux;
> > +
> > +	if (!pin)
> > +		return -EINVAL;
> > +
> > +	reg = spacemit_pin_to_reg(pctrl, spin->pin);
> > +
> > +	raw_spin_lock_irqsave(&pctrl->lock, flags);
> > +	mux = readl_relaxed(reg) & PAD_MUX;
> > +	writel_relaxed(mux | value, reg);
> > +	raw_spin_unlock_irqrestore(&pctrl->lock, flags);
> > +
> > +	return 0;
> > +}
> > +
> > +static int spacemit_pinconf_set(struct pinctrl_dev *pctldev,
> > +				unsigned int pin, unsigned long *configs,
> > +				unsigned int num_configs)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> > +	u32 value;
> > +
> > +	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
> > +		return -EINVAL;
> > +
> > +	return spacemit_pin_set_config(pctrl, pin, value);
> > +}
> > +
> > +static int spacemit_pinconf_group_set(struct pinctrl_dev *pctldev,
> > +				      unsigned int gsel,
> > +				      unsigned long *configs,
> > +				      unsigned int num_configs)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct spacemit_pin *spin;
> > +	const struct group_desc *group;
> > +	u32 value;
> > +	int i;
> > +
> > +	group = pinctrl_generic_get_group(pctldev, gsel);
> > +	if (!group)
> > +		return -EINVAL;
> > +
> > +	spin = spacemit_get_pin(pctrl, group->grp.pins[0]);
> > +	if (spacemit_pinconf_generate_config(spin, configs, num_configs, &value))
> > +		return -EINVAL;
> > +
> > +	for (i = 0; i < group->grp.npins; i++)
> > +		spacemit_pin_set_config(pctrl, group->grp.pins[i], value);
> > +
> > +	return 0;
> > +}
> > +
> 
> > +static void spacemit_pinconf_dbg_show(struct pinctrl_dev *pctldev,
> > +				      struct seq_file *seq, unsigned int pin)
> > +{
> > +	struct spacemit_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev);
> > +	const struct spacemit_pin *spin = spacemit_get_pin(pctrl, pin);
> > +	enum spacemit_pin_io_type type = spacemit_to_pin_io_type(spin);
> > +	void __iomem *reg;
> > +
> > +	reg = spacemit_pin_to_reg(pctrl, pin);
> > +	seq_printf(seq, " reg (0x%x), io (%d)\n", readl(reg), type);
> > +}
> > +
> 
> I think this is a duplicate of "spacemit_pctrl_dbg_show".
> 
yes, right

that's due to the hw using single register to provide all conf & mux,
I would drop this function, or improve it to describe more underlying pinconf bits

> > +static const struct pinconf_ops spacemit_pinconf_ops = {
> > +	.pin_config_get			= spacemit_pinconf_get,
> > +	.pin_config_set			= spacemit_pinconf_set,
> > +	.pin_config_group_set		= spacemit_pinconf_group_set,
> > +	.pin_config_dbg_show		= spacemit_pinconf_dbg_show,
> > +	.is_generic			= true,
> > +};
> > +
> > +static int spacemit_pinctrl_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct device *dev = &pdev->dev;
> > +	struct spacemit_pinctrl *pctrl;
> > +	const struct spacemit_pinctrl_data *pctrl_data;
> > +	int ret;
> > +
> > +	pctrl_data = device_get_match_data(dev);
> > +	if (!pctrl_data)
> > +		return -ENODEV;
> > +
> > +	if (pctrl_data->npins == 0)
> > +		return dev_err_probe(dev, -EINVAL, "invalid pin data\n");
> > +
> > +	pctrl = devm_kzalloc(dev, sizeof(*pctrl), GFP_KERNEL);
> > +	if (!pctrl)
> > +		return -ENOMEM;
> > +
> > +	pctrl->regs = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(pctrl->regs))
> > +		return PTR_ERR(pctrl->regs);
> > +
> > +	pctrl->pdesc.name = dev_name(dev);
> > +	pctrl->pdesc.pins = pctrl_data->pins;
> > +	pctrl->pdesc.npins = pctrl_data->npins;
> > +	pctrl->pdesc.pctlops = &spacemit_pctrl_ops;
> > +	pctrl->pdesc.pmxops = &spacemit_pmx_ops;
> > +	pctrl->pdesc.confops = &spacemit_pinconf_ops;
> > +	pctrl->pdesc.num_custom_params = ARRAY_SIZE(spacemit_pinconf_custom_params);
> > +	pctrl->pdesc.custom_params = spacemit_pinconf_custom_params;
> > +	pctrl->pdesc.owner = THIS_MODULE;
> > +
> > +	pctrl->data = pctrl_data;
> > +	pctrl->dev = dev;
> > +	raw_spin_lock_init(&pctrl->lock);
> > +	INIT_LIST_HEAD(&pctrl->gpiofuncs);
> > +	mutex_init(&pctrl->mutex);
> > +
> > +	platform_set_drvdata(pdev, pctrl);
> > +
> > +	ret = devm_pinctrl_register_and_init(dev, &pctrl->pdesc,
> > +					     pctrl, &pctrl->pctl_dev);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret,
> > +				     "fail to register pinctrl driver\n");
> > +	ret = spacemit_add_gpio_func(np, pctrl);
> > +	if (ret < 0)
> > +		return dev_err_probe(dev, ret,
> > +				     "fail to get gpio function\n");
> > +
> > +	return pinctrl_enable(pctrl->pctl_dev);
> > +}
> > +
> > +static const struct pinctrl_pin_desc k1_pin_desc[] = {
> > +	PINCTRL_PIN(GPIO_00, "GPIO_00"),
> > +	PINCTRL_PIN(GPIO_01, "GPIO_01"),
> > +	PINCTRL_PIN(GPIO_02, "GPIO_02"),
> > +	PINCTRL_PIN(GPIO_03, "GPIO_03"),
> > +	PINCTRL_PIN(GPIO_04, "GPIO_04"),
> > +	PINCTRL_PIN(GPIO_05, "GPIO_05"),
> > +	PINCTRL_PIN(GPIO_06, "GPIO_06"),
> > +	PINCTRL_PIN(GPIO_07, "GPIO_07"),
> > +	PINCTRL_PIN(GPIO_08, "GPIO_08"),
> > +	PINCTRL_PIN(GPIO_09, "GPIO_09"),
> > +	PINCTRL_PIN(GPIO_10, "GPIO_10"),
> > +	PINCTRL_PIN(GPIO_11, "GPIO_11"),
> > +	PINCTRL_PIN(GPIO_12, "GPIO_12"),
> > +	PINCTRL_PIN(GPIO_13, "GPIO_13"),
> > +	PINCTRL_PIN(GPIO_14, "GPIO_14"),
> > +	PINCTRL_PIN(GPIO_15, "GPIO_15"),
> > +	PINCTRL_PIN(GPIO_16, "GPIO_16"),
> > +	PINCTRL_PIN(GPIO_17, "GPIO_17"),
> > +	PINCTRL_PIN(GPIO_18, "GPIO_18"),
> > +	PINCTRL_PIN(GPIO_19, "GPIO_19"),
> > +	PINCTRL_PIN(GPIO_20, "GPIO_20"),
> > +	PINCTRL_PIN(GPIO_21, "GPIO_21"),
> > +	PINCTRL_PIN(GPIO_22, "GPIO_22"),
> > +	PINCTRL_PIN(GPIO_23, "GPIO_23"),
> > +	PINCTRL_PIN(GPIO_24, "GPIO_24"),
> > +	PINCTRL_PIN(GPIO_25, "GPIO_25"),
> > +	PINCTRL_PIN(GPIO_26, "GPIO_26"),
> > +	PINCTRL_PIN(GPIO_27, "GPIO_27"),
> > +	PINCTRL_PIN(GPIO_28, "GPIO_28"),
> > +	PINCTRL_PIN(GPIO_29, "GPIO_29"),
> > +	PINCTRL_PIN(GPIO_30, "GPIO_30"),
> > +	PINCTRL_PIN(GPIO_31, "GPIO_31"),
> > +	PINCTRL_PIN(GPIO_32, "GPIO_32"),
> > +	PINCTRL_PIN(GPIO_33, "GPIO_33"),
> > +	PINCTRL_PIN(GPIO_34, "GPIO_34"),
> > +	PINCTRL_PIN(GPIO_35, "GPIO_35"),
> > +	PINCTRL_PIN(GPIO_36, "GPIO_36"),
> > +	PINCTRL_PIN(GPIO_37, "GPIO_37"),
> > +	PINCTRL_PIN(GPIO_38, "GPIO_38"),
> > +	PINCTRL_PIN(GPIO_39, "GPIO_39"),
> > +	PINCTRL_PIN(GPIO_40, "GPIO_40"),
> > +	PINCTRL_PIN(GPIO_41, "GPIO_41"),
> > +	PINCTRL_PIN(GPIO_42, "GPIO_42"),
> > +	PINCTRL_PIN(GPIO_43, "GPIO_43"),
> > +	PINCTRL_PIN(GPIO_44, "GPIO_44"),
> > +	PINCTRL_PIN(GPIO_45, "GPIO_45"),
> > +	PINCTRL_PIN(GPIO_46, "GPIO_46"),
> > +	PINCTRL_PIN(GPIO_47, "GPIO_47"),
> > +	PINCTRL_PIN(GPIO_48, "GPIO_48"),
> > +	PINCTRL_PIN(GPIO_49, "GPIO_49"),
> > +	PINCTRL_PIN(GPIO_50, "GPIO_50"),
> > +	PINCTRL_PIN(GPIO_51, "GPIO_51"),
> > +	PINCTRL_PIN(GPIO_52, "GPIO_52"),
> > +	PINCTRL_PIN(GPIO_53, "GPIO_53"),
> > +	PINCTRL_PIN(GPIO_54, "GPIO_54"),
> > +	PINCTRL_PIN(GPIO_55, "GPIO_55"),
> > +	PINCTRL_PIN(GPIO_56, "GPIO_56"),
> > +	PINCTRL_PIN(GPIO_57, "GPIO_57"),
> > +	PINCTRL_PIN(GPIO_58, "GPIO_58"),
> > +	PINCTRL_PIN(GPIO_59, "GPIO_59"),
> > +	PINCTRL_PIN(GPIO_60, "GPIO_60"),
> > +	PINCTRL_PIN(GPIO_61, "GPIO_61"),
> > +	PINCTRL_PIN(GPIO_62, "GPIO_62"),
> > +	PINCTRL_PIN(GPIO_63, "GPIO_63"),
> > +	PINCTRL_PIN(GPIO_64, "GPIO_64"),
> > +	PINCTRL_PIN(GPIO_65, "GPIO_65"),
> > +	PINCTRL_PIN(GPIO_66, "GPIO_66"),
> > +	PINCTRL_PIN(GPIO_67, "GPIO_67"),
> > +	PINCTRL_PIN(GPIO_68, "GPIO_68"),
> > +	PINCTRL_PIN(GPIO_69, "GPIO_69"),
> > +	PINCTRL_PIN(GPIO_70, "GPIO_70/PRI_DTI"),
> > +	PINCTRL_PIN(GPIO_71, "GPIO_71/PRI_TMS"),
> > +	PINCTRL_PIN(GPIO_72, "GPIO_72/PRI_TCK"),
> > +	PINCTRL_PIN(GPIO_73, "GPIO_73/PRI_TDO"),
> > +	PINCTRL_PIN(GPIO_74, "GPIO_74"),
> > +	PINCTRL_PIN(GPIO_75, "GPIO_75"),
> > +	PINCTRL_PIN(GPIO_76, "GPIO_76"),
> > +	PINCTRL_PIN(GPIO_77, "GPIO_77"),
> > +	PINCTRL_PIN(GPIO_78, "GPIO_78"),
> > +	PINCTRL_PIN(GPIO_79, "GPIO_79"),
> > +	PINCTRL_PIN(GPIO_80, "GPIO_80"),
> > +	PINCTRL_PIN(GPIO_81, "GPIO_81"),
> > +	PINCTRL_PIN(GPIO_82, "GPIO_82"),
> > +	PINCTRL_PIN(GPIO_83, "GPIO_83"),
> > +	PINCTRL_PIN(GPIO_84, "GPIO_84"),
> > +	PINCTRL_PIN(GPIO_85, "GPIO_85"),
> > +	PINCTRL_PIN(GPIO_86, "GPIO_86"),
> > +	PINCTRL_PIN(GPIO_87, "GPIO_87"),
> > +	PINCTRL_PIN(GPIO_88, "GPIO_88"),
> > +	PINCTRL_PIN(GPIO_89, "GPIO_89"),
> > +	PINCTRL_PIN(GPIO_90, "GPIO_90"),
> > +	PINCTRL_PIN(GPIO_91, "GPIO_91"),
> > +	PINCTRL_PIN(GPIO_92, "GPIO_92"),
> > +	PINCTRL_PIN(GPIO_93, "GPIO_93/PWR_SCL"),
> > +	PINCTRL_PIN(GPIO_94, "GPIO_94/PWR_SDA"),
> > +	PINCTRL_PIN(GPIO_95, "GPIO_95/VCX0_EN"),
> > +	PINCTRL_PIN(GPIO_96, "GPIO_96/DVL0"),
> > +	PINCTRL_PIN(GPIO_97, "GPIO_97/DVL1"),
> > +	PINCTRL_PIN(GPIO_98,  "GPIO_98/QSPI_DAT3"),
> > +	PINCTRL_PIN(GPIO_99,  "GPIO_99/QSPI_DAT2"),
> > +	PINCTRL_PIN(GPIO_100, "GPIO_100/QSPI_DAT1"),
> > +	PINCTRL_PIN(GPIO_101, "GPIO_101/QSPI_DAT0"),
> > +	PINCTRL_PIN(GPIO_102, "GPIO_102/QSPI_CLK"),
> > +	PINCTRL_PIN(GPIO_103, "GPIO_103/QSPI_CSI"),
> > +	PINCTRL_PIN(GPIO_104, "GPIO_104/MMC1_DAT3"),
> > +	PINCTRL_PIN(GPIO_105, "GPIO_105/MMC1_DAT2"),
> > +	PINCTRL_PIN(GPIO_106, "GPIO_106/MMC1_DAT1"),
> > +	PINCTRL_PIN(GPIO_107, "GPIO_107/MMC1_DAT0"),
> > +	PINCTRL_PIN(GPIO_108, "GPIO_108/MMC1_CMD"),
> > +	PINCTRL_PIN(GPIO_109, "GPIO_109/MMC1_CLK"),
> > +	PINCTRL_PIN(GPIO_110, "GPIO_110"),
> > +	PINCTRL_PIN(GPIO_111, "GPIO_111"),
> > +	PINCTRL_PIN(GPIO_112, "GPIO_112"),
> > +	PINCTRL_PIN(GPIO_113, "GPIO_113"),
> > +	PINCTRL_PIN(GPIO_114, "GPIO_114"),
> > +	PINCTRL_PIN(GPIO_115, "GPIO_115"),
> > +	PINCTRL_PIN(GPIO_116, "GPIO_116"),
> > +	PINCTRL_PIN(GPIO_117, "GPIO_117"),
> > +	PINCTRL_PIN(GPIO_118, "GPIO_118"),
> > +	PINCTRL_PIN(GPIO_119, "GPIO_119"),
> > +	PINCTRL_PIN(GPIO_120, "GPIO_120"),
> > +	PINCTRL_PIN(GPIO_121, "GPIO_121"),
> > +	PINCTRL_PIN(GPIO_122, "GPIO_122"),
> > +	PINCTRL_PIN(GPIO_123, "GPIO_123"),
> > +	PINCTRL_PIN(GPIO_124, "GPIO_124"),
> > +	PINCTRL_PIN(GPIO_125, "GPIO_125"),
> > +	PINCTRL_PIN(GPIO_126, "GPIO_126"),
> > +	PINCTRL_PIN(GPIO_127, "GPIO_127"),
> > +};
> > +
> > +static const struct spacemit_pin k1_pin_data[ARRAY_SIZE(k1_pin_desc)] = {
> > +	K1_FUNC_PIN(GPIO_00, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_01, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_02, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_03, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_04, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_05, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_06, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_07, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_08, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_09, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_10, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_11, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_12, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_13, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_14, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_15, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_16, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_17, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_18, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_19, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_20, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_21, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_22, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_23, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_24, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_25, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_26, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_27, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_28, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_29, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_30, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_31, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_32, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_33, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_34, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_35, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_36, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_37, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_38, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_39, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_40, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_41, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_42, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_43, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_44, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_45, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_46, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_47, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_48, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_49, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_50, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_51, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_52, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_53, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_54, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_55, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_56, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_57, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_58, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_59, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_60, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_61, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_62, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_63, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_64, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_65, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_66, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_67, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_68, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_69, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_70, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_71, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_72, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_73, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_74, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_75, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_76, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_77, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_78, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_79, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_80, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_81, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_82, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_83, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_84, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_85, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_86, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_87, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_88, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_89, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_90, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_91, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_92, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_93, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_94, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_95, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_96, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_97, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_98, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_99, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_100, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_101, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_102, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_103, IO_TYPE_EXTERNAL),
> > +	K1_FUNC_PIN(GPIO_104, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_105, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_106, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_107, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_108, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_109, IO_TYPE_3V3),
> > +	K1_FUNC_PIN(GPIO_110, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_111, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_112, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_113, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_114, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_115, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_116, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_117, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_118, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_119, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_120, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_121, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_122, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_123, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_124, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_125, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_126, IO_TYPE_1V8),
> > +	K1_FUNC_PIN(GPIO_127, IO_TYPE_1V8),
> > +};
> > +
> > +static const struct spacemit_pinctrl_data k1_pinctrl_data = {
> > +	.pins = k1_pin_desc,
> > +	.data = k1_pin_data,
> > +	.npins = ARRAY_SIZE(k1_pin_desc),
> > +};
> > +
> > +static const struct of_device_id k1_pinctrl_ids[] = {
> > +	{ .compatible = "spacemit,k1-pinctrl", .data = &k1_pinctrl_data },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, k1_pinctrl_ids);
> > +
> > +static struct platform_driver k1_pinctrl_driver = {
> > +	.probe	= spacemit_pinctrl_probe,
> > +	.driver	= {
> > +		.name			= "k1-pinctrl",
> > +		.suppress_bind_attrs	= true,
> > +		.of_match_table		= k1_pinctrl_ids,
> > +	},
> > +};
> > +module_platform_driver(k1_pinctrl_driver);
> > +
> > +MODULE_AUTHOR("Yixun Lan <dlan@gentoo.org>");
> > +MODULE_DESCRIPTION("Pinctrl driver for the SpacemiT K1 SoC");
> > +MODULE_LICENSE("GPL");
> > diff --git a/drivers/pinctrl/spacemit/pinctrl-k1.h b/drivers/pinctrl/spacemit/pinctrl-k1.h
> > new file mode 100644
> > index 0000000000000..9d6589b626e9d
> > --- /dev/null
> > +++ b/drivers/pinctrl/spacemit/pinctrl-k1.h
> > @@ -0,0 +1,36 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/* Copyright (c) 2024 Yixun Lan <dlan@gentoo.org> */
> > +
> > +#ifndef _PINCTRL_SPACEMIT_K1_H
> > +#define _PINCTRL_SPACEMIT_K1_H
> > +
> > +#include <linux/bits.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/device.h>
> > +#include <linux/mutex.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pinctrl/pinctrl.h>
> > +#include <linux/pinctrl/pinconf.h>
> > +
> > +enum spacemit_pin_io_type {
> > +	IO_TYPE_NONE = 0,
> > +	IO_TYPE_1V8,
> > +	IO_TYPE_3V3,
> > +	IO_TYPE_EXTERNAL,
> > +};
> > +
> > +#define K1_PIN_IO_TYPE		GENMASK(2, 1)
> > +
> > +#define K1_PIN_CAP_IO_TYPE(type)		\
> > +	FIELD_PREP_CONST(K1_PIN_IO_TYPE, type)
> > +#define K1_PIN_GET_IO_TYPE(val)			\
> > +	FIELD_GET(K1_PIN_IO_TYPE, val)
> > +
> > +#define K1_FUNC_PIN(_id, _io)				\
> > +	{							\
> > +		.pin = (_id),					\
> > +		.flags = (K1_PIN_CAP_IO_TYPE(_io))		\
> > +	}
> > +
> > +#endif /* _PINCTRL_SPACEMIT_K1_H */
> > 
> > -- 
> > 2.45.2
> > 

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
       [not found]       ` <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com>
@ 2024-08-27 15:13         ` Conor Dooley
  0 siblings, 0 replies; 19+ messages in thread
From: Conor Dooley @ 2024-08-27 15:13 UTC (permalink / raw)
  To: Yixun Lan
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Yangyu Chen, Jesse Taube,
	Jisheng Zhang, Inochi Amaoto, Icenowy Zheng, Meng Zhang,
	Meng Zhang, devicetree, linux-riscv, linux-kernel, linux-gpio

[-- Attachment #1: Type: text/plain, Size: 3638 bytes --]

On Tue, Aug 27, 2024 at 02:23:05AM +0000, Yixun Lan wrote:
> Hi Conor:
> 
> On 17:22 Mon 26 Aug     , Conor Dooley wrote:
> > On Mon, Aug 26, 2024 at 03:09:39AM +0000, Yixun Lan wrote:
> > > 
> > > On 13:10 Sun 25 Aug     , Yixun Lan wrote:
> > > > Add dt-binding for the pinctrl driver of SpacemiT's K1 SoC.
> > > > 
> > > > Two vendor specific properties are introduced here, As the pinctrl
> > > > has dedicated slew rate enable control - bit[7], so we have
> > > > spacemit,slew-rate-{enable,disable} for this. For the same reason,
> > > > creating spacemit,strong-pull-up for the strong pull up control.
> > > > 
> > > > Signed-off-by: Yixun Lan <dlan@gentoo.org>
> > 
> > I got this mail, and one of your other ones, 5 times. What's going wrong
> > with your mail setup? 
> > 
> Oops, sorry for this, it's the second time you complain this..
> TBO, I have no idea what's happened, asked Yangyu, he didn't have this problem
> 
> I'm using mutt+msmtp to reply, while using b4 to send the patch series,

The patches only appear once, it's the replies that have the problem.

> for all the mails you received, do they all have same message-id?
> I have a local filter for duplicated mails myself, could this help?
> 
> (I leave only one address of your mail in this reply, see if still have problem?)

It's nothing to do with me having multiple addresses, the mails have
different message ids, and as you can see below, even different sizes too.
For example, two copies of the current reply came in as:
Message-ID: <66cd38b0.050a0220.113bce.d5acSMTPIN_ADDED_BROKEN@mx.google.com>
Message-ID: <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com>

Your mails also don't properly appear on lore, you can see some [not
found] mails there:
https://lore.kernel.org/linux-devicetree/20240827033057.GYB208832.dlan.gentoo/

The SMPTIN_ADDED_BROKEN might be a hint as to what is wrong with your
setup?

> 
> > | 250   T Aug 25 Yixun Lan       (6.0K) ┌─>[PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3
> > | 251 N T Aug 26 Inochi Amaoto   ( 12K) │ ┌─>
> > | 252   T Aug 25 Yixun Lan       (6.8K) ├─>[PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for K1 SoC
> > | 253 N T Aug 26 Inochi Amaoto   ( 46K) │ ┌─>
> > | 254   T Aug 25 Yixun Lan       ( 38K) ├─>[PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT K1 SoC
> > | 255 N T Aug 26 Inochi Amaoto   ( 22K) │ ┌─>
> > | 256   T Aug 26 Yixun Lan       ( 333) │ ├─>
> > | 257   T Aug 26 Yixun Lan       ( 338) │ ├─>
> > | 258   T Aug 26 Yixun Lan       ( 334) │ ├─>
> > | 259   T Aug 26 Yixun Lan       ( 334) │ ├─>
> > | 260   T Aug 26 Yixun Lan       ( 333) │ ├─>
> > | 261   C Aug 25 Rob Herring (Ar (9.0K) │ ├─>
> > | 262 N C Aug 26 Krzysztof Kozlo ( 14K) │ │ ┌─>
> > | 263 N C Aug 26 Inochi Amaoto   ( 19K) │ │ ├─>
> > | 264   C Aug 26 Yixun Lan       ( 285) │ │ ├─>
> > | 265   C Aug 26 Yixun Lan       ( 281) │ │ ├─>
> > | 266 N C Aug 26 Yixun Lan       ( 14K) │ │ ├─>
> > | 267 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
> > | 268 N C Aug 26 Yixun Lan       ( 12K) │ │ ├─>
> > | 269   T Aug 25 Krzysztof Kozlo ( 13K) │ ├─>
> > | 270   T Aug 25 Yixun Lan       ( 14K) ├─>[PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for K1 SoC
> > | 271   T Aug 25 Yixun Lan       (8.5K) [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC
> > 
> 
> 
> -- 
> Yixun Lan (dlan)
> Gentoo Linux Developer
> GPG Key ID AABEFD55

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2024-08-27 15:13 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-25 13:10 [PATCH v2 0/4] riscv: spacemit: add pinctrl support to K1 SoC Yixun Lan
2024-08-25 13:10 ` [PATCH v2 1/4] dt-binding: pinctrl: spacemit: add documents for " Yixun Lan
2024-08-25 13:48   ` Krzysztof Kozlowski
2024-08-26  1:36     ` Yixun Lan
2024-08-26  4:19       ` Inochi Amaoto
     [not found]     ` <66cbdc2a.050a0220.2d7994.f671SMTPIN_ADDED_BROKEN@mx.google.com>
2024-08-26  5:37       ` Krzysztof Kozlowski
2024-08-25 14:22   ` Rob Herring (Arm)
2024-08-26  3:09   ` Yixun Lan
2024-08-26  4:23   ` Inochi Amaoto
     [not found]   ` <66cbf3bb.050a0220.2632ed.b191SMTPIN_ADDED_BROKEN@mx.google.com>
2024-08-26 16:22     ` Conor Dooley
2024-08-27  2:23       ` Yixun Lan
     [not found]       ` <66cd3abe.050a0220.bf184.b4feSMTPIN_ADDED_BROKEN@mx.google.com>
2024-08-27 15:13         ` Conor Dooley
2024-08-25 13:10 ` [PATCH v2 2/4] pinctrl: spacemit: add support for SpacemiT " Yixun Lan
2024-08-26  8:16   ` Inochi Amaoto
2024-08-27  3:30     ` Yixun Lan
2024-08-25 13:10 ` [PATCH v2 3/4] riscv: dts: spacemit: add pinctrl support for " Yixun Lan
2024-08-26  7:55   ` Inochi Amaoto
2024-08-27  3:03     ` Yixun Lan
2024-08-25 13:10 ` [PATCH v2 4/4] riscv: dts: spacemit: add pinctrl property to uart0 in BPI-F3 Yixun Lan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).