[PATCH v2 0/5] Pinctrl: Add Amlogic pinctrl driver

[PATCH v2 0/5] Pinctrl: Add Amlogic pinctrl driver

[Xianwei Zhao via B4 Relay]

Add pinctrl driver support for Amloigc SoCs

Base on the previous discussion,
https://lore.kernel.org/r/20241113-a4_pinctrl-v6-0-35ba2401ee35@amlogic.com
The existed meson driver failed to meet the requirement of the current dt-binding.
So we start this new pinctrl driver to solve problem.

The advantage of this version: Once the source file and binding document
are added, adding dts node will be only operation for subsequent Amlogic SoCs
(such as A4, A5).

The code in DTS file is also readable when using GPIO, as below:
reset-gpios = <&gpiob 6 GPIO_ACTIVE_LOW>;

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Changes in v2:
- Some modifications and optimizations were made to bindings according to Rob's suggestions.
- Refined some details on functions node for bindings. 
- Some source code optimizations were made according to Linus's suggestions.
- Add stand API to generic to deal pinmux propertity.
- Add private reg data to deal with for future SoCs.
- Simplified some unused processing (previously used for extension).
- Sync add to MAINTAINERS files.
- Link to v1: https://lore.kernel.org/r/20241211-amlogic-pinctrl-v1-0-410727335119@amlogic.com

---
Xianwei Zhao (5):
      dt-bindings: pinctrl: Add support for Amlogic SoCs
      pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file
      pinctrl: Add driver support for Amlogic SoCs
      arm64: dts: amlogic: a4: add pinctrl node
      MAINTAINERS: Add an entry for Amlogic pinctrl driver

 .../bindings/pinctrl/amlogic,pinctrl-a4.yaml       |  155 +++
 MAINTAINERS                                        |    8 +
 arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi        |  133 +++
 drivers/pinctrl/Kconfig                            |   18 +
 drivers/pinctrl/Makefile                           |    1 +
 drivers/pinctrl/pinconf-generic.c                  |  130 +++
 drivers/pinctrl/pinconf.h                          |    4 +
 drivers/pinctrl/pinctrl-amlogic.c                  | 1047 ++++++++++++++++++++
 include/dt-bindings/pinctrl/amlogic,pinctrl.h      |   68 ++
 include/linux/pinctrl/pinconf-generic.h            |    4 +
 10 files changed, 1568 insertions(+)
---
base-commit: 4de5110762b94b9978fb8182a568572fb2194f8b
change-id: 20241211-amlogic-pinctrl-22ea61820d0d

Best regards,
-- 
Xianwei Zhao <xianwei.zhao@amlogic.com>

[PATCH v2 1/5] dt-bindings: pinctrl: Add support for Amlogic SoCs

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add the dt-bindings for Amlogic pin controller, and add a new
dt-binding header file which document the GPIO bank names and
alternative func value of all Amlogic subsequent SoCs.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 .../bindings/pinctrl/amlogic,pinctrl-a4.yaml       | 155 +++++++++++++++++++++
 include/dt-bindings/pinctrl/amlogic,pinctrl.h      |  68 +++++++++
 2 files changed, 223 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
new file mode 100644
index 000000000000..75863d179933
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
@@ -0,0 +1,155 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl-a4.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic pinmux controller
+
+maintainers:
+  - Xianwei Zhao <xianwei.zhao@amlogic.com>
+
+allOf:
+  - $ref: pinctrl.yaml#
+
+properties:
+  compatible:
+    const: amlogic,pinctrl-a4
+
+  reg:
+    minItems: 2
+
+  reg-names:
+    items:
+      - const: mux
+      - const: gpio
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^gpio@[0-9a-f]+$":
+    type: object
+
+    properties:
+      reg:
+        minItems: 2
+
+      mask:
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      gpio-controller: true
+
+      "#gpio-cells":
+        const: 2
+
+      ngpios:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 1
+        maximum: 32
+
+      identity:
+        description: |
+          identifier are provided by the pin controller header file at:
+          <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
+
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+    required:
+      - reg
+      - gpio-controller
+      - "#gpio-cells"
+      - ngpios
+      - identity
+
+    additionalProperties: false
+
+  "^func-[0-9a-z-]+$":
+    type: object
+    additionalProperties:
+      type: object
+      allOf:
+        - $ref: pincfg-node.yaml#
+        - $ref: pinmux-node.yaml#
+
+      description:
+        A pin multiplexing sub-node describes how to configure a set of (or a
+        single) pin in some desired alternate function mode.
+        A single sub-node may define several pin configurations.
+
+      properties:
+        pinmux:
+          description: |
+            Integer array representing pin number and pin multiplexing
+            configuration.
+            When a pin has to be configured in alternate function mode, use
+            this property to identify the pin by its global index, and provide
+            its alternate function configuration number along with it.
+            bank identifier are provided by the pin controller header file at:
+            <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
+            Integers values in "pinmux" argument list are assembled as:
+            (((BANK << 8) + PIN) << 8)  | MUX_FUNC))
+
+      required:
+        - pinmux
+
+      additionalProperties: true
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
+    apb {
+      #address-cells = <2>;
+      #size-cells = <2>;
+      periphs_pinctrl: pinctrl@8e700 {
+        compatible = "amlogic,pinctrl-a4";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        reg = <0x0 0x8e700 0x0 0x04>,
+              <0x0 0x8e704 0x0 0x60>;
+        reg-names = "mux", "gpio";
+
+        gpio@14 {
+          reg = <0x14>,<0x30>;
+          gpio-controller;
+          #gpio-cells = <2>;
+          ngpios = <10>;
+          identity = <AMLOGIC_GPIO_B>;
+        };
+
+        func-uart-b {
+          uart-b-default {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 1, AF4)>;
+            bias-pull-up;
+            drive-strength-microamp = <4000>;
+          };
+
+          uart-b-pins1 {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 5, AF2)>;
+            bias-pull-up;
+            drive-strength-microamp = <4000>;
+          };
+        };
+
+        func-uart-c {
+          uart-c-default {
+            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 3, AF1)>,
+                     <AML_PINMUX(AMLOGIC_GPIO_B, 2, AF1)>;
+            bias-pull-up;
+            drive-strength-microamp = <4000>;
+          };
+        };
+      };
+    };
diff --git a/include/dt-bindings/pinctrl/amlogic,pinctrl.h b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
new file mode 100644
index 000000000000..03db0a730e8b
--- /dev/null
+++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
+/*
+ * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
+ * Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
+ */
+
+#ifndef _DT_BINDINGS_AMLOGIC_PINCTRL_H
+#define _DT_BINDINGS_AMLOGIC_PINCTRL_H
+
+/* define PIN modes */
+#define AF0	0x0
+#define AF1	0x1
+#define AF2	0x2
+#define AF3	0x3
+#define AF4	0x4
+#define AF5	0x5
+#define AF6	0x6
+#define AF7	0x7
+#define AF8	0x8
+#define AF9	0x9
+#define AF10	0xa
+#define AF11	0xb
+#define AF12	0xc
+#define AF13	0xd
+#define AF14	0xe
+#define AF15	0xf
+
+#define AML_PIN_ALT_FUNC_MASK	0xf
+
+/* Normal PIN bank */
+#define AMLOGIC_GPIO_A		0
+#define AMLOGIC_GPIO_B		1
+#define AMLOGIC_GPIO_C		2
+#define AMLOGIC_GPIO_D		3
+#define AMLOGIC_GPIO_E		4
+#define AMLOGIC_GPIO_F		5
+#define AMLOGIC_GPIO_G		6
+#define AMLOGIC_GPIO_H		7
+#define AMLOGIC_GPIO_I		8
+#define AMLOGIC_GPIO_J		9
+#define AMLOGIC_GPIO_K		10
+#define AMLOGIC_GPIO_L		11
+#define AMLOGIC_GPIO_M		12
+#define AMLOGIC_GPIO_N		13
+#define AMLOGIC_GPIO_O		14
+#define AMLOGIC_GPIO_P		15
+#define AMLOGIC_GPIO_Q		16
+#define AMLOGIC_GPIO_R		17
+#define AMLOGIC_GPIO_S		18
+#define AMLOGIC_GPIO_T		19
+#define AMLOGIC_GPIO_U		20
+#define AMLOGIC_GPIO_V		21
+#define AMLOGIC_GPIO_W		22
+#define AMLOGIC_GPIO_X		23
+#define AMLOGIC_GPIO_Y		24
+#define AMLOGIC_GPIO_Z		25
+
+/* Special PIN bank */
+#define AMLOGIC_GPIO_DV		26
+#define AMLOGIC_GPIO_AO		27
+#define AMLOGIC_GPIO_CC		28
+#define AMLOGIC_GPIO_TEST_N	29
+
+#define AML_PINMUX(bank, offset, mode)	(((((bank) << 8) + (offset)) << 8) | (mode))
+#define AML_PINMUX_TO_BANK(pinmux)	(((pinmux) >> 16) & 0xff)
+#define AML_PINMUX_TO_OFFSET(pinmux)	(((pinmux) >> 8) & 0xff)
+
+#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */

-- 
2.37.1

[PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

When describing pin mux func through pinmux propertity,
a standard API is added for support. The pinmux contains pin
identification and mux values, which can include multiple
pins. And groups configuration use other word. DTS such as:

func-name {
	group_alias: group-name{
		pinmux= <pin_id << 8 | mux_value)>,
			<pin_id << 8 | mux_value)>;
		bias-pull-up;
		drive-strength-microamp = <4000>;
	};
};

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/pinctrl/pinconf-generic.c       | 130 ++++++++++++++++++++++++++++++++
 drivers/pinctrl/pinconf.h               |   4 +
 include/linux/pinctrl/pinconf-generic.h |   4 +
 3 files changed, 138 insertions(+)

diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
index 0b13d7f17b32..a4d3c12a80c4 100644
--- a/drivers/pinctrl/pinconf-generic.c
+++ b/drivers/pinctrl/pinconf-generic.c
@@ -233,6 +233,67 @@ static void parse_dt_cfg(struct device_node *np,
 	}
 }
 
+/**
+ * pinconf_generic_parse_dt_pinmux()
+ * parse the pinmux properties into generic pin mux values.
+ * @np: node containing the pinmux properties
+ * @pctldev: pincontrol device
+ * @pid: array with pin identity entries
+ * @pmux: array with pin mux value entries
+ * @npins: number of pins
+ *
+ * pinmux propertity: mux value [0,7]bits and pin identity [8,31]bits.
+ */
+int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
+				    unsigned int **pid, unsigned int **pmux,
+				    unsigned int *npins)
+{
+	unsigned int *pid_t;
+	unsigned int *pmux_t;
+	struct property *prop;
+	unsigned int npins_t, i;
+	u32 value;
+	int ret;
+
+	prop = of_find_property(np, "pinmux", NULL);
+	if (!prop) {
+		dev_info(dev, "Missing pinmux property\n");
+		return -ENOENT;
+	}
+
+	if (!pid || !pmux || !npins) {
+		dev_err(dev, "paramers error\n");
+		return -EINVAL;
+	}
+
+	npins_t = prop->length / sizeof(u32);
+	pid_t = devm_kcalloc(dev, npins_t, sizeof(*pid_t), GFP_KERNEL);
+	pmux_t = devm_kcalloc(dev, npins_t, sizeof(*pmux_t), GFP_KERNEL);
+	if (!pid_t || !pmux_t) {
+		dev_err(dev, "kalloc memory fail\n");
+		return -ENOMEM;
+	}
+	for (i = 0; i < npins_t; i++) {
+		ret = of_property_read_u32_index(np, "pinmux", i, &value);
+		if (ret) {
+			dev_err(dev, "get pinmux value fail\n");
+			goto exit;
+		}
+		pmux_t[i] = value & 0xff;
+		pid_t[i] = (value >> 8) & 0xffffff;
+	}
+	*pid = pid_t;
+	*pmux = pmux_t;
+	*npins = npins_t;
+
+	return 0;
+exit:
+	devm_kfree(dev, pid_t);
+	devm_kfree(dev, pmux_t);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_pinmux);
+
 /**
  * pinconf_generic_parse_dt_config()
  * parse the config properties into generic pinconfig values.
@@ -295,6 +356,75 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_config);
 
+int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
+					  struct device_node *np,
+					  struct pinctrl_map **map,
+					  unsigned int *num_maps)
+{
+	struct device *dev = pctldev->dev;
+	struct device_node *pnode;
+	unsigned long *configs = NULL;
+	unsigned int num_configs = 0;
+	struct property *prop;
+	unsigned int reserved_maps;
+	int reserve;
+	int ret;
+
+	prop = of_find_property(np, "pinmux", NULL);
+	if (!prop) {
+		dev_info(dev, "Missing pinmux property\n");
+		return -ENOENT;
+	}
+
+	pnode = of_get_parent(np);
+	if (!pnode) {
+		dev_info(dev, "Missing function node\n");
+		return -EINVAL;
+	}
+
+	reserved_maps = 0;
+	*map = NULL;
+	*num_maps = 0;
+
+	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
+					      &num_configs);
+	if (ret < 0) {
+		dev_err(dev, "%pOF: could not parse node property\n", np);
+		return ret;
+	}
+
+	reserve = 1;
+	if (num_configs)
+		reserve++;
+
+	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps,
+					num_maps, reserve);
+	if (ret < 0)
+		goto exit;
+
+	ret = pinctrl_utils_add_map_mux(pctldev, map,
+					&reserved_maps, num_maps, np->name,
+					pnode->name);
+	if (ret < 0)
+		goto exit;
+
+	if (num_configs) {
+		ret = pinctrl_utils_add_map_configs(pctldev, map, &reserved_maps,
+						    num_maps, np->name, configs,
+						    num_configs, PIN_MAP_TYPE_CONFIGS_GROUP);
+		if (ret < 0)
+			goto exit;
+	}
+
+exit:
+	kfree(configs);
+	if (ret)
+		pinctrl_utils_free_map(pctldev, *map, *num_maps);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pinconf_generic_dt_node_to_map_pinmux);
+
 int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
 		struct device_node *np, struct pinctrl_map **map,
 		unsigned int *reserved_maps, unsigned int *num_maps,
diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
index a14c950bc700..a171195b3615 100644
--- a/drivers/pinctrl/pinconf.h
+++ b/drivers/pinctrl/pinconf.h
@@ -138,4 +138,8 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
 				    struct pinctrl_dev *pctldev,
 				    unsigned long **configs,
 				    unsigned int *nconfigs);
+
+int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
+				    unsigned int **pid, unsigned int **pmux,
+				    unsigned int *npins);
 #endif
diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
index 53cfde98433d..1bcf071b860e 100644
--- a/include/linux/pinctrl/pinconf-generic.h
+++ b/include/linux/pinctrl/pinconf-generic.h
@@ -232,4 +232,8 @@ static inline int pinconf_generic_dt_node_to_map_all(struct pinctrl_dev *pctldev
 			PIN_MAP_TYPE_INVALID);
 }
 
+int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
+					  struct device_node *np,
+					  struct pinctrl_map **map,
+					  unsigned int *num_maps);
 #endif /* __LINUX_PINCTRL_PINCONF_GENERIC_H */

-- 
2.37.1

[PATCH v2 3/5] pinctrl: Add driver support for Amlogic SoCs

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add a new pinctrl driver for Amlogic SoCs. All future Amlogic
SoCs pinctrl drives use this, such A4, A5, S6, S7 etc. To support
new Amlogic SoCs, only need to add the corresponding dts file.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 drivers/pinctrl/Kconfig           |   18 +
 drivers/pinctrl/Makefile          |    1 +
 drivers/pinctrl/pinctrl-amlogic.c | 1047 +++++++++++++++++++++++++++++++++++++
 3 files changed, 1066 insertions(+)

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index 95a8e2b9a614..3d44f8761056 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -49,6 +49,24 @@ config PINCTRL_AMD
 	  Requires ACPI/FDT device enumeration code to set up a platform
 	  device.
 
+config PINCTRL_AMLOGIC
+	bool "AMLOGIC pincontrol"
+	depends on ARCH_MESON || COMPILE_TEST
+	depends on OF
+	default y
+	select PINMUX
+	select PINCONF
+	select GENERIC_PINCONF
+	select GPIOLIB
+	select OF_GPIO
+	select REGMAP_MMIO
+	help
+	  This is the driver for the pin controller found on Amlogic SoCs.
+
+	  This driver is simplify subsequent support for new amlogic SoCs,
+	  to support new Amlogic SoCs, only need to add the corresponding dts file,
+	  no additional binding header files or C file are added.
+
 config PINCTRL_APPLE_GPIO
 	tristate "Apple SoC GPIO pin controller driver"
 	depends on ARCH_APPLE
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index fba1c56624c0..638ffec63ff1 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_GENERIC_PINCONF)	+= pinconf-generic.o
 obj-$(CONFIG_OF)		+= devicetree.o
 
 obj-$(CONFIG_PINCTRL_AMD)	+= pinctrl-amd.o
+obj-$(CONFIG_PINCTRL_AMLOGIC) 	+= pinctrl-amlogic.o
 obj-$(CONFIG_PINCTRL_APPLE_GPIO) += pinctrl-apple-gpio.o
 obj-$(CONFIG_PINCTRL_ARTPEC6)	+= pinctrl-artpec6.o
 obj-$(CONFIG_PINCTRL_AS3722)	+= pinctrl-as3722.o
diff --git a/drivers/pinctrl/pinctrl-amlogic.c b/drivers/pinctrl/pinctrl-amlogic.c
new file mode 100644
index 000000000000..8af02b523b69
--- /dev/null
+++ b/drivers/pinctrl/pinctrl-amlogic.c
@@ -0,0 +1,1047 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
+ * Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
+ */
+
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/string_helpers.h>
+
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <dt-bindings/pinctrl/amlogic,pinctrl.h>
+
+#include "core.h"
+#include "pinconf.h"
+
+#define gpio_chip_to_bank(chip) \
+		container_of(chip, struct aml_gpio_bank, gpio_chip)
+
+#define AML_REG_PULLEN		0
+#define AML_REG_PULL		1
+#define AML_REG_DIR		2
+#define AML_REG_OUT		3
+#define AML_REG_IN		4
+#define AML_REG_DS		5
+#define AML_NUM_REG		6
+
+enum aml_pinconf_drv {
+	PINCONF_DRV_500UA,
+	PINCONF_DRV_2500UA,
+	PINCONF_DRV_3000UA,
+	PINCONF_DRV_4000UA,
+};
+
+struct aml_pio_control {
+	u32 gpio_offset;
+	u32 mux_offset;
+	u32 mux_mask;
+	u32 reg_offset[AML_NUM_REG];
+	u32 bit_offset[AML_NUM_REG];
+};
+
+struct  aml_reg_bit {
+	u32 bank_id;
+	u32 reg_offs[AML_NUM_REG];
+	u32 bit_offs[AML_NUM_REG];
+};
+
+struct aml_pctl_data {
+	unsigned int number;
+	struct aml_reg_bit rb_offs[];
+};
+
+struct aml_pmx_func {
+	const char	*name;
+	const char	**groups;
+	unsigned int	ngroups;
+};
+
+struct aml_pctl_group {
+	const char		*name;
+	unsigned int		npins;
+	unsigned int		*pins;
+	unsigned int		*func;
+};
+
+struct aml_gpio_bank {
+	struct gpio_chip		gpio_chip;
+	struct aml_pio_control		pc;
+	u32				bank_id;
+	unsigned int			pin_base;
+};
+
+struct aml_pinctrl {
+	struct device			*dev;
+	struct pinctrl_dev		*pctl;
+	struct aml_gpio_bank		*banks;
+	int				nbanks;
+	struct aml_pmx_func		*functions;
+	int				nfunctions;
+	struct aml_pctl_group		*groups;
+	int				ngroups;
+
+	struct regmap			*reg_mux;
+	struct regmap			*reg_gpio;
+	struct regmap			*reg_ds;
+	const struct aml_pctl_data	*data;
+};
+
+static const unsigned int aml_bit_strides[AML_NUM_REG] = {
+	1, 1, 1, 1, 1, 2
+};
+
+static const unsigned int aml_def_regoffs[AML_NUM_REG] = {
+	3, 4, 2, 1, 0, 7
+};
+
+static const char *aml_bank_name[30] = {
+"GPIOA", "GPIOB", "GPIOC", "GPIOD", "GPIOE", "GPIOF", "GPIOG",
+"GPIOH", "GPIOI", "GPIOJ", "GPIOK", "GPIOL", "GPIOM", "GPION",
+"GPIOO", "GPIOP", "GPIOQ", "GPIOR", "GPIOS", "GPIOT", "GPIOU",
+"GPIOV", "GPIOW", "GPIOX", "GPIOY", "GPIOZ", "GPIODV", "GPIOAO",
+"GPIOCC", "TEST_N"
+};
+
+static int aml_pmx_calc_reg_and_offset(struct pinctrl_gpio_range *range,
+				       unsigned int pin, unsigned int *reg,
+				       unsigned int *offset)
+{
+	struct aml_gpio_bank *bank = gpio_chip_to_bank(range->gc);
+	unsigned int shift;
+
+	shift = ((pin - range->pin_base) << 2) + (ffs(bank->pc.mux_mask) - 1);
+	*reg = (bank->pc.mux_offset + (shift / 32)) * 4;
+	*offset = shift % 32;
+
+	return 0;
+}
+
+static int aml_pctl_set_function(struct aml_pinctrl *info,
+				 struct pinctrl_gpio_range *range,
+				 int pin_id, int func)
+{
+	int reg;
+	int offset;
+
+	aml_pmx_calc_reg_and_offset(range, pin_id, &reg, &offset);
+	return regmap_update_bits(info->reg_mux, reg,
+			0xf << offset, (func & 0xf) << offset);
+}
+
+static int aml_pmx_get_funcs_count(struct pinctrl_dev *pctldev)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->nfunctions;
+}
+
+static const char *aml_pmx_get_fname(struct pinctrl_dev *pctldev,
+				     unsigned int selector)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->functions[selector].name;
+}
+
+static int aml_pmx_get_groups(struct pinctrl_dev *pctldev,
+			      unsigned int selector,
+			      const char * const **grps,
+			      unsigned * const ngrps)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	*grps = info->functions[selector].groups;
+	*ngrps = info->functions[selector].ngroups;
+
+	return 0;
+}
+
+static int aml_pmx_set_mux(struct pinctrl_dev *pctldev, unsigned int fselector,
+			   unsigned int group_id)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+	struct aml_pctl_group *group = &info->groups[group_id];
+	struct pinctrl_gpio_range *range;
+	int i;
+
+	for (i = 0; i < group->npins; i++) {
+		range =  pinctrl_find_gpio_range_from_pin(pctldev, group->pins[i]);
+		aml_pctl_set_function(info, range, group->pins[i], group->func[i]);
+	}
+
+	return 0;
+}
+
+static int aml_pmx_request_gpio(struct pinctrl_dev *pctldev,
+				struct pinctrl_gpio_range *range,
+				unsigned int pin)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return aml_pctl_set_function(info, range, pin, 0);
+}
+
+static const struct pinmux_ops aml_pmx_ops = {
+	.set_mux		= aml_pmx_set_mux,
+	.get_functions_count	= aml_pmx_get_funcs_count,
+	.get_function_name	= aml_pmx_get_fname,
+	.get_function_groups	= aml_pmx_get_groups,
+	.gpio_request_enable	= aml_pmx_request_gpio,
+};
+
+static int aml_calc_reg_and_bit(struct pinctrl_dev *pctldev,
+				unsigned int pin,
+				unsigned int reg_type,
+				unsigned int *reg, unsigned int *bit)
+{
+	struct pinctrl_gpio_range *range =
+			 pinctrl_find_gpio_range_from_pin(pctldev, pin);
+	struct aml_gpio_bank *bank = gpio_chip_to_bank(range->gc);
+
+	*bit = (pin - range->pin_base) * aml_bit_strides[reg_type]
+		+ bank->pc.bit_offset[reg_type];
+	*reg = (bank->pc.reg_offset[reg_type] + (*bit / 32)) * 4;
+	*bit &= 0x1f;
+
+	return 0;
+}
+
+static int aml_pinconf_get_pull(struct aml_pinctrl *info, unsigned int pin)
+{
+	unsigned int reg, bit, val;
+	int ret, conf;
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_PULLEN, &reg, &bit);
+
+	ret = regmap_read(info->reg_gpio, reg, &val);
+	if (ret)
+		return ret;
+
+	if (!(val & BIT(bit))) {
+		conf = PIN_CONFIG_BIAS_DISABLE;
+	} else {
+		aml_calc_reg_and_bit(info->pctl, pin, AML_REG_PULL, &reg, &bit);
+
+		ret = regmap_read(info->reg_gpio, reg, &val);
+		if (ret)
+			return ret;
+
+		if (val & BIT(bit))
+			conf = PIN_CONFIG_BIAS_PULL_UP;
+		else
+			conf = PIN_CONFIG_BIAS_PULL_DOWN;
+	}
+
+	return conf;
+}
+
+static int aml_pinconf_get_drive_strength(struct aml_pinctrl *info,
+					  unsigned int pin,
+					  u16 *drive_strength_ua)
+{
+	unsigned int reg, bit;
+	unsigned int val;
+	int ret;
+
+	if (!info->reg_ds)
+		return -EOPNOTSUPP;
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_DS, &reg, &bit);
+	ret = regmap_read(info->reg_ds, reg, &val);
+	if (ret)
+		return ret;
+
+	switch ((val >> bit) & 0x3) {
+	case PINCONF_DRV_500UA:
+		*drive_strength_ua = 500;
+		break;
+	case PINCONF_DRV_2500UA:
+		*drive_strength_ua = 2500;
+		break;
+	case PINCONF_DRV_3000UA:
+		*drive_strength_ua = 3000;
+		break;
+	case PINCONF_DRV_4000UA:
+		*drive_strength_ua = 4000;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int aml_pinconf_get_gpio_bit(struct aml_pinctrl *info,
+				    unsigned int pin,
+				    unsigned int reg_type)
+{
+	unsigned int reg, bit, val;
+	int ret;
+
+	aml_calc_reg_and_bit(info->pctl, pin, reg_type, &reg, &bit);
+	ret = regmap_read(info->reg_gpio, reg, &val);
+	if (ret)
+		return ret;
+
+	return BIT(bit) & val ? 1 : 0;
+}
+
+static int aml_pinconf_get_output(struct aml_pinctrl *info,
+				  unsigned int pin)
+{
+	int ret = aml_pinconf_get_gpio_bit(info, pin, AML_REG_DIR);
+
+	if (ret < 0)
+		return ret;
+
+	return !ret;
+}
+
+static int aml_pinconf_get_drive(struct aml_pinctrl *info,
+				 unsigned int pin)
+{
+	return aml_pinconf_get_gpio_bit(info, pin, AML_REG_OUT);
+}
+
+static int aml_pinconf_get(struct pinctrl_dev *pcdev, unsigned int pin,
+			   unsigned long *config)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pcdev);
+	enum pin_config_param param = pinconf_to_config_param(*config);
+	u16 arg;
+	int ret;
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (aml_pinconf_get_pull(info, pin) == param)
+			arg = 1;
+		else
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH_UA:
+		ret = aml_pinconf_get_drive_strength(info, pin, &arg);
+		if (ret)
+			return ret;
+		break;
+	case PIN_CONFIG_OUTPUT_ENABLE:
+		ret = aml_pinconf_get_output(info, pin);
+		if (ret <= 0)
+			return -EINVAL;
+		arg = 1;
+		break;
+	case PIN_CONFIG_OUTPUT:
+		ret = aml_pinconf_get_output(info, pin);
+		if (ret <= 0)
+			return -EINVAL;
+
+		ret = aml_pinconf_get_drive(info, pin);
+		if (ret < 0)
+			return -EINVAL;
+
+		arg = ret;
+		break;
+
+	default:
+		return -ENOTSUPP;
+	}
+
+	*config = pinconf_to_config_packed(param, arg);
+	dev_dbg(info->dev, "pinconf for pin %u is %lu\n", pin, *config);
+
+	return 0;
+}
+
+static int aml_pinconf_disable_bias(struct aml_pinctrl *info,
+				    unsigned int pin)
+{
+	unsigned int reg, bit = 0;
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_PULLEN, &reg, &bit);
+
+	return regmap_update_bits(info->reg_gpio, reg, BIT(bit), 0);
+}
+
+static int aml_pinconf_enable_bias(struct aml_pinctrl *info, unsigned int pin,
+				   bool pull_up)
+{
+	unsigned int reg, bit, val = 0;
+	int ret;
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_PULL, &reg, &bit);
+	if (pull_up)
+		val = BIT(bit);
+
+	ret = regmap_update_bits(info->reg_gpio, reg, BIT(bit), val);
+	if (ret)
+		return ret;
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_PULLEN, &reg, &bit);
+	return regmap_update_bits(info->reg_gpio, reg, BIT(bit), BIT(bit));
+}
+
+static int aml_pinconf_set_drive_strength(struct aml_pinctrl *info,
+					  unsigned int pin,
+					  u16 drive_strength_ua)
+{
+	unsigned int reg, bit, ds_val;
+
+	if (!info->reg_ds) {
+		dev_err(info->dev, "drive-strength not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	aml_calc_reg_and_bit(info->pctl, pin, AML_REG_DS, &reg, &bit);
+
+	if (drive_strength_ua <= 500) {
+		ds_val = PINCONF_DRV_500UA;
+	} else if (drive_strength_ua <= 2500) {
+		ds_val = PINCONF_DRV_2500UA;
+	} else if (drive_strength_ua <= 3000) {
+		ds_val = PINCONF_DRV_3000UA;
+	} else if (drive_strength_ua <= 4000) {
+		ds_val = PINCONF_DRV_4000UA;
+	} else {
+		dev_warn_once(info->dev,
+			      "pin %u: invalid drive-strength : %d , default to 4mA\n",
+			      pin, drive_strength_ua);
+		ds_val = PINCONF_DRV_4000UA;
+	}
+
+	return regmap_update_bits(info->reg_ds, reg, 0x3 << bit, ds_val << bit);
+}
+
+static int aml_pinconf_set_gpio_bit(struct aml_pinctrl *info,
+				    unsigned int pin,
+				    unsigned int reg_type,
+				    bool arg)
+{
+	unsigned int reg, bit;
+
+	aml_calc_reg_and_bit(info->pctl, pin, reg_type, &reg, &bit);
+	return regmap_update_bits(info->reg_gpio, reg, BIT(bit),
+				  arg ? BIT(bit) : 0);
+}
+
+static int aml_pinconf_set_output(struct aml_pinctrl *info,
+				  unsigned int pin,
+				  bool out)
+{
+	return aml_pinconf_set_gpio_bit(info, pin, AML_REG_DIR, !out);
+}
+
+static int aml_pinconf_set_drive(struct aml_pinctrl *info,
+				 unsigned int pin,
+				 bool high)
+{
+	return aml_pinconf_set_gpio_bit(info, pin, AML_REG_OUT, high);
+}
+
+static int aml_pinconf_set_output_drive(struct aml_pinctrl *info,
+					unsigned int pin,
+					bool high)
+{
+	int ret;
+
+	ret = aml_pinconf_set_output(info, pin, true);
+	if (ret)
+		return ret;
+
+	return aml_pinconf_set_drive(info, pin, high);
+}
+
+static int aml_pinconf_set(struct pinctrl_dev *pcdev, unsigned int pin,
+			   unsigned long *configs, unsigned int num_configs)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pcdev);
+	enum pin_config_param param;
+	unsigned int arg = 0;
+	int i, ret;
+
+	for (i = 0; i < num_configs; i++) {
+		param = pinconf_to_config_param(configs[i]);
+
+		switch (param) {
+		case PIN_CONFIG_DRIVE_STRENGTH_UA:
+		case PIN_CONFIG_OUTPUT_ENABLE:
+		case PIN_CONFIG_OUTPUT:
+			arg = pinconf_to_config_argument(configs[i]);
+			break;
+
+		default:
+			break;
+		}
+
+		switch (param) {
+		case PIN_CONFIG_BIAS_DISABLE:
+			ret = aml_pinconf_disable_bias(info, pin);
+			break;
+		case PIN_CONFIG_BIAS_PULL_UP:
+			ret = aml_pinconf_enable_bias(info, pin, true);
+			break;
+		case PIN_CONFIG_BIAS_PULL_DOWN:
+			ret = aml_pinconf_enable_bias(info, pin, false);
+			break;
+		case PIN_CONFIG_DRIVE_STRENGTH_UA:
+			ret = aml_pinconf_set_drive_strength(info, pin, arg);
+			break;
+		case PIN_CONFIG_OUTPUT_ENABLE:
+			ret = aml_pinconf_set_output(info, pin, arg);
+			break;
+		case PIN_CONFIG_OUTPUT:
+			ret = aml_pinconf_set_output_drive(info, pin, arg);
+			break;
+		default:
+			ret = -ENOTSUPP;
+		}
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int aml_pinconf_group_set(struct pinctrl_dev *pcdev,
+				 unsigned int num_group,
+				 unsigned long *configs,
+				 unsigned int num_configs)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pcdev);
+	int i;
+
+	for (i = 0; i < info->groups[num_group].npins; i++) {
+		aml_pinconf_set(pcdev, info->groups[num_group].pins[i], configs,
+				num_configs);
+	}
+
+	return 0;
+}
+
+static int aml_pinconf_group_get(struct pinctrl_dev *pcdev,
+				 unsigned int group, unsigned long *config)
+{
+	return -EOPNOTSUPP;
+}
+
+static const struct pinconf_ops aml_pinconf_ops = {
+	.pin_config_get		= aml_pinconf_get,
+	.pin_config_set		= aml_pinconf_set,
+	.pin_config_group_get	= aml_pinconf_group_get,
+	.pin_config_group_set	= aml_pinconf_group_set,
+	.is_generic		= true,
+};
+
+static int aml_get_groups_count(struct pinctrl_dev *pctldev)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->ngroups;
+}
+
+static const char *aml_get_group_name(struct pinctrl_dev *pctldev,
+				      unsigned int selector)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	return info->groups[selector].name;
+}
+
+static int aml_get_group_pins(struct pinctrl_dev *pctldev,
+			      unsigned int selector, const unsigned int **pins,
+			      unsigned int *npins)
+{
+	struct aml_pinctrl *info = pinctrl_dev_get_drvdata(pctldev);
+
+	if (selector >= info->ngroups)
+		return -EINVAL;
+
+	*pins = info->groups[selector].pins;
+	*npins = info->groups[selector].npins;
+
+	return 0;
+}
+
+static inline const struct aml_pctl_group *
+	aml_pctl_find_group_by_name(const struct aml_pinctrl *info,
+				    const char *name)
+{
+	int i;
+
+	for (i = 0; i < info->ngroups; i++) {
+		if (!strcmp(info->groups[i].name, name))
+			return &info->groups[i];
+	}
+
+	return NULL;
+}
+
+static void aml_pin_dbg_show(struct pinctrl_dev *pcdev, struct seq_file *s,
+			     unsigned int offset)
+{
+	seq_printf(s, " %s", dev_name(pcdev->dev));
+}
+
+static const struct pinctrl_ops aml_pctrl_ops = {
+	.get_groups_count	= aml_get_groups_count,
+	.get_group_name		= aml_get_group_name,
+	.get_group_pins		= aml_get_group_pins,
+	.dt_node_to_map		= pinconf_generic_dt_node_to_map_pinmux,
+	.dt_free_map		= pinconf_generic_dt_free_map,
+	.pin_dbg_show		= aml_pin_dbg_show,
+};
+
+static int aml_pctl_parse_functions(struct device_node *np,
+				    struct aml_pinctrl *info, u32 index,
+				    int *grp_index)
+{
+	struct device *dev = info->dev;
+	struct aml_pmx_func *func;
+	struct aml_pctl_group *grp;
+	int ret, i;
+
+	func = &info->functions[index];
+	func->name = np->name;
+	func->ngroups = of_get_child_count(np);
+	if (func->ngroups == 0)
+		return dev_err_probe(dev, -EINVAL, "No groups defined\n");
+
+	func->groups = devm_kcalloc(dev, func->ngroups, sizeof(*func->groups), GFP_KERNEL);
+	if (!func->groups)
+		return -ENOMEM;
+
+	i = 0;
+	for_each_child_of_node_scoped(np, child) {
+		func->groups[i++] = child->name;
+		grp = &info->groups[*grp_index];
+		grp->name = child->name;
+		*grp_index += 1;
+		ret = pinconf_generic_parse_dt_pinmux(child, dev, &grp->pins,
+						      &grp->func, &grp->npins);
+		if (ret) {
+			dev_err(dev, "function :%s, groups:%s fail\n", func->name, child->name);
+			return ret;
+		}
+	}
+	dev_dbg(dev, "Function[%d\t name:%s,\tgroups:%d]\n", index, func->name, func->ngroups);
+
+	return 0;
+}
+
+static u32 aml_bank_pins(struct device_node *np)
+{
+	u32 value;
+
+	if (of_property_read_u32(np, "ngpios", &value) < 0)
+		return 0;
+	else
+		return value;
+}
+
+static unsigned int aml_count_pins(struct device_node *np)
+{
+	struct device_node *child;
+	unsigned int pins = 0;
+
+	for_each_child_of_node(np, child) {
+		if (of_property_read_bool(child, "gpio-controller"))
+			pins += aml_bank_pins(child);
+	}
+
+	return pins;
+}
+
+/*
+ * A pinctrl device contains two types of nodes. The one named GPIO
+ * bank which includes gpio-controller property. The other one named
+ * function which includes one or more pin groups. The pin group
+ * include pinmux property(global index in pinctrl dev, and mux vlaue
+ * in mux reg) and pin configuration properties.
+ */
+static void aml_pctl_dt_child_count(struct aml_pinctrl *info,
+				    struct device_node *np)
+{
+	struct device_node *child;
+
+	for_each_child_of_node(np, child) {
+		if (of_property_read_bool(child, "gpio-controller")) {
+			info->nbanks++;
+		} else {
+			info->nfunctions++;
+			info->ngroups += of_get_child_count(child);
+		}
+	}
+}
+
+static struct regmap_config aml_regmap_config = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+};
+
+static struct regmap *aml_map_resource(struct platform_device *pdev,
+				       struct device_node *node, char *name)
+{
+	struct resource *res;
+	void __iomem *base;
+	int i;
+
+	i = of_property_match_string(node, "reg-names", name);
+	if (i < 0)
+		return ERR_PTR(i);
+
+	base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
+	if (IS_ERR(base))
+		return ERR_CAST(base);
+
+	aml_regmap_config.max_register = resource_size(res) - 4;
+	aml_regmap_config.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+						"%pOFn-%s", node,
+						name);
+	if (!aml_regmap_config.name)
+		return ERR_PTR(-ENOMEM);
+
+	return devm_regmap_init_mmio(&pdev->dev, base, &aml_regmap_config);
+}
+
+static inline int aml_gpio_calc_reg_and_bit(struct aml_gpio_bank *bank,
+					    unsigned int reg_type,
+					    unsigned int gpio,
+					    unsigned int *reg,
+					    unsigned int *bit)
+{
+	*bit = gpio * aml_bit_strides[reg_type] + bank->pc.bit_offset[reg_type];
+	*reg = (bank->pc.reg_offset[reg_type] + (*bit / 32)) * 4;
+	*bit &= 0x1f;
+
+	return 0;
+}
+
+static int aml_gpio_get_direction(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct aml_gpio_bank *bank = gpiochip_get_data(chip);
+	struct aml_pinctrl *info = dev_get_drvdata(chip->parent);
+	unsigned int bit, reg, val;
+	int ret;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_DIR, gpio, &reg, &bit);
+
+	ret = regmap_read(info->reg_gpio, reg, &val);
+	if (ret)
+		return ret;
+
+	return BIT(bit) & val ? GPIO_LINE_DIRECTION_OUT : GPIO_LINE_DIRECTION_IN;
+}
+
+static int aml_gpio_direction_input(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct aml_gpio_bank *bank = gpiochip_get_data(chip);
+	struct aml_pinctrl *info = dev_get_drvdata(chip->parent);
+	unsigned int bit, reg;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_DIR, gpio, &reg, &bit);
+
+	return regmap_update_bits(info->reg_gpio, reg, BIT(bit), 0);
+}
+
+static int aml_gpio_direction_output(struct gpio_chip *chip, unsigned int gpio,
+				     int value)
+{
+	struct aml_gpio_bank *bank = gpiochip_get_data(chip);
+	struct aml_pinctrl *info = dev_get_drvdata(chip->parent);
+	unsigned int bit, reg;
+	int ret;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_DIR, gpio, &reg, &bit);
+	ret = regmap_update_bits(info->reg_gpio, reg, BIT(bit), BIT(bit));
+	if (ret < 0)
+		return ret;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_OUT, gpio, &reg, &bit);
+
+	return regmap_update_bits(info->reg_gpio, reg, BIT(bit),
+				  value ? BIT(bit) : 0);
+}
+
+static void aml_gpio_set(struct gpio_chip *chip, unsigned int gpio, int value)
+{
+	struct aml_gpio_bank *bank = gpiochip_get_data(chip);
+	struct aml_pinctrl *info = dev_get_drvdata(chip->parent);
+	unsigned int bit, reg;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_OUT, gpio, &reg, &bit);
+
+	regmap_update_bits(info->reg_gpio, reg, BIT(bit),
+			   value ? BIT(bit) : 0);
+}
+
+static int aml_gpio_get(struct gpio_chip *chip, unsigned int gpio)
+{
+	struct aml_gpio_bank *bank = gpiochip_get_data(chip);
+	struct aml_pinctrl *info = dev_get_drvdata(chip->parent);
+	unsigned int reg, bit, val;
+
+	aml_gpio_calc_reg_and_bit(bank, AML_REG_IN, gpio, &reg, &bit);
+	regmap_read(info->reg_gpio, reg, &val);
+
+	return !!(val & BIT(bit));
+}
+
+static const struct gpio_chip aml_gpio_template = {
+	.request		= gpiochip_generic_request,
+	.free			= gpiochip_generic_free,
+	.set_config		= gpiochip_generic_config,
+	.set			= aml_gpio_set,
+	.get			= aml_gpio_get,
+	.direction_input	= aml_gpio_direction_input,
+	.direction_output	= aml_gpio_direction_output,
+	.get_direction		= aml_gpio_get_direction,
+	.can_sleep		= false,
+};
+
+static void init_bank_register_bit(struct aml_pinctrl *info,
+				   struct aml_gpio_bank *bank)
+{
+	const struct aml_pctl_data *data = info->data;
+	const struct aml_reg_bit *aml_rb;
+	bool def_offs = true;
+	int i;
+
+	if (data) {
+		for (i = 0; i < data->number; i++) {
+			aml_rb = &data->rb_offs[i];
+			if (bank->bank_id == aml_rb->bank_id) {
+				def_offs = false;
+				break;
+			}
+		}
+	};
+
+	if (def_offs) {
+		for (i = 0; i < AML_NUM_REG; i++) {
+			bank->pc.reg_offset[i] = bank->pc.gpio_offset + aml_def_regoffs[i];
+			bank->pc.bit_offset[i] = 0;
+		}
+	} else {
+		for (i = 0; i < AML_NUM_REG; i++) {
+			bank->pc.reg_offset[i] = bank->pc.gpio_offset + aml_rb->reg_offs[i];
+			bank->pc.bit_offset[i] = aml_rb->bit_offs[i];
+		}
+	}
+}
+
+static int aml_gpiolib_register_bank(struct aml_pinctrl *info,
+				     int bank_nr, struct device_node *np)
+{
+	struct aml_gpio_bank *bank = &info->banks[bank_nr];
+	struct device *dev = info->dev;
+	int err;
+
+	if (of_property_read_u32_index(np, "reg", 0, &bank->pc.gpio_offset) &&
+	    of_property_read_u32_index(np, "reg", 1, &bank->pc.mux_offset)) {
+		dev_err(dev, "get num=%d bank reg fail.\n", bank_nr);
+		return -EINVAL;
+	}
+
+	if (of_property_read_u32(np, "mask", &bank->pc.mux_mask))
+		bank->pc.mux_mask = 0xf;
+
+	bank->gpio_chip = aml_gpio_template;
+	bank->gpio_chip.base = -1;
+	bank->gpio_chip.ngpio = aml_bank_pins(np);
+	bank->gpio_chip.fwnode = of_fwnode_handle(np);
+	bank->gpio_chip.parent = dev;
+
+	if (of_property_read_u32(np, "identity", &bank->bank_id)) {
+		dev_err(dev, "get num=%d bank identity fail\n", bank_nr);
+		return -EINVAL;
+	}
+
+	init_bank_register_bit(info, bank);
+	bank->gpio_chip.label = aml_bank_name[bank->bank_id];
+
+	bank->pin_base = bank->bank_id << 8;
+
+	err  = gpiochip_add_data(&bank->gpio_chip, bank);
+	if (err)
+		return dev_err_probe(dev, err, "Failed to add gpiochip(%d)!\n", bank_nr);
+
+	dev_dbg(dev, "%s bank added.\n", bank->gpio_chip.label);
+
+	return 0;
+}
+
+static int aml_pctl_probe_dt(struct platform_device *pdev,
+			     struct pinctrl_desc *pctl_desc,
+			     struct aml_pinctrl *info)
+{
+	struct device *dev = &pdev->dev;
+	struct pinctrl_pin_desc *pdesc;
+	struct device_node *np = dev->of_node;
+	int grp_index = 0;
+	int i = 0, j = 0, k = 0, bank;
+	int ret = 0;
+
+	aml_pctl_dt_child_count(info, np);
+	if (!info->nbanks)
+		return dev_err_probe(dev, -EINVAL, "you need at least one gpio bank\n");
+
+	dev_dbg(dev, "nbanks = %d\n", info->nbanks);
+	dev_dbg(dev, "nfunctions = %d\n", info->nfunctions);
+	dev_dbg(dev, "ngroups = %d\n", info->ngroups);
+
+	info->functions = devm_kcalloc(dev, info->nfunctions, sizeof(*info->functions), GFP_KERNEL);
+
+	info->groups = devm_kcalloc(dev, info->ngroups, sizeof(*info->groups), GFP_KERNEL);
+
+	info->banks = devm_kcalloc(dev, info->nbanks, sizeof(*info->banks), GFP_KERNEL);
+
+	if (!info->functions || !info->groups || !info->banks)
+		return -ENOMEM;
+
+	info->reg_mux = aml_map_resource(pdev, np, "mux");
+	if (IS_ERR_OR_NULL(info->reg_mux))
+		return dev_err_probe(dev, info->reg_mux ? PTR_ERR(info->reg_mux) : -ENOENT,
+				     "mux registers not found\n");
+
+	info->reg_gpio = aml_map_resource(pdev, np, "gpio");
+	if (IS_ERR_OR_NULL(info->reg_gpio))
+		return dev_err_probe(dev, info->reg_gpio ? PTR_ERR(info->reg_gpio) : -ENOENT,
+				     "gpio registers not found\n");
+
+	info->reg_ds = aml_map_resource(pdev, np, "ds");
+	if (IS_ERR(info->reg_ds)) {
+		dev_dbg(info->dev, "ds registers not found - skipping\n");
+		info->reg_ds = info->reg_gpio;
+	}
+
+	info->reg_ds = info->reg_gpio;
+
+	info->data = (struct aml_pctl_data *)of_device_get_match_data(dev);
+
+	pctl_desc->npins = aml_count_pins(np);
+
+	pdesc =	devm_kcalloc(dev, pctl_desc->npins, sizeof(*pdesc), GFP_KERNEL);
+	if (!pdesc)
+		return -ENOMEM;
+
+	pctl_desc->pins = pdesc;
+
+	bank = 0;
+	for_each_child_of_node_scoped(np, child) {
+		if (of_property_read_bool(child, "gpio-controller")) {
+			const char *bank_name = NULL;
+			char **pin_names;
+
+			ret = aml_gpiolib_register_bank(info, bank, child);
+			if (ret)
+				return ret;
+
+			k = info->banks[bank].pin_base;
+			bank_name = info->banks[bank].gpio_chip.label;
+
+			pin_names = devm_kasprintf_strarray(dev, bank_name,
+							    info->banks[bank].gpio_chip.ngpio);
+			if (IS_ERR(pin_names))
+				return PTR_ERR(pin_names);
+
+			for (j = 0; j < info->banks[bank].gpio_chip.ngpio; j++, k++) {
+				pdesc->number = k;
+				pdesc->name = pin_names[j];
+				pdesc++;
+			}
+			bank++;
+		} else {
+			ret = aml_pctl_parse_functions(child, info,
+						       i++, &grp_index);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int aml_pctl_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct aml_pinctrl *info;
+	struct pinctrl_desc *pctl_desc;
+	int ret, i;
+
+	pctl_desc = devm_kzalloc(dev, sizeof(*pctl_desc), GFP_KERNEL);
+	if (!pctl_desc)
+		return -ENOMEM;
+
+	info = devm_kzalloc(dev, sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->dev = dev;
+	platform_set_drvdata(pdev, info);
+	ret = aml_pctl_probe_dt(pdev, pctl_desc, info);
+	if (ret)
+		return ret;
+
+	pctl_desc->owner	= THIS_MODULE;
+	pctl_desc->pctlops	= &aml_pctrl_ops;
+	pctl_desc->pmxops	= &aml_pmx_ops;
+	pctl_desc->confops	= &aml_pinconf_ops;
+	pctl_desc->name		= dev_name(dev);
+
+	info->pctl = devm_pinctrl_register(dev, pctl_desc, info);
+	if (IS_ERR(info->pctl)) {
+		dev_err(dev, "Failed pinctrl registration\n");
+		ret = PTR_ERR(info->pctl);
+		goto error;
+	}
+
+	for (i = 0; i < info->nbanks; i++) {
+		ret = gpiochip_add_pin_range(&info->banks[i].gpio_chip, dev_name(dev), 0,
+					     info->banks[i].pin_base,
+					     info->banks[i].gpio_chip.ngpio);
+		if (ret < 0) {
+			dev_err(dev, "Failed add pin range\n");
+			goto error;
+		}
+	}
+
+	return 0;
+error:
+	for (i = 0; i < info->nbanks; i++)
+		gpiochip_remove(&info->banks[i].gpio_chip);
+	return ret;
+}
+
+static const struct of_device_id aml_pctl_of_match[] = {
+	{ .compatible = "amlogic,pinctrl-a4", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, aml_pctl_dt_match);
+
+static struct platform_driver aml_pctl_driver = {
+	.driver = {
+		.name = "amlogic-pinctrl",
+		.of_match_table = aml_pctl_of_match,
+	},
+	.probe = aml_pctl_probe,
+};
+module_platform_driver(aml_pctl_driver);
+
+MODULE_AUTHOR("Xianwei Zhao <xianwei.zhao@amlogic.com>");
+MODULE_DESCRIPTION("Pin controller and GPIO driver for Amlogic SoC");
+MODULE_LICENSE("Dual BSD/GPL");

-- 
2.37.1

[PATCH v2 4/5] arm64: dts: amlogic: a4: add pinctrl node

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add pinctrl device to support Amlogic A4 and add uart pinconf.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi | 133 ++++++++++++++++++++++++++++
 1 file changed, 133 insertions(+)

diff --git a/arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi
index de10e7aebf21..90ef74b015bd 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-a4.dtsi
@@ -5,6 +5,7 @@
 
 #include "amlogic-a4-common.dtsi"
 #include <dt-bindings/power/amlogic,a4-pwrc.h>
+#include <dt-bindings/pinctrl/amlogic,pinctrl.h>
 / {
 	cpus {
 		#address-cells = <2>;
@@ -48,3 +49,135 @@ pwrc: power-controller {
 		};
 	};
 };
+
+&apb {
+	periphs_pinctrl: pinctrl@4000 {
+		compatible = "amlogic,pinctrl-a4";
+		reg = <0x0 0x4000 0x0 0x0050>,
+		      <0x0 0x40c0 0x0 0x0220>;
+		reg-names = "mux", "gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		gpiox: gpio@10 {
+			reg = <0x10>, <0x3>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <18>;
+			identity = <AMLOGIC_GPIO_X>;
+		};
+
+		gpiot: gpio@20 {
+			reg = <0x20>, <0xb>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <23>;
+			identity =  <AMLOGIC_GPIO_T>;
+		};
+
+		gpiod: gpio@30 {
+			reg = <0x30>, <0x10>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <16>;
+			identity = <AMLOGIC_GPIO_D>;
+		};
+
+		gpioe: gpio@40 {
+			reg = <0x40>, <0x12>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <2>;
+			identity = <AMLOGIC_GPIO_E>;
+		};
+
+		gpiob: gpio@60 {
+			reg = <0x60>, <0x0>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <14>;
+			identity = <AMLOGIC_GPIO_B>;
+		};
+
+		func-uart-a {
+			uart_a_default: uart-a-pins1 {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_X, 11, AF1)>,
+					<AML_PINMUX(AMLOGIC_GPIO_X, 12, AF1)>,
+					<AML_PINMUX(AMLOGIC_GPIO_X, 13, AF1)>,
+					<AML_PINMUX(AMLOGIC_GPIO_X, 14, AF1)>;
+			};
+
+			uart-a-pins2 {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_D, 2, AF3)>,
+					<AML_PINMUX(AMLOGIC_GPIO_D, 3, AF3)>;
+				bias-pull-up;
+				drive-strength-microamp = <4000>;
+			};
+		};
+
+		func-uart-b {
+			uart_b_default: uart-b-pins {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_E, 0, AF3)>,
+					<AML_PINMUX(AMLOGIC_GPIO_E, 1, AF3)>;
+				bias-pull-up;
+				drive-strength-microamp = <4000>;
+			};
+		};
+
+		func-uart-d {
+			uart_d_default: uart-d-pins1 {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_T, 18, AF4)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 19, AF4)>;
+				bias-pull-up;
+				drive-strength-microamp = <4000>;
+			};
+
+			uart-d-pins2 {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_T, 7, AF2)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 8, AF2)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 9, AF2)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 10, AF2)>;
+				bias-pull-up;
+				drive-strength-microamp = <4000>;
+			};
+		};
+
+		func-uart-e {
+			uart_e_default: uart-e-pins {
+				pinmux= <AML_PINMUX(AMLOGIC_GPIO_T, 14, AF3)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 15, AF3)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 16, AF3)>,
+					<AML_PINMUX(AMLOGIC_GPIO_T, 17, AF3)>;
+				bias-pull-up;
+				drive-strength-microamp = <4000>;
+			};
+		};
+	};
+
+	aobus_pinctrl: pinctrl@8e700 {
+		compatible = "amlogic,pinctrl-a4";
+		reg = <0x0 0x8e700 0x0 0x04>,
+		      <0x0 0x8e704 0x0 0x60>;
+		reg-names = "mux", "gpio";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		gpioao: gpio@0 {
+			reg = <0x0>, <0x0>;
+			mask = <0xfffffff>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <7>;
+			identity = <AMLOGIC_GPIO_AO>;
+		};
+
+		test_n: gpio@10 {
+			reg = <0x10>, <0x0>;
+			mask = <0xf0000000>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			ngpios = <1>;
+			identity = <AMLOGIC_GPIO_TEST_N>;
+		};
+	};
+};

-- 
2.37.1

[PATCH v2 5/5] MAINTAINERS: Add an entry for Amlogic pinctrl driver

[Xianwei Zhao via B4 Relay]

From: Xianwei Zhao <xianwei.zhao@amlogic.com>

Add Amlogic pinctrl entry to MAINTAINERS to clarify the maintainers.

Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 1e930c7a58b1..b8905e8aa802 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1218,6 +1218,14 @@ F:	Documentation/devicetree/bindings/perf/amlogic,g12-ddr-pmu.yaml
 F:	drivers/perf/amlogic/
 F:	include/soc/amlogic/
 
+AMLOGIC PINCTRL DRIVER
+M:	Xianwei Zhao <xianwei.zhao@amlogic.com>
+L:	linux-amlogic@lists.infradead.org
+L:	linux-gpio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
+F:	drivers/pinctrl/pinctrl-amlogic.c
+
 AMLOGIC RTC DRIVER
 M:	Yiting Deng <yiting.deng@amlogic.com>
 M:	Xianwei Zhao <xianwei.zhao@amlogic.com>

-- 
2.37.1

Re: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[kernel test robot]

Hi Xianwei,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 4de5110762b94b9978fb8182a568572fb2194f8b]

url:    https://github.com/intel-lab-lkp/linux/commits/Xianwei-Zhao-via-B4-Relay/dt-bindings-pinctrl-Add-support-for-Amlogic-SoCs/20241226-155844
base:   4de5110762b94b9978fb8182a568572fb2194f8b
patch link:    https://lore.kernel.org/r/20241226-amlogic-pinctrl-v2-2-cdae42a67b76%40amlogic.com
patch subject: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file
config: arc-randconfig-001-20241226 (https://download.01.org/0day-ci/archive/20241226/202412261752.6HK0iJXu-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241226/202412261752.6HK0iJXu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412261752.6HK0iJXu-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pinctrl/pinconf-generic.c:250: warning: Function parameter or struct member 'dev' not described in 'pinconf_generic_parse_dt_pinmux'
>> drivers/pinctrl/pinconf-generic.c:250: warning: Excess function parameter 'pctldev' description in 'pinconf_generic_parse_dt_pinmux'


vim +250 drivers/pinctrl/pinconf-generic.c

   235	
   236	/**
   237	 * pinconf_generic_parse_dt_pinmux()
   238	 * parse the pinmux properties into generic pin mux values.
   239	 * @np: node containing the pinmux properties
   240	 * @pctldev: pincontrol device
   241	 * @pid: array with pin identity entries
   242	 * @pmux: array with pin mux value entries
   243	 * @npins: number of pins
   244	 *
   245	 * pinmux propertity: mux value [0,7]bits and pin identity [8,31]bits.
   246	 */
   247	int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
   248					    unsigned int **pid, unsigned int **pmux,
   249					    unsigned int *npins)
 > 250	{
   251		unsigned int *pid_t;
   252		unsigned int *pmux_t;
   253		struct property *prop;
   254		unsigned int npins_t, i;
   255		u32 value;
   256		int ret;
   257	
   258		prop = of_find_property(np, "pinmux", NULL);
   259		if (!prop) {
   260			dev_info(dev, "Missing pinmux property\n");
   261			return -ENOENT;
   262		}
   263	
   264		if (!pid || !pmux || !npins) {
   265			dev_err(dev, "paramers error\n");
   266			return -EINVAL;
   267		}
   268	
   269		npins_t = prop->length / sizeof(u32);
   270		pid_t = devm_kcalloc(dev, npins_t, sizeof(*pid_t), GFP_KERNEL);
   271		pmux_t = devm_kcalloc(dev, npins_t, sizeof(*pmux_t), GFP_KERNEL);
   272		if (!pid_t || !pmux_t) {
   273			dev_err(dev, "kalloc memory fail\n");
   274			return -ENOMEM;
   275		}
   276		for (i = 0; i < npins_t; i++) {
   277			ret = of_property_read_u32_index(np, "pinmux", i, &value);
   278			if (ret) {
   279				dev_err(dev, "get pinmux value fail\n");
   280				goto exit;
   281			}
   282			pmux_t[i] = value & 0xff;
   283			pid_t[i] = (value >> 8) & 0xffffff;
   284		}
   285		*pid = pid_t;
   286		*pmux = pmux_t;
   287		*npins = npins_t;
   288	
   289		return 0;
   290	exit:
   291		devm_kfree(dev, pid_t);
   292		devm_kfree(dev, pmux_t);
   293		return ret;
   294	}
   295	EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_pinmux);
   296	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 3/5] pinctrl: Add driver support for Amlogic SoCs

[Martin Blumenstingl]

Hi Xianwei,

On Thu, Dec 26, 2024 at 8:57 AM Xianwei Zhao via B4 Relay
<devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
[...]
> +       if (of_property_read_u32(np, "mask", &bank->pc.mux_mask))
> +               bank->pc.mux_mask = 0xf;
I don't see any documentation for the mask property.
The discussion about the dt-bindings is still pretty heated.
Everything else is already part of the documentation (which is good!).
Please document his property as well (or remove the code to read it if
there's no need to have it dynamic).

This is not a full code-review. However, since the discussion about
the whole dt-bindings is still ongoing we need to make sure that
whatever we document in the bindings is actually all that we need to
write the driver.


Best regards,
Martin

Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add support for Amlogic SoCs

[Rob Herring]

On Thu, Dec 26, 2024 at 03:57:41PM +0800, Xianwei Zhao wrote:
> Add the dt-bindings for Amlogic pin controller, and add a new
> dt-binding header file which document the GPIO bank names and
> alternative func value of all Amlogic subsequent SoCs.
> 
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>  .../bindings/pinctrl/amlogic,pinctrl-a4.yaml       | 155 +++++++++++++++++++++
>  include/dt-bindings/pinctrl/amlogic,pinctrl.h      |  68 +++++++++
>  2 files changed, 223 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
> new file mode 100644
> index 000000000000..75863d179933
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
> @@ -0,0 +1,155 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl-a4.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic pinmux controller
> +
> +maintainers:
> +  - Xianwei Zhao <xianwei.zhao@amlogic.com>
> +
> +allOf:
> +  - $ref: pinctrl.yaml#
> +
> +properties:
> +  compatible:
> +    const: amlogic,pinctrl-a4
> +
> +  reg:
> +    minItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: mux
> +      - const: gpio
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0

The addresses in 'reg' below are MMIO, right? If so, #size-cells can't 
be 0 and you also need 'ranges' here.

> +
> +required:

Move after 'patternProperties'.

> +  - compatible
> +  - reg
> +  - reg-names
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +patternProperties:
> +  "^gpio@[0-9a-f]+$":
> +    type: object
> +
> +    properties:
> +      reg:
> +        minItems: 2

You need to describe what each entry is:

items:
  - description: foo
  - description: bar

> +
> +      mask:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +      gpio-controller: true
> +
> +      "#gpio-cells":
> +        const: 2
> +
> +      ngpios:
> +        $ref: /schemas/types.yaml#/definitions/uint32

Don't need a type here.

> +        minimum: 1
> +        maximum: 32
> +
> +      identity:

Needs a better name. This is the bank number?

But shouldn't you be able to use gpio-ranges here:

gpio-ranges = <&periphs_pinctrl 0 (bank# << 8) (value of npgios)>;

And do you really need ngpios if you have the value here?

> +        description: |
> +          identifier are provided by the pin controller header file at:
> +          <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
> +
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +
> +    required:
> +      - reg
> +      - gpio-controller
> +      - "#gpio-cells"
> +      - ngpios
> +      - identity
> +
> +    additionalProperties: false
> +
> +  "^func-[0-9a-z-]+$":
> +    type: object
> +    additionalProperties:

Define a pattern for the node names instead. We only allow anything for 
existing bindings.

> +      type: object
> +      allOf:
> +        - $ref: pincfg-node.yaml#
> +        - $ref: pinmux-node.yaml#
> +
> +      description:
> +        A pin multiplexing sub-node describes how to configure a set of (or a
> +        single) pin in some desired alternate function mode.
> +        A single sub-node may define several pin configurations.
> +
> +      properties:
> +        pinmux:
> +          description: |
> +            Integer array representing pin number and pin multiplexing
> +            configuration.
> +            When a pin has to be configured in alternate function mode, use
> +            this property to identify the pin by its global index, and provide
> +            its alternate function configuration number along with it.
> +            bank identifier are provided by the pin controller header file at:
> +            <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
> +            Integers values in "pinmux" argument list are assembled as:
> +            (((BANK << 8) + PIN) << 8)  | MUX_FUNC))
> +
> +      required:
> +        - pinmux
> +
> +      additionalProperties: true

No, not allowed to be true except on common schemas.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
> +    apb {
> +      #address-cells = <2>;
> +      #size-cells = <2>;
> +      periphs_pinctrl: pinctrl@8e700 {
> +        compatible = "amlogic,pinctrl-a4";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        reg = <0x0 0x8e700 0x0 0x04>,
> +              <0x0 0x8e704 0x0 0x60>;
> +        reg-names = "mux", "gpio";
> +
> +        gpio@14 {
> +          reg = <0x14>,<0x30>;
> +          gpio-controller;
> +          #gpio-cells = <2>;
> +          ngpios = <10>;
> +          identity = <AMLOGIC_GPIO_B>;
> +        };
> +
> +        func-uart-b {
> +          uart-b-default {
> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 1, AF4)>;
> +            bias-pull-up;
> +            drive-strength-microamp = <4000>;
> +          };
> +
> +          uart-b-pins1 {
> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 5, AF2)>;
> +            bias-pull-up;
> +            drive-strength-microamp = <4000>;
> +          };
> +        };
> +
> +        func-uart-c {
> +          uart-c-default {
> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 3, AF1)>,
> +                     <AML_PINMUX(AMLOGIC_GPIO_B, 2, AF1)>;
> +            bias-pull-up;
> +            drive-strength-microamp = <4000>;
> +          };
> +        };
> +      };
> +    };
> diff --git a/include/dt-bindings/pinctrl/amlogic,pinctrl.h b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
> new file mode 100644
> index 000000000000..03db0a730e8b
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
> +/*
> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
> + * Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
> + */
> +
> +#ifndef _DT_BINDINGS_AMLOGIC_PINCTRL_H
> +#define _DT_BINDINGS_AMLOGIC_PINCTRL_H
> +
> +/* define PIN modes */
> +#define AF0	0x0
> +#define AF1	0x1
> +#define AF2	0x2
> +#define AF3	0x3
> +#define AF4	0x4
> +#define AF5	0x5
> +#define AF6	0x6
> +#define AF7	0x7
> +#define AF8	0x8
> +#define AF9	0x9
> +#define AF10	0xa
> +#define AF11	0xb
> +#define AF12	0xc
> +#define AF13	0xd
> +#define AF14	0xe
> +#define AF15	0xf

There's no need for defines in the form "FOOn n".

> +
> +#define AML_PIN_ALT_FUNC_MASK	0xf
> +
> +/* Normal PIN bank */
> +#define AMLOGIC_GPIO_A		0
> +#define AMLOGIC_GPIO_B		1
> +#define AMLOGIC_GPIO_C		2
> +#define AMLOGIC_GPIO_D		3
> +#define AMLOGIC_GPIO_E		4
> +#define AMLOGIC_GPIO_F		5
> +#define AMLOGIC_GPIO_G		6
> +#define AMLOGIC_GPIO_H		7
> +#define AMLOGIC_GPIO_I		8
> +#define AMLOGIC_GPIO_J		9
> +#define AMLOGIC_GPIO_K		10
> +#define AMLOGIC_GPIO_L		11
> +#define AMLOGIC_GPIO_M		12
> +#define AMLOGIC_GPIO_N		13
> +#define AMLOGIC_GPIO_O		14
> +#define AMLOGIC_GPIO_P		15
> +#define AMLOGIC_GPIO_Q		16
> +#define AMLOGIC_GPIO_R		17
> +#define AMLOGIC_GPIO_S		18
> +#define AMLOGIC_GPIO_T		19
> +#define AMLOGIC_GPIO_U		20
> +#define AMLOGIC_GPIO_V		21
> +#define AMLOGIC_GPIO_W		22
> +#define AMLOGIC_GPIO_X		23
> +#define AMLOGIC_GPIO_Y		24
> +#define AMLOGIC_GPIO_Z		25
> +
> +/* Special PIN bank */
> +#define AMLOGIC_GPIO_DV		26
> +#define AMLOGIC_GPIO_AO		27
> +#define AMLOGIC_GPIO_CC		28
> +#define AMLOGIC_GPIO_TEST_N	29
> +
> +#define AML_PINMUX(bank, offset, mode)	(((((bank) << 8) + (offset)) << 8) | (mode))
> +#define AML_PINMUX_TO_BANK(pinmux)	(((pinmux) >> 16) & 0xff)
> +#define AML_PINMUX_TO_OFFSET(pinmux)	(((pinmux) >> 8) & 0xff)
> +
> +#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */
> 
> -- 
> 2.37.1
> 

Re: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[Linus Walleij]

On Thu, Dec 26, 2024 at 8:57 AM Xianwei Zhao via B4 Relay
<devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:

> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>
> When describing pin mux func through pinmux propertity,
> a standard API is added for support. The pinmux contains pin
> identification and mux values, which can include multiple
> pins. And groups configuration use other word. DTS such as:
>
> func-name {
>         group_alias: group-name{
>                 pinmux= <pin_id << 8 | mux_value)>,
>                         <pin_id << 8 | mux_value)>;
>                 bias-pull-up;
>                 drive-strength-microamp = <4000>;
>         };
> };
>
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>

There are some build errors, but I really like the path taken with
the utility function here!

Yours,
Linus Walleij

Re: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[Jerome Brunet]

On Thu 26 Dec 2024 at 15:57, Xianwei Zhao via B4 Relay <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:

> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>
> When describing pin mux func through pinmux propertity,
> a standard API is added for support. The pinmux contains pin
> identification and mux values, which can include multiple
> pins. And groups configuration use other word. DTS such as:
>
> func-name {
> 	group_alias: group-name{
> 		pinmux= <pin_id << 8 | mux_value)>,
> 			<pin_id << 8 | mux_value)>;

This representation does not seem very generic but more tailored to
your use-case.

> 		bias-pull-up;
> 		drive-strength-microamp = <4000>;

If you want to add pinmux (aka alternate function) selection as a
pinconf prop then I think there should be a single pinmux setting per
group, and as many groups as you need per function defined.

something like

func-foo {
	group-a {
        	groups = "pin_a", "pin_b";
                bias-pull-up;
                alternate-function = <2>;
        };

	group-b {
        	groups = "pin_c"";
                bias-disable;
                alternate-function = <5>;
        };
};

Something similar is already done to handle different pin bias requirement
on single function on amlogic platforms:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-axg.dtsi?h=v6.13-rc6#n421


> 	};
> };
>
> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> ---
>  drivers/pinctrl/pinconf-generic.c       | 130 ++++++++++++++++++++++++++++++++
>  drivers/pinctrl/pinconf.h               |   4 +
>  include/linux/pinctrl/pinconf-generic.h |   4 +
>  3 files changed, 138 insertions(+)
>
> diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
> index 0b13d7f17b32..a4d3c12a80c4 100644
> --- a/drivers/pinctrl/pinconf-generic.c
> +++ b/drivers/pinctrl/pinconf-generic.c
> @@ -233,6 +233,67 @@ static void parse_dt_cfg(struct device_node *np,
>  	}
>  }
>  
> +/**
> + * pinconf_generic_parse_dt_pinmux()
> + * parse the pinmux properties into generic pin mux values.
> + * @np: node containing the pinmux properties
> + * @pctldev: pincontrol device
> + * @pid: array with pin identity entries
> + * @pmux: array with pin mux value entries
> + * @npins: number of pins
> + *
> + * pinmux propertity: mux value [0,7]bits and pin identity [8,31]bits.
> + */
> +int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
> +				    unsigned int **pid, unsigned int **pmux,
> +				    unsigned int *npins)
> +{
> +	unsigned int *pid_t;
> +	unsigned int *pmux_t;
> +	struct property *prop;
> +	unsigned int npins_t, i;
> +	u32 value;
> +	int ret;
> +
> +	prop = of_find_property(np, "pinmux", NULL);
> +	if (!prop) {
> +		dev_info(dev, "Missing pinmux property\n");
> +		return -ENOENT;
> +	}
> +
> +	if (!pid || !pmux || !npins) {
> +		dev_err(dev, "paramers error\n");
> +		return -EINVAL;
> +	}
> +
> +	npins_t = prop->length / sizeof(u32);
> +	pid_t = devm_kcalloc(dev, npins_t, sizeof(*pid_t), GFP_KERNEL);
> +	pmux_t = devm_kcalloc(dev, npins_t, sizeof(*pmux_t), GFP_KERNEL);
> +	if (!pid_t || !pmux_t) {
> +		dev_err(dev, "kalloc memory fail\n");
> +		return -ENOMEM;
> +	}
> +	for (i = 0; i < npins_t; i++) {
> +		ret = of_property_read_u32_index(np, "pinmux", i, &value);
> +		if (ret) {
> +			dev_err(dev, "get pinmux value fail\n");
> +			goto exit;
> +		}
> +		pmux_t[i] = value & 0xff;
> +		pid_t[i] = (value >> 8) & 0xffffff;
> +	}
> +	*pid = pid_t;
> +	*pmux = pmux_t;
> +	*npins = npins_t;
> +
> +	return 0;
> +exit:
> +	devm_kfree(dev, pid_t);
> +	devm_kfree(dev, pmux_t);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_pinmux);
> +
>  /**
>   * pinconf_generic_parse_dt_config()
>   * parse the config properties into generic pinconfig values.
> @@ -295,6 +356,75 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>  }
>  EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_config);
>  
> +int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
> +					  struct device_node *np,
> +					  struct pinctrl_map **map,
> +					  unsigned int *num_maps)
> +{
> +	struct device *dev = pctldev->dev;
> +	struct device_node *pnode;
> +	unsigned long *configs = NULL;
> +	unsigned int num_configs = 0;
> +	struct property *prop;
> +	unsigned int reserved_maps;
> +	int reserve;
> +	int ret;
> +
> +	prop = of_find_property(np, "pinmux", NULL);
> +	if (!prop) {
> +		dev_info(dev, "Missing pinmux property\n");
> +		return -ENOENT;
> +	}
> +
> +	pnode = of_get_parent(np);
> +	if (!pnode) {
> +		dev_info(dev, "Missing function node\n");
> +		return -EINVAL;
> +	}
> +
> +	reserved_maps = 0;
> +	*map = NULL;
> +	*num_maps = 0;
> +
> +	ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
> +					      &num_configs);
> +	if (ret < 0) {
> +		dev_err(dev, "%pOF: could not parse node property\n", np);
> +		return ret;
> +	}
> +
> +	reserve = 1;
> +	if (num_configs)
> +		reserve++;
> +
> +	ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps,
> +					num_maps, reserve);
> +	if (ret < 0)
> +		goto exit;
> +
> +	ret = pinctrl_utils_add_map_mux(pctldev, map,
> +					&reserved_maps, num_maps, np->name,
> +					pnode->name);
> +	if (ret < 0)
> +		goto exit;
> +
> +	if (num_configs) {
> +		ret = pinctrl_utils_add_map_configs(pctldev, map, &reserved_maps,
> +						    num_maps, np->name, configs,
> +						    num_configs, PIN_MAP_TYPE_CONFIGS_GROUP);
> +		if (ret < 0)
> +			goto exit;
> +	}
> +
> +exit:
> +	kfree(configs);
> +	if (ret)
> +		pinctrl_utils_free_map(pctldev, *map, *num_maps);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pinconf_generic_dt_node_to_map_pinmux);
> +
>  int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
>  		struct device_node *np, struct pinctrl_map **map,
>  		unsigned int *reserved_maps, unsigned int *num_maps,
> diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
> index a14c950bc700..a171195b3615 100644
> --- a/drivers/pinctrl/pinconf.h
> +++ b/drivers/pinctrl/pinconf.h
> @@ -138,4 +138,8 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>  				    struct pinctrl_dev *pctldev,
>  				    unsigned long **configs,
>  				    unsigned int *nconfigs);
> +
> +int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
> +				    unsigned int **pid, unsigned int **pmux,
> +				    unsigned int *npins);
>  #endif
> diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
> index 53cfde98433d..1bcf071b860e 100644
> --- a/include/linux/pinctrl/pinconf-generic.h
> +++ b/include/linux/pinctrl/pinconf-generic.h
> @@ -232,4 +232,8 @@ static inline int pinconf_generic_dt_node_to_map_all(struct pinctrl_dev *pctldev
>  			PIN_MAP_TYPE_INVALID);
>  }
>  
> +int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
> +					  struct device_node *np,
> +					  struct pinctrl_map **map,
> +					  unsigned int *num_maps);
>  #endif /* __LINUX_PINCTRL_PINCONF_GENERIC_H */

-- 
Jerome

Re: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[Xianwei Zhao]

Hi Linus,
    Thanks for your reply.

On 2025/1/7 23:18, Linus Walleij wrote:
> [ EXTERNAL EMAIL ]
> 
> On Thu, Dec 26, 2024 at 8:57 AM Xianwei Zhao via B4 Relay
> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
> 
>> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>>
>> When describing pin mux func through pinmux propertity,
>> a standard API is added for support. The pinmux contains pin
>> identification and mux values, which can include multiple
>> pins. And groups configuration use other word. DTS such as:
>>
>> func-name {
>>          group_alias: group-name{
>>                  pinmux= <pin_id << 8 | mux_value)>,
>>                          <pin_id << 8 | mux_value)>;
>>                  bias-pull-up;
>>                  drive-strength-microamp = <4000>;
>>          };
>> };
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
> 
> There are some build errors, but I really like the path taken with
> the utility function here!
> 
I will fix the issue Reported by kernel test robot next version.
> Yours,
> Linus Walleij

Re: [PATCH v2 1/5] dt-bindings: pinctrl: Add support for Amlogic SoCs

[Xianwei Zhao]

Hi Rob,
    Thanks for your reply.

On 2025/1/3 05:24, Rob Herring wrote:
> [ EXTERNAL EMAIL ]
> 
> On Thu, Dec 26, 2024 at 03:57:41PM +0800, Xianwei Zhao wrote:
>> Add the dt-bindings for Amlogic pin controller, and add a new
>> dt-binding header file which document the GPIO bank names and
>> alternative func value of all Amlogic subsequent SoCs.
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> ---
>>   .../bindings/pinctrl/amlogic,pinctrl-a4.yaml       | 155 +++++++++++++++++++++
>>   include/dt-bindings/pinctrl/amlogic,pinctrl.h      |  68 +++++++++
>>   2 files changed, 223 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
>> new file mode 100644
>> index 000000000000..75863d179933
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/amlogic,pinctrl-a4.yaml
>> @@ -0,0 +1,155 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/pinctrl/amlogic,pinctrl-a4.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic pinmux controller
>> +
>> +maintainers:
>> +  - Xianwei Zhao <xianwei.zhao@amlogic.com>
>> +
>> +allOf:
>> +  - $ref: pinctrl.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: amlogic,pinctrl-a4
>> +
>> +  reg:
>> +    minItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: mux
>> +      - const: gpio
>> +
>> +  "#address-cells":
>> +    const: 1
>> +
>> +  "#size-cells":
>> +    const: 0
> 
> The addresses in 'reg' below are MMIO, right? If so, #size-cells can't
> be 0 and you also need 'ranges' here.
> 

Will add ranges here, reg and reg-names move into gpio object.

>> +
>> +required:
> 
> Move after 'patternProperties'.
> 
Will do.
>> +  - compatible
>> +  - reg
>> +  - reg-names
>> +  - "#address-cells"
>> +  - "#size-cells"
>> +
>> +patternProperties:
>> +  "^gpio@[0-9a-f]+$":
>> +    type: object
>> +
>> +    properties:
>> +      reg:
>> +        minItems: 2
> 
> You need to describe what each entry is:
> 
> items:
>    - description: foo
>    - description: bar
> 
Will add description in next version
>> +
>> +      mask:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +      gpio-controller: true
>> +
>> +      "#gpio-cells":
>> +        const: 2
>> +
>> +      ngpios:
>> +        $ref: /schemas/types.yaml#/definitions/uint32
> 
> Don't need a type here.
> 

Will drop ngpios, use gpio_rangs instead.

>> +        minimum: 1
>> +        maximum: 32
>> +
>> +      identity:
> 
> Needs a better name. This is the bank number?
> 

named 'bank-number'.

> But shouldn't you be able to use gpio-ranges here:
> 
> gpio-ranges = <&periphs_pinctrl 0 (bank# << 8) (value of npgios)>;
> 
> And do you really need ngpios if you have the value here?
> 
>> +        description: |
>> +          identifier are provided by the pin controller header file at:
>> +          <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
>> +
>> +        $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +    required:
>> +      - reg
>> +      - gpio-controller
>> +      - "#gpio-cells"
>> +      - ngpios
>> +      - identity
>> +
>> +    additionalProperties: false
>> +
>> +  "^func-[0-9a-z-]+$":
>> +    type: object
>> +    additionalProperties:
> 
> Define a pattern for the node names instead. We only allow anything for
> existing bindings.
>

Will use patternProperies

>> +      type: object
>> +      allOf:
>> +        - $ref: pincfg-node.yaml#
>> +        - $ref: pinmux-node.yaml#
>> +
>> +      description:
>> +        A pin multiplexing sub-node describes how to configure a set of (or a
>> +        single) pin in some desired alternate function mode.
>> +        A single sub-node may define several pin configurations.
>> +
>> +      properties:
>> +        pinmux:
>> +          description: |
>> +            Integer array representing pin number and pin multiplexing
>> +            configuration.
>> +            When a pin has to be configured in alternate function mode, use
>> +            this property to identify the pin by its global index, and provide
>> +            its alternate function configuration number along with it.
>> +            bank identifier are provided by the pin controller header file at:
>> +            <include/dt-bindings/pinctrl/amlogic,pinctrl.h>
>> +            Integers values in "pinmux" argument list are assembled as:
>> +            (((BANK << 8) + PIN) << 8)  | MUX_FUNC))
>> +
>> +      required:
>> +        - pinmux
>> +
>> +      additionalProperties: true
> 
> No, not allowed to be true except on common schemas.
> 

This object  I will only save
      allOf:
        - $ref: pincfg-node.yaml#
        - $ref: pinmux-node.yaml#
others(description, properties,required,  additionalProperties) will be 
dropped.

>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/pinctrl/amlogic,pinctrl.h>
>> +    apb {
>> +      #address-cells = <2>;
>> +      #size-cells = <2>;
>> +      periphs_pinctrl: pinctrl@8e700 {
>> +        compatible = "amlogic,pinctrl-a4";
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        reg = <0x0 0x8e700 0x0 0x04>,
>> +              <0x0 0x8e704 0x0 0x60>;
>> +        reg-names = "mux", "gpio";
>> +
>> +        gpio@14 {
>> +          reg = <0x14>,<0x30>;
>> +          gpio-controller;
>> +          #gpio-cells = <2>;
>> +          ngpios = <10>;
>> +          identity = <AMLOGIC_GPIO_B>;
>> +        };
>> +
>> +        func-uart-b {
>> +          uart-b-default {
>> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 1, AF4)>;
>> +            bias-pull-up;
>> +            drive-strength-microamp = <4000>;
>> +          };
>> +
>> +          uart-b-pins1 {
>> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 5, AF2)>;
>> +            bias-pull-up;
>> +            drive-strength-microamp = <4000>;
>> +          };
>> +        };
>> +
>> +        func-uart-c {
>> +          uart-c-default {
>> +            pinmux = <AML_PINMUX(AMLOGIC_GPIO_B, 3, AF1)>,
>> +                     <AML_PINMUX(AMLOGIC_GPIO_B, 2, AF1)>;
>> +            bias-pull-up;
>> +            drive-strength-microamp = <4000>;
>> +          };
>> +        };
>> +      };
>> +    };
>> diff --git a/include/dt-bindings/pinctrl/amlogic,pinctrl.h b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
>> new file mode 100644
>> index 000000000000..03db0a730e8b
>> --- /dev/null
>> +++ b/include/dt-bindings/pinctrl/amlogic,pinctrl.h
>> @@ -0,0 +1,68 @@
>> +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */
>> +/*
>> + * Copyright (c) 2024 Amlogic, Inc. All rights reserved.
>> + * Author: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_AMLOGIC_PINCTRL_H
>> +#define _DT_BINDINGS_AMLOGIC_PINCTRL_H
>> +
>> +/* define PIN modes */
>> +#define AF0  0x0
>> +#define AF1  0x1
>> +#define AF2  0x2
>> +#define AF3  0x3
>> +#define AF4  0x4
>> +#define AF5  0x5
>> +#define AF6  0x6
>> +#define AF7  0x7
>> +#define AF8  0x8
>> +#define AF9  0x9
>> +#define AF10 0xa
>> +#define AF11 0xb
>> +#define AF12 0xc
>> +#define AF13 0xd
>> +#define AF14 0xe
>> +#define AF15 0xf
> 
> There's no need for defines in the form "FOOn n".
> 

Will do.

>> +
>> +#define AML_PIN_ALT_FUNC_MASK        0xf
>> +
>> +/* Normal PIN bank */
>> +#define AMLOGIC_GPIO_A               0
>> +#define AMLOGIC_GPIO_B               1
>> +#define AMLOGIC_GPIO_C               2
>> +#define AMLOGIC_GPIO_D               3
>> +#define AMLOGIC_GPIO_E               4
>> +#define AMLOGIC_GPIO_F               5
>> +#define AMLOGIC_GPIO_G               6
>> +#define AMLOGIC_GPIO_H               7
>> +#define AMLOGIC_GPIO_I               8
>> +#define AMLOGIC_GPIO_J               9
>> +#define AMLOGIC_GPIO_K               10
>> +#define AMLOGIC_GPIO_L               11
>> +#define AMLOGIC_GPIO_M               12
>> +#define AMLOGIC_GPIO_N               13
>> +#define AMLOGIC_GPIO_O               14
>> +#define AMLOGIC_GPIO_P               15
>> +#define AMLOGIC_GPIO_Q               16
>> +#define AMLOGIC_GPIO_R               17
>> +#define AMLOGIC_GPIO_S               18
>> +#define AMLOGIC_GPIO_T               19
>> +#define AMLOGIC_GPIO_U               20
>> +#define AMLOGIC_GPIO_V               21
>> +#define AMLOGIC_GPIO_W               22
>> +#define AMLOGIC_GPIO_X               23
>> +#define AMLOGIC_GPIO_Y               24
>> +#define AMLOGIC_GPIO_Z               25
>> +
>> +/* Special PIN bank */
>> +#define AMLOGIC_GPIO_DV              26
>> +#define AMLOGIC_GPIO_AO              27
>> +#define AMLOGIC_GPIO_CC              28
>> +#define AMLOGIC_GPIO_TEST_N  29
>> +
>> +#define AML_PINMUX(bank, offset, mode)       (((((bank) << 8) + (offset)) << 8) | (mode))
>> +#define AML_PINMUX_TO_BANK(pinmux)   (((pinmux) >> 16) & 0xff)
>> +#define AML_PINMUX_TO_OFFSET(pinmux) (((pinmux) >> 8) & 0xff)
>> +
>> +#endif /* _DT_BINDINGS_AMLOGIC_PINCTRL_H */
>>
>> --
>> 2.37.1
>>

Re: [PATCH v2 3/5] pinctrl: Add driver support for Amlogic SoCs

[Xianwei Zhao]

Hi Martin,
    Thanks for your reply.

On 2024/12/28 05:11, Martin Blumenstingl wrote:
> [ EXTERNAL EMAIL ]
> 
> Hi Xianwei,
> 
> On Thu, Dec 26, 2024 at 8:57 AM Xianwei Zhao via B4 Relay
> <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
> [...]
>> +       if (of_property_read_u32(np, "mask", &bank->pc.mux_mask))
>> +               bank->pc.mux_mask = 0xf;
> I don't see any documentation for the mask property.
> The discussion about the dt-bindings is still pretty heated.
> Everything else is already part of the documentation (which is good!).
> Please document his property as well (or remove the code to read it if
> there's no need to have it dynamic).
> 
> This is not a full code-review. However, since the discussion about
> the whole dt-bindings is still ongoing we need to make sure that
> whatever we document in the bindings is actually all that we need to
> write the driver.
> 

The whole dt-bindings is still under discussion and revision. The 'mask' 
property will add description next version.

> 
> Best regards,
> Martin

Re: [PATCH v2 2/5] pinctrl: pinconf-generic: Add API for pinmux propertity in DTS file

[Xianwei Zhao]

Hi Jerome,
     Thanks for your reply.

On 2025/1/8 00:47, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
> 
> On Thu 26 Dec 2024 at 15:57, Xianwei Zhao via B4 Relay <devnull+xianwei.zhao.amlogic.com@kernel.org> wrote:
> 
>> From: Xianwei Zhao <xianwei.zhao@amlogic.com>
>>
>> When describing pin mux func through pinmux propertity,
>> a standard API is added for support. The pinmux contains pin
>> identification and mux values, which can include multiple
>> pins. And groups configuration use other word. DTS such as:
>>
>> func-name {
>>        group_alias: group-name{
>>                pinmux= <pin_id << 8 | mux_value)>,
>>                        <pin_id << 8 | mux_value)>;
> 
> This representation does not seem very generic but more tailored to
> your use-case
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml?h=next-20250113

The document above includes a description of the pinmux property. This 
is not my personal usage but rather a common practice, as other 
manufacturers' drivers also use it in a similar way. However, the 
specific definitions within pinmux may vary between manufacturers.

> 
>>                bias-pull-up;
>>                drive-strength-microamp = <4000>;
> 
> If you want to add pinmux (aka alternate function) selection as a
> pinconf prop then I think there should be a single pinmux setting per
> group, and as many groups as you need per function defined.
> 
> something like
> 
> func-foo {
>          group-a {
>                  groups = "pin_a", "pin_b";
>                  bias-pull-up;
>                  alternate-function = <2>;
>          };
> 
>          group-b {
>                  groups = "pin_c"";
>                  bias-disable;
>                  alternate-function = <5>;
>          };
> };
> 
> Something similar is already done to handle different pin bias requirement
> on single function on amlogic platforms:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/amlogic/meson-axg.dtsi?h=v6.13-rc6#n421
> 
> 
>>        };
>> };
>>
>> Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
>> ---
>>   drivers/pinctrl/pinconf-generic.c       | 130 ++++++++++++++++++++++++++++++++
>>   drivers/pinctrl/pinconf.h               |   4 +
>>   include/linux/pinctrl/pinconf-generic.h |   4 +
>>   3 files changed, 138 insertions(+)
>>
>> diff --git a/drivers/pinctrl/pinconf-generic.c b/drivers/pinctrl/pinconf-generic.c
>> index 0b13d7f17b32..a4d3c12a80c4 100644
>> --- a/drivers/pinctrl/pinconf-generic.c
>> +++ b/drivers/pinctrl/pinconf-generic.c
>> @@ -233,6 +233,67 @@ static void parse_dt_cfg(struct device_node *np,
>>        }
>>   }
>>
>> +/**
>> + * pinconf_generic_parse_dt_pinmux()
>> + * parse the pinmux properties into generic pin mux values.
>> + * @np: node containing the pinmux properties
>> + * @pctldev: pincontrol device
>> + * @pid: array with pin identity entries
>> + * @pmux: array with pin mux value entries
>> + * @npins: number of pins
>> + *
>> + * pinmux propertity: mux value [0,7]bits and pin identity [8,31]bits.
>> + */
>> +int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
>> +                                 unsigned int **pid, unsigned int **pmux,
>> +                                 unsigned int *npins)
>> +{
>> +     unsigned int *pid_t;
>> +     unsigned int *pmux_t;
>> +     struct property *prop;
>> +     unsigned int npins_t, i;
>> +     u32 value;
>> +     int ret;
>> +
>> +     prop = of_find_property(np, "pinmux", NULL);
>> +     if (!prop) {
>> +             dev_info(dev, "Missing pinmux property\n");
>> +             return -ENOENT;
>> +     }
>> +
>> +     if (!pid || !pmux || !npins) {
>> +             dev_err(dev, "paramers error\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     npins_t = prop->length / sizeof(u32);
>> +     pid_t = devm_kcalloc(dev, npins_t, sizeof(*pid_t), GFP_KERNEL);
>> +     pmux_t = devm_kcalloc(dev, npins_t, sizeof(*pmux_t), GFP_KERNEL);
>> +     if (!pid_t || !pmux_t) {
>> +             dev_err(dev, "kalloc memory fail\n");
>> +             return -ENOMEM;
>> +     }
>> +     for (i = 0; i < npins_t; i++) {
>> +             ret = of_property_read_u32_index(np, "pinmux", i, &value);
>> +             if (ret) {
>> +                     dev_err(dev, "get pinmux value fail\n");
>> +                     goto exit;
>> +             }
>> +             pmux_t[i] = value & 0xff;
>> +             pid_t[i] = (value >> 8) & 0xffffff;
>> +     }
>> +     *pid = pid_t;
>> +     *pmux = pmux_t;
>> +     *npins = npins_t;
>> +
>> +     return 0;
>> +exit:
>> +     devm_kfree(dev, pid_t);
>> +     devm_kfree(dev, pmux_t);
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_pinmux);
>> +
>>   /**
>>    * pinconf_generic_parse_dt_config()
>>    * parse the config properties into generic pinconfig values.
>> @@ -295,6 +356,75 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>>   }
>>   EXPORT_SYMBOL_GPL(pinconf_generic_parse_dt_config);
>>
>> +int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
>> +                                       struct device_node *np,
>> +                                       struct pinctrl_map **map,
>> +                                       unsigned int *num_maps)
>> +{
>> +     struct device *dev = pctldev->dev;
>> +     struct device_node *pnode;
>> +     unsigned long *configs = NULL;
>> +     unsigned int num_configs = 0;
>> +     struct property *prop;
>> +     unsigned int reserved_maps;
>> +     int reserve;
>> +     int ret;
>> +
>> +     prop = of_find_property(np, "pinmux", NULL);
>> +     if (!prop) {
>> +             dev_info(dev, "Missing pinmux property\n");
>> +             return -ENOENT;
>> +     }
>> +
>> +     pnode = of_get_parent(np);
>> +     if (!pnode) {
>> +             dev_info(dev, "Missing function node\n");
>> +             return -EINVAL;
>> +     }
>> +
>> +     reserved_maps = 0;
>> +     *map = NULL;
>> +     *num_maps = 0;
>> +
>> +     ret = pinconf_generic_parse_dt_config(np, pctldev, &configs,
>> +                                           &num_configs);
>> +     if (ret < 0) {
>> +             dev_err(dev, "%pOF: could not parse node property\n", np);
>> +             return ret;
>> +     }
>> +
>> +     reserve = 1;
>> +     if (num_configs)
>> +             reserve++;
>> +
>> +     ret = pinctrl_utils_reserve_map(pctldev, map, &reserved_maps,
>> +                                     num_maps, reserve);
>> +     if (ret < 0)
>> +             goto exit;
>> +
>> +     ret = pinctrl_utils_add_map_mux(pctldev, map,
>> +                                     &reserved_maps, num_maps, np->name,
>> +                                     pnode->name);
>> +     if (ret < 0)
>> +             goto exit;
>> +
>> +     if (num_configs) {
>> +             ret = pinctrl_utils_add_map_configs(pctldev, map, &reserved_maps,
>> +                                                 num_maps, np->name, configs,
>> +                                                 num_configs, PIN_MAP_TYPE_CONFIGS_GROUP);
>> +             if (ret < 0)
>> +                     goto exit;
>> +     }
>> +
>> +exit:
>> +     kfree(configs);
>> +     if (ret)
>> +             pinctrl_utils_free_map(pctldev, *map, *num_maps);
>> +
>> +     return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(pinconf_generic_dt_node_to_map_pinmux);
>> +
>>   int pinconf_generic_dt_subnode_to_map(struct pinctrl_dev *pctldev,
>>                struct device_node *np, struct pinctrl_map **map,
>>                unsigned int *reserved_maps, unsigned int *num_maps,
>> diff --git a/drivers/pinctrl/pinconf.h b/drivers/pinctrl/pinconf.h
>> index a14c950bc700..a171195b3615 100644
>> --- a/drivers/pinctrl/pinconf.h
>> +++ b/drivers/pinctrl/pinconf.h
>> @@ -138,4 +138,8 @@ int pinconf_generic_parse_dt_config(struct device_node *np,
>>                                    struct pinctrl_dev *pctldev,
>>                                    unsigned long **configs,
>>                                    unsigned int *nconfigs);
>> +
>> +int pinconf_generic_parse_dt_pinmux(struct device_node *np, struct device *dev,
>> +                                 unsigned int **pid, unsigned int **pmux,
>> +                                 unsigned int *npins);
>>   #endif
>> diff --git a/include/linux/pinctrl/pinconf-generic.h b/include/linux/pinctrl/pinconf-generic.h
>> index 53cfde98433d..1bcf071b860e 100644
>> --- a/include/linux/pinctrl/pinconf-generic.h
>> +++ b/include/linux/pinctrl/pinconf-generic.h
>> @@ -232,4 +232,8 @@ static inline int pinconf_generic_dt_node_to_map_all(struct pinctrl_dev *pctldev
>>                        PIN_MAP_TYPE_INVALID);
>>   }
>>
>> +int pinconf_generic_dt_node_to_map_pinmux(struct pinctrl_dev *pctldev,
>> +                                       struct device_node *np,
>> +                                       struct pinctrl_map **map,
>> +                                       unsigned int *num_maps);
>>   #endif /* __LINUX_PINCTRL_PINCONF_GENERIC_H */
> 
> --
> Jerome