[PATCH 0/3] Add pinctrl support for Sky1

[PATCH 0/3] Add pinctrl support for Sky1

[Gary Yang]

patch 1: add Cix pinctrl driver which needs to support pinmux and pinconfigs
patch 2: add Cix pinctrl nodes and header file
patch 3: add yaml docs

Gary Yang (3):
  pinctrl: cix: Add pin-controller support for sky1
  dt-bindings: pinctrl: Add cix,sky1-pinctrl
  arm64: dts: cix: Add pinctrl nodes for sky1

 .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
 arch/arm64/boot/dts/cix/sky1-orion-o6.dts     |  28 +
 arch/arm64/boot/dts/cix/sky1.dtsi             |  10 +
 drivers/pinctrl/Kconfig                       |   1 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/cix/Kconfig                   |  14 +
 drivers/pinctrl/cix/Makefile                  |   4 +
 drivers/pinctrl/cix/pinctrl-sky1-base.c       | 622 ++++++++++++++++++
 drivers/pinctrl/cix/pinctrl-sky1.c            | 502 ++++++++++++++
 drivers/pinctrl/cix/pinctrl-sky1.h            |  55 ++
 include/dt-bindings/pinctrl/pads-sky1.h       | 592 +++++++++++++++++
 11 files changed, 1906 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
 create mode 100644 drivers/pinctrl/cix/Kconfig
 create mode 100644 drivers/pinctrl/cix/Makefile
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1-base.c
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1.c
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1.h
 create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h

-- 
2.49.0

[PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Gary Yang]

Add the pin-controller driver for Sky1 platform

Signed-off-by: Gary Yang <gary.yang@cixtech.com>
---
 drivers/pinctrl/Kconfig                 |   1 +
 drivers/pinctrl/Makefile                |   1 +
 drivers/pinctrl/cix/Kconfig             |  14 +
 drivers/pinctrl/cix/Makefile            |   4 +
 drivers/pinctrl/cix/pinctrl-sky1-base.c | 622 ++++++++++++++++++++++++
 drivers/pinctrl/cix/pinctrl-sky1.c      | 502 +++++++++++++++++++
 drivers/pinctrl/cix/pinctrl-sky1.h      |  55 +++
 7 files changed, 1199 insertions(+)
 create mode 100644 drivers/pinctrl/cix/Kconfig
 create mode 100644 drivers/pinctrl/cix/Makefile
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1-base.c
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1.c
 create mode 100644 drivers/pinctrl/cix/pinctrl-sky1.h

diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index ddd11668457c..a5b7177be115 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -653,6 +653,7 @@ source "drivers/pinctrl/aspeed/Kconfig"
 source "drivers/pinctrl/bcm/Kconfig"
 source "drivers/pinctrl/berlin/Kconfig"
 source "drivers/pinctrl/cirrus/Kconfig"
+source "drivers/pinctrl/cix/Kconfig"
 source "drivers/pinctrl/freescale/Kconfig"
 source "drivers/pinctrl/intel/Kconfig"
 source "drivers/pinctrl/mediatek/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 909ab89a56d2..8f353fc38e0e 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -67,6 +67,7 @@ obj-$(CONFIG_ARCH_ASPEED)	+= aspeed/
 obj-y				+= bcm/
 obj-$(CONFIG_PINCTRL_BERLIN)	+= berlin/
 obj-y				+= cirrus/
+obj-y				+= cix/
 obj-y				+= freescale/
 obj-$(CONFIG_X86)		+= intel/
 obj-y				+= mediatek/
diff --git a/drivers/pinctrl/cix/Kconfig b/drivers/pinctrl/cix/Kconfig
new file mode 100644
index 000000000000..42e064b6e7a2
--- /dev/null
+++ b/drivers/pinctrl/cix/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config PINCTRL_SKY1_BASE
+	tristate
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	select REGMAP
+
+config PINCTRL_SKY1
+	tristate "Cix Sky1 pinctrl driver"
+	depends on ARCH_CIX
+	select PINCTRL_SKY1_BASE
+	help
+	  Say Y here to enable the sky1 pinctrl driver
diff --git a/drivers/pinctrl/cix/Makefile b/drivers/pinctrl/cix/Makefile
new file mode 100644
index 000000000000..22685d6a107b
--- /dev/null
+++ b/drivers/pinctrl/cix/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+# Cix Sky1 pin control drivers
+obj-$(CONFIG_PINCTRL_SKY1_BASE)	+= pinctrl-sky1-base.o
+obj-$(CONFIG_PINCTRL_SKY1)	+= pinctrl-sky1.o
diff --git a/drivers/pinctrl/cix/pinctrl-sky1-base.c b/drivers/pinctrl/cix/pinctrl-sky1-base.c
new file mode 100644
index 000000000000..a80c68fdadfc
--- /dev/null
+++ b/drivers/pinctrl/cix/pinctrl-sky1-base.c
@@ -0,0 +1,622 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Author: Jerry Zhu <Jerry.Zhu@cixtech.com>
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/of_address.h>
+#include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/pinctrl/pinmux.h>
+#include <linux/platform_device.h>
+#include <linux/seq_file.h>
+#include <linux/slab.h>
+#include <linux/regmap.h>
+
+#include "../core.h"
+#include "../pinconf.h"
+#include "../pinmux.h"
+#include "pinctrl-sky1.h"
+
+#define SKY1_PIN_SIZE		(0xc)
+#define SKY1_MUX_MASK		(0x180L)
+#define SKY1_CONF_MASK		(0x7fL)
+#define PADS_FUNCS_MASK		(0x3)
+#define PADS_FUNCS_BITS		(0x7)
+#define PADS_CONFS_MASK		(0x7f)
+
+static inline const struct group_desc *sky1_pinctrl_find_group_by_name(
+				struct pinctrl_dev *pctldev,
+				const char *name)
+{
+	const struct group_desc *grp = NULL;
+	int i;
+
+	for (i = 0; i < pctldev->num_groups; i++) {
+		grp = pinctrl_generic_get_group(pctldev, i);
+		if (grp && !strcmp(grp->grp.name, name))
+			break;
+	}
+
+	return grp;
+}
+
+static void sky1_pin_dbg_show(struct pinctrl_dev *pctldev, struct seq_file *s,
+		   unsigned int offset)
+{
+	seq_printf(s, "%s", dev_name(pctldev->dev));
+}
+
+static int sky1_dt_node_to_map(struct pinctrl_dev *pctldev,
+			struct device_node *np,
+			struct pinctrl_map **map, unsigned int *num_maps)
+{
+	struct sky1_pinctrl *spctl = pinctrl_dev_get_drvdata(pctldev);
+	const struct group_desc *grp;
+	struct pinctrl_map *new_map;
+	struct device_node *parent;
+	struct sky1_pin *pin;
+	int map_num = 1;
+	int i, j;
+
+	/*
+	 * first find the group of this node and check if we need create
+	 * config maps for pins
+	 */
+	grp = sky1_pinctrl_find_group_by_name(pctldev, np->name);
+	if (!grp) {
+		dev_err(spctl->dev, "unable to find group for node %pOFn\n", np);
+		return -EINVAL;
+	}
+
+	map_num += grp->grp.npins;
+
+	new_map = kmalloc_array(map_num, sizeof(struct pinctrl_map),
+				GFP_KERNEL);
+	if (!new_map)
+		return -ENOMEM;
+
+	*map = new_map;
+	*num_maps = map_num;
+
+	/* create mux map */
+	parent = of_get_parent(np);
+	if (!parent) {
+		kfree(new_map);
+		return -EINVAL;
+	}
+	new_map[0].type = PIN_MAP_TYPE_MUX_GROUP;
+	new_map[0].data.mux.function = parent->name;
+	new_map[0].data.mux.group = np->name;
+	of_node_put(parent);
+
+	/* create config map */
+	new_map++;
+	for (i = j = 0; i < grp->grp.npins; i++) {
+		pin = &((struct sky1_pin *)(grp->data))[i];
+
+		new_map[j].type = PIN_MAP_TYPE_CONFIGS_PIN;
+		new_map[j].data.configs.group_or_pin =
+			pin_get_name(pctldev, pin->offset/4);
+		new_map[j].data.configs.configs = &pin->configs;
+		new_map[j].data.configs.num_configs = 1;
+
+		j++;
+	}
+
+	dev_dbg(pctldev->dev, "maps: function %s group %s num %d\n",
+		(*map)->data.mux.function, (*map)->data.mux.group, map_num);
+
+	return 0;
+}
+
+static inline const char *
+sky1_pinctrl_find_pin_group(struct pinctrl_dev *pctldev,
+			    unsigned int selector,
+			    unsigned int *pins_in,
+			    size_t npins_in)
+{
+	int i, j;
+	int ret;
+	const char *const *groups;
+	unsigned int num_groups;
+	const unsigned int *pins;
+	unsigned int num_pins;
+
+	ret = pinmux_generic_get_function_groups(pctldev, selector, &groups,
+						 &num_groups);
+	for (i = 0; i < num_groups; i++) {
+		ret = pinctrl_get_group_pins(pctldev, groups[i], &pins,
+					     &num_pins);
+		if (npins_in != num_pins)
+			continue;
+		for (j = 0; j < num_pins; j++) {
+			if (pins[j] == pins_in[0])
+				return groups[i];
+		}
+	}
+	return NULL;
+}
+
+static void sky1_dt_free_map(struct pinctrl_dev *pctldev,
+			     struct pinctrl_map *map,
+			     unsigned int num_maps)
+{
+	kfree(map);
+}
+
+static const struct pinctrl_ops sky1_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 = sky1_pin_dbg_show,
+	.dt_node_to_map = sky1_dt_node_to_map,
+	.dt_free_map = sky1_dt_free_map,
+};
+
+static int sky1_pmx_set_one_pin(struct sky1_pinctrl *spctl,
+				    struct sky1_pin *pin)
+{
+	u32 reg_val;
+	u32 *pin_reg;
+
+	pin_reg = spctl->base + pin->offset;
+	reg_val = readl(pin_reg);
+	reg_val &= ~SKY1_MUX_MASK;
+	reg_val |= pin->configs & SKY1_MUX_MASK;
+	writel(reg_val, pin_reg);
+
+	dev_dbg(spctl->dev, "write: offset 0x%x val 0x%x\n",
+		pin->offset, reg_val);
+	return 0;
+}
+
+static int sky1_pmx_set(struct pinctrl_dev *pctldev, unsigned int selector,
+		       unsigned int group)
+{
+	struct sky1_pinctrl *spctl = pinctrl_dev_get_drvdata(pctldev);
+	struct function_desc *func;
+	struct group_desc *grp;
+	struct sky1_pin *pin;
+	unsigned int npins;
+	int i, err;
+
+	/*
+	 * Configure the mux mode for each pin in the group for a specific
+	 * function.
+	 */
+	grp = pinctrl_generic_get_group(pctldev, group);
+	if (!grp)
+		return -EINVAL;
+
+	func = pinmux_generic_get_function(pctldev, selector);
+	if (!func)
+		return -EINVAL;
+
+	npins = grp->grp.npins;
+
+	dev_dbg(spctl->dev, "enable function %s group %s\n",
+		func->func.name, grp->grp.name);
+
+	for (i = 0; i < npins; i++) {
+		/*
+		 * Config for Sky1 one pin
+		 */
+		pin = &((struct sky1_pin *)(grp->data))[i];
+		err = sky1_pmx_set_one_pin(spctl, pin);
+		if (err)
+			return err;
+	}
+
+	return 0;
+}
+
+static const struct pinmux_ops sky1_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 = sky1_pmx_set,
+};
+
+static int sky1_pinconf_get(struct pinctrl_dev *pctldev,
+			   unsigned int pin_id, unsigned long *config)
+{
+	struct sky1_pinctrl *spctl = pinctrl_dev_get_drvdata(pctldev);
+	*config = readl(spctl->base + spctl->pin_regs[pin_id]);
+
+	return 0;
+}
+
+static int sky1_pinconf_set(struct pinctrl_dev *pctldev,
+			   unsigned int pin_id, unsigned long *configs,
+			   unsigned int num_configs)
+{
+	struct sky1_pinctrl *spctl = pinctrl_dev_get_drvdata(pctldev);
+	u32 reg_val;
+	u32 *pin_reg;
+	int i;
+
+	pin_reg = spctl->base + spctl->pin_regs[pin_id];
+
+	for (i = 0; i < num_configs; i++) {
+		reg_val = readl(pin_reg);
+		reg_val &= ~SKY1_CONF_MASK;
+		reg_val |= (configs[i] & SKY1_CONF_MASK);
+		writel(reg_val, pin_reg);
+
+		dev_dbg(spctl->dev, "write: offset 0x%x val 0x%x\n",
+			spctl->pin_regs[pin_id], reg_val);
+	}
+
+	return 0;
+}
+
+static void sky1_pinconf_dbg_show(struct pinctrl_dev *pctldev,
+				   struct seq_file *s, unsigned int pin_id)
+{
+	struct sky1_pinctrl *spctl = pinctrl_dev_get_drvdata(pctldev);
+	u32 config;
+	u32 *pin_reg;
+
+	if (spctl->pin_regs[pin_id] == -1) {
+		seq_puts(s, "N/A");
+		return;
+	}
+
+	pin_reg = spctl->base + spctl->pin_regs[pin_id];
+	config = readl(pin_reg) & SKY1_CONF_MASK;
+
+	seq_printf(s, "0x%x", config);
+}
+
+static void sky1_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					 struct seq_file *s, unsigned int group)
+{
+	struct group_desc *grp;
+	unsigned long config;
+	const char *name;
+	int i, pin_id, ret;
+
+	if (group >= pctldev->num_groups)
+		return;
+
+	seq_puts(s, "\n");
+	grp = pinctrl_generic_get_group(pctldev, group);
+	if (!grp)
+		return;
+
+	for (i = 0; i < grp->grp.npins; i++) {
+		struct sky1_pin *pin = &(((struct sky1_pin *)(grp->data))[i]);
+
+		pin_id = pin->offset / 4;
+		name = pin_get_name(pctldev, pin_id);
+		ret = sky1_pinconf_get(pctldev, pin_id, &config);
+		if (ret)
+			return;
+		seq_printf(s, "  %s: 0x%lx\n", name, config);
+	}
+}
+
+static const struct pinconf_ops sky1_pinconf_ops = {
+	.pin_config_get = sky1_pinconf_get,
+	.pin_config_set = sky1_pinconf_set,
+	.pin_config_dbg_show = sky1_pinconf_dbg_show,
+	.pin_config_group_dbg_show = sky1_pinconf_group_dbg_show,
+};
+
+/*
+ * Each pin represented in cix,pins consists of
+ * a number of u32 OFFSET and a number of u32 CONFIGS,
+ * the total size is OFFSET + CONFIGS for each pin.
+ *
+ * Default:
+ *     <offset, configs>
+ *     <4byte,  4byte>
+ */
+
+static void sky1_pinctrl_parse_pin(struct sky1_pinctrl *spctl,
+				       unsigned int *pin_id,
+				       struct sky1_pin *pin,
+				       const __be32 **list_p,
+				       struct device_node *np)
+{
+	const __be32 *list = *list_p;
+	unsigned int configs0, configs1;
+	const struct sky1_pinctrl_soc_info *info = spctl->info;
+
+	pin->offset = be32_to_cpu(*list++);
+	*pin_id = pin->offset / 4;
+	pin->pin_id = *pin_id;
+
+	configs0 = be32_to_cpu(*list++);
+	configs1 = be32_to_cpu(*list++);
+	pin->configs =  (((configs0 & PADS_FUNCS_MASK) << PADS_FUNCS_BITS) |
+			(configs1 & PADS_CONFS_MASK));
+	spctl->pin_regs[*pin_id] = pin->offset;
+
+	*list_p = list;
+
+	dev_dbg(spctl->dev, "%s: 0x%x 0x%08lx", info->pins[*pin_id].name,
+		pin->offset, pin->configs);
+}
+
+static int sky1_pinctrl_parse_groups(struct device_node *np,
+				    struct group_desc *grp,
+				    struct sky1_pinctrl *spctl,
+				    u32 index)
+{
+	struct sky1_pin *pin;
+	int size;
+	const __be32 *list;
+	int i;
+	unsigned int *pins;
+
+	dev_dbg(spctl->dev, "group(%d): %pOFn\n", index, np);
+
+	/* Initialise group */
+	grp->grp.name = np->name;
+
+	/*
+	 * the binding format is cix,pins = <PIN_FUNC_ID CONFIG ...>,
+	 * do sanity check and calculate pins number
+	 *
+	 * First try legacy 'cix,pins' property, then fall back to the
+	 * generic 'pinmux'.
+	 *
+	 * Note: for generic 'pinmux' case, there's no CONFIG part in
+	 * the binding format.
+	 */
+	list = of_get_property(np, "cix,pins", &size);
+	if (!list) {
+		list = of_get_property(np, "pinmux", &size);
+		if (!list) {
+			dev_err(spctl->dev,
+				"no cix,pins and pins property in node %pOF\n", np);
+			return -EINVAL;
+		}
+	}
+
+	/* we do not check return since it's safe node passed down */
+	if (!size || size % SKY1_PIN_SIZE) {
+		dev_err(spctl->dev, "Invalid cix,pins or pins property in node %pOF\n", np);
+		return -EINVAL;
+	}
+
+	grp->grp.npins = size / SKY1_PIN_SIZE;
+	grp->data = devm_kcalloc(spctl->dev,
+				 grp->grp.npins, sizeof(struct sky1_pin),
+				 GFP_KERNEL);
+	pins = devm_kcalloc(spctl->dev,
+				 grp->grp.npins, sizeof(unsigned int),
+				 GFP_KERNEL);
+	if (!pins || !grp->data)
+		return -ENOMEM;
+
+	for (i = 0; i < grp->grp.npins; i++) {
+		pin = &((struct sky1_pin *)(grp->data))[i];
+		sky1_pinctrl_parse_pin(spctl, &pins[i], pin, &list, np);
+	}
+	grp->grp.pins = pins;
+	return 0;
+}
+
+static int sky1_pinctrl_parse_functions(struct device_node *np,
+				       struct sky1_pinctrl *spctl,
+				       u32 index)
+{
+	struct pinctrl_dev *pctl = spctl->pctl;
+	struct device_node *child;
+	struct function_desc *func;
+	struct group_desc *grp;
+	const char **group_names;
+	u32 i = 0;
+
+	dev_dbg(pctl->dev, "parse function(%d): %pOFn\n", index, np);
+
+	func = pinmux_generic_get_function(pctl, index);
+	if (!func)
+		return -EINVAL;
+
+	/* Initialise function */
+	func->func.name = np->name;
+	func->func.ngroups = of_get_child_count(np);
+	if (func->func.ngroups == 0) {
+		dev_err(spctl->dev, "no groups defined in %pOF\n", np);
+		return -EINVAL;
+	}
+	group_names = devm_kcalloc(spctl->dev, func->func.ngroups,
+					 sizeof(char *), GFP_KERNEL);
+	if (!group_names)
+		return -ENOMEM;
+
+	for_each_child_of_node(np, child)
+		group_names[i++] = child->name;
+	func->func.groups = group_names;
+
+	i = 0;
+	for_each_child_of_node(np, child) {
+		grp = devm_kzalloc(spctl->dev, sizeof(struct group_desc),
+				   GFP_KERNEL);
+		if (!grp) {
+			of_node_put(child);
+			return -ENOMEM;
+		}
+
+		mutex_lock(&spctl->mutex);
+		radix_tree_insert(&pctl->pin_group_tree,
+				  spctl->group_index++, grp);
+		mutex_unlock(&spctl->mutex);
+
+		sky1_pinctrl_parse_groups(child, grp, spctl, i++);
+	}
+
+	return 0;
+}
+
+/*
+ * Check if the DT contains pins in the direct child nodes. This indicates the
+ * newer DT format to store pins. This function returns true if the first found
+ * cix,pins property is in a child of np. Otherwise false is returned.
+ */
+static bool sky1_pinctrl_dt_is_flat_functions(struct device_node *np)
+{
+	struct device_node *function_np;
+	struct device_node *pinctrl_np;
+
+	for_each_child_of_node(np, function_np) {
+		if (of_property_read_bool(function_np, "cix,pins")) {
+			of_node_put(function_np);
+			return true;
+		}
+
+		for_each_child_of_node(function_np, pinctrl_np) {
+			if (of_property_read_bool(pinctrl_np, "cix,pins")) {
+				of_node_put(pinctrl_np);
+				of_node_put(function_np);
+				return false;
+			}
+		}
+	}
+
+	return true;
+}
+
+static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
+				struct sky1_pinctrl *spctl)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *child;
+	struct pinctrl_dev *pctl = spctl->pctl;
+	u32 nfuncs = 0;
+	u32 i = 0;
+	bool flat_funcs;
+
+	if (!np)
+		return -ENODEV;
+
+	flat_funcs = sky1_pinctrl_dt_is_flat_functions(np);
+	if (flat_funcs) {
+		nfuncs = 1;
+	} else {
+		nfuncs = of_get_child_count(np);
+		if (nfuncs == 0) {
+			dev_err(&pdev->dev, "no functions defined\n");
+			return -EINVAL;
+		}
+	}
+
+	for (i = 0; i < nfuncs; i++) {
+		struct function_desc *function;
+
+		function = devm_kzalloc(&pdev->dev, sizeof(*function),
+					GFP_KERNEL);
+		if (!function)
+			return -ENOMEM;
+
+		mutex_lock(&spctl->mutex);
+		radix_tree_insert(&pctl->pin_function_tree, i, function);
+		mutex_unlock(&spctl->mutex);
+	}
+	pctl->num_functions = nfuncs;
+
+	spctl->group_index = 0;
+	if (flat_funcs) {
+		pctl->num_groups = of_get_child_count(np);
+	} else {
+		pctl->num_groups = 0;
+		for_each_child_of_node(np, child)
+			pctl->num_groups += of_get_child_count(child);
+	}
+
+	if (flat_funcs) {
+		sky1_pinctrl_parse_functions(np, spctl, 0);
+	} else {
+		i = 0;
+		for_each_child_of_node(np, child)
+			sky1_pinctrl_parse_functions(child, spctl, i++);
+	}
+
+	return 0;
+}
+
+int sky1_base_pinctrl_probe(struct platform_device *pdev,
+		      const struct sky1_pinctrl_soc_info *info)
+{
+	struct pinctrl_desc *sky1_pinctrl_desc;
+	struct sky1_pinctrl *spctl;
+	int ret, i;
+
+	if (!info || !info->pins || !info->npins) {
+		dev_err(&pdev->dev, "wrong pinctrl info\n");
+		return -EINVAL;
+	}
+
+	/* Create state holders etc for this driver */
+	spctl = devm_kzalloc(&pdev->dev, sizeof(*spctl), GFP_KERNEL);
+	if (!spctl)
+		return -ENOMEM;
+
+	spctl->pin_regs = devm_kmalloc_array(&pdev->dev, info->npins,
+					    sizeof(*spctl->pin_regs),
+					    GFP_KERNEL);
+	if (!spctl->pin_regs)
+		return -ENOMEM;
+
+	for (i = 0; i < info->npins; i++)
+		spctl->pin_regs[i] = -1;
+
+	spctl->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(spctl->base))
+		return PTR_ERR(spctl->base);
+
+	sky1_pinctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*sky1_pinctrl_desc),
+					GFP_KERNEL);
+	if (!sky1_pinctrl_desc)
+		return -ENOMEM;
+
+	sky1_pinctrl_desc->name = dev_name(&pdev->dev);
+	sky1_pinctrl_desc->pins = info->pins;
+	sky1_pinctrl_desc->npins = info->npins;
+	sky1_pinctrl_desc->pctlops = &sky1_pctrl_ops;
+	sky1_pinctrl_desc->pmxops = &sky1_pmx_ops;
+	sky1_pinctrl_desc->confops = &sky1_pinconf_ops;
+	sky1_pinctrl_desc->owner = THIS_MODULE;
+
+	mutex_init(&spctl->mutex);
+
+	spctl->info = info;
+	spctl->dev = &pdev->dev;
+	platform_set_drvdata(pdev, spctl);
+	ret = devm_pinctrl_register_and_init(&pdev->dev,
+					     sky1_pinctrl_desc, spctl,
+					     &spctl->pctl);
+	if (ret) {
+		dev_err(&pdev->dev, "could not register SKY1 pinctrl driver\n");
+		return ret;
+	}
+
+	ret = sky1_pinctrl_probe_dt(pdev, spctl);
+
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		return ret;
+	}
+
+	pinctrl_provide_dummies();
+	dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
+
+	return pinctrl_enable(spctl->pctl);
+}
+EXPORT_SYMBOL_GPL(sky1_base_pinctrl_probe);
+
+
+MODULE_AUTHOR("Jerry Zhu <Jerry.Zhu@cixtech.com>");
+MODULE_DESCRIPTION("Cix SKy1 pinctrl driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/cix/pinctrl-sky1.c b/drivers/pinctrl/cix/pinctrl-sky1.c
new file mode 100644
index 000000000000..0e184eacbc0b
--- /dev/null
+++ b/drivers/pinctrl/cix/pinctrl-sky1.c
@@ -0,0 +1,502 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/platform_device.h>
+#include "linux/stddef.h"
+
+#include "../core.h"
+#include "pinctrl-sky1.h"
+
+enum sky1_pads_s5 {
+	SKY1_IOMUXC_GPIO1 =	0,
+	SKY1_IOMUXC_GPIO2 =	1,
+	SKY1_IOMUXC_GPIO3 =	2,
+	SKY1_IOMUXC_GPIO4 =	3,
+	SKY1_IOMUXC_GPIO5 =	4,
+	SKY1_IOMUXC_GPIO6 =	5,
+	SKY1_IOMUXC_GPIO7 =	6,
+	SKY1_IOMUXC_GPIO8 =	7,
+	SKY1_IOMUXC_GPIO9 =	8,
+	SKY1_IOMUXC_GPIO10 =	9,
+	SKY1_IOMUXC_GPIO11 =	10,
+	SKY1_IOMUXC_GPIO12 =	11,
+	SKY1_IOMUXC_GPIO13 =	12,
+	SKY1_IOMUXC_GPIO14 =	13,
+	SKY1_IOMUXC_RSMRST_L =	14,
+	SKY1_IOMUXC_SRST_L =	15,
+	SKY1_IOMUXC_SLP_S3_L =	16,
+	SKY1_IOMUXC_SLP_S5_L =	17,
+	SKY1_IOMUXC_PWRGD =	18,
+	SKY1_IOMUXC_PWROK =	19,
+	SKY1_IOMUXC_PWRBTN_L =	20,
+	SKY1_IOMUXC_VDD_DDRIO_GATE =	21,
+	SKY1_IOMUXC_JTAG_GPIO_L =	22,
+	SKY1_IOMUXC_JTAG_TCK =		23,
+	SKY1_IOMUXC_JTAG_TDI =		24,
+	SKY1_IOMUXC_JTAG_TDO =		25,
+	SKY1_IOMUXC_TMS =		26,
+	SKY1_IOMUXC_TRST_L  =		27,
+	SKY1_IOMUXC_SFI_I2C0_SCL = 28,
+	SKY1_IOMUXC_SFI_I2C0_SDA = 29,
+	SKY1_IOMUXC_SFI_I2C1_SCL = 30,
+	SKY1_IOMUXC_SFI_I2C1_SDA = 31,
+	SKY1_IOMUXC_SFI_GPIO0	 = 32,
+	SKY1_IOMUXC_SFI_GPIO1	 = 33,
+	SKY1_IOMUXC_SFI_GPIO2	 = 34,
+	SKY1_IOMUXC_SFI_GPIO3	 = 35,
+	SKY1_IOMUXC_SFI_GPIO4	 = 36,
+	SKY1_IOMUXC_SFI_GPIO5	 = 37,
+	SKY1_IOMUXC_SFI_GPIO6	 = 38,
+	SKY1_IOMUXC_SFI_GPIO7	 = 39,
+	SKY1_IOMUXC_SFI_GPIO8	 = 40,
+	SKY1_IOMUXC_SFI_GPIO9	 = 41,
+	SKY1_IOMUXC_SPI1_MISO	 = 42,
+	SKY1_IOMUXC_SPI1_CS0	 = 43,
+	SKY1_IOMUXC_SPI1_CS1	 = 44,
+	SKY1_IOMUXC_SPI1_MOSI	 = 45,
+	SKY1_IOMUXC_SPI1_CLK	 = 46,
+	SKY1_IOMUXC_USB_OC0_L	 = 47,
+	SKY1_IOMUXC_USB_OC1_L	 = 48,
+	SKY1_IOMUXC_USB_OC2_L	 = 49,
+	SKY1_IOMUXC_USB_OC3_L	 = 50,
+	SKY1_IOMUXC_USB_OC4_L	 = 51,
+	SKY1_IOMUXC_USB_OC5_L	 = 52,
+	SKY1_IOMUXC_USB_OC6_L	 = 53,
+	SKY1_IOMUXC_USB_OC7_L	 = 54,
+	SKY1_IOMUXC_USB_OC8_L	 = 55,
+	SKY1_IOMUXC_USB_OC9_L	 = 56,
+	SKY1_IOMUXC_DRIVE_VBUS0	 = 57,
+	SKY1_IOMUXC_DRIVE_VBUS4	 = 58,
+	SKY1_IOMUXC_DRIVE_VBUS5  = 59,
+	SKY1_IOMUXC_SE_QSPI_CLK =  60,
+	SKY1_IOMUXC_SE_QSPI_CS_L = 61,
+	SKY1_IOMUXC_SE_QSPI_DATA0 = 62,
+	SKY1_IOMUXC_SE_QSPI_DATA1 = 63,
+	SKY1_IOMUXC_SE_QSPI_DATA2 = 64,
+	SKY1_IOMUXC_SE_QSPI_DATA3 = 65,
+};
+
+enum sky1_pads {
+	SKY1_IOMUXC_GPIO43	  = 0,
+	SKY1_IOMUXC_GPIO44	  = 1,
+	SKY1_IOMUXC_GPIO45	  = 2,
+	SKY1_IOMUXC_GPIO46	  = 3,
+	SKY1_IOMUXC_RESET_IN_L	  = 4,
+	SKY1_IOMUXC_PLT_RESET_L	  = 5,
+	SKY1_IOMUXC_THERMRIP_L	  = 6,
+	SKY1_IOMUXC_PROCHOT_L	  = 7,
+	SKY1_IOMUXC_PM_I2C0_CLK	  = 8,
+	SKY1_IOMUXC_PM_I2C0_DATA  = 9,
+	SKY1_IOMUXC_PM_I2C1_CLK	  = 10,
+	SKY1_IOMUXC_PM_I2C1_DATA  = 11,
+	SKY1_IOMUXC_PM_I2C2_CLK	  = 12,
+	SKY1_IOMUXC_PM_I2C2_DATA  = 13,
+	SKY1_IOMUXC_PM_I2C3_CLK	  = 14,
+	SKY1_IOMUXC_PM_I2C3_DATA  = 15,
+	SKY1_IOMUXC_STRAP0	  = 16,
+	SKY1_IOMUXC_STRAP1	  = 17,
+	SKY1_IOMUXC_DP2_DIGON	  = 18,
+	SKY1_IOMUXC_DP2_BLON	  = 19,
+	SKY1_IOMUXC_DP2_VARY_BL	  = 20,
+	SKY1_IOMUXC_I2C7_SCL	  = 21,
+	SKY1_IOMUXC_I2C7_SDA	  = 22,
+	SKY1_IOMUXC_UART6_CSU_SE_TXD = 23,
+	SKY1_IOMUXC_CLK_REQ1_L	  = 24,
+	SKY1_IOMUXC_CLK_REQ3_L	  = 25,
+	SKY1_IOMUXC_I2C5_SCL	  = 26,
+	SKY1_IOMUXC_I2C5_SDA	  = 27,
+	SKY1_IOMUXC_I2C6_SCL	  = 28,
+	SKY1_IOMUXC_I2C6_SDA	  = 29,
+	SKY1_IOMUXC_I2C0_CLK	  = 30,
+	SKY1_IOMUXC_I2C0_SDA	  = 31,
+	SKY1_IOMUXC_I2C1_CLK	  = 32,
+	SKY1_IOMUXC_I2C1_SDA	  = 33,
+	SKY1_IOMUXC_I2C2_SCL	  = 34,
+	SKY1_IOMUXC_I2C2_SDA	  = 35,
+	SKY1_IOMUXC_I3C0_PUR_EN_L = 36,
+	SKY1_IOMUXC_I2C3_CLK	  = 37,
+	SKY1_IOMUXC_I2C3_SDA	  = 38,
+	SKY1_IOMUXC_I3C1_PUR_EN_L = 39,
+	SKY1_IOMUXC_I2C4_CLK	  = 40,
+	SKY1_IOMUXC_I2C4_SDA	  = 41,
+	SKY1_IOMUXC_HDA_BITCLK	  = 42,
+	SKY1_IOMUXC_HDA_RST_L	  = 43,
+	SKY1_IOMUXC_HDA_SDIN0	  = 44,
+	SKY1_IOMUXC_HDA_SDOUT0	  = 45,
+	SKY1_IOMUXC_HDA_SYNC	  = 46,
+	SKY1_IOMUXC_HDA_SDIN1	  = 47,
+	SKY1_IOMUXC_HDA_SDOUT1	  = 48,
+	SKY1_IOMUXC_I2S1_MCLK	  = 49,
+	SKY1_IOMUXC_I2S1_SCK	  = 50,
+	SKY1_IOMUXC_I2S1_WS	  = 51,
+	SKY1_IOMUXC_I2S1_DATA_IN  = 52,
+	SKY1_IOMUXC_I2S1_DATA_OUT = 53,
+	SKY1_IOMUXC_I2S2_MCLK	  = 54,
+	SKY1_IOMUXC_I2S2_RSCK	  = 55,
+	SKY1_IOMUXC_I2S2_RWS	  = 56,
+	SKY1_IOMUXC_I2S2_TSCK	  = 57,
+	SKY1_IOMUXC_I2S2_TWS	  = 58,
+	SKY1_IOMUXC_I2S2_DATA_IN0  = 59,
+	SKY1_IOMUXC_I2S2_DATA_IN1  = 60,
+	SKY1_IOMUXC_I2S2_DATA_OUT0 = 61,
+	SKY1_IOMUXC_I2S2_DATA_OUT1 = 62,
+	SKY1_IOMUXC_I2S2_DATA_OUT2 = 63,
+	SKY1_IOMUXC_I2S2_DATA_OUT3 = 64,
+	SKY1_IOMUXC_I2S3_MCLK	  = 65,
+	SKY1_IOMUXC_I2S3_RSCK	  = 66,
+	SKY1_IOMUXC_I2S3_RWS	  = 67,
+	SKY1_IOMUXC_I2S3_TSCK	  = 68,
+	SKY1_IOMUXC_I2S3_TWS	  = 69,
+	SKY1_IOMUXC_I2S3_DATA_IN0  = 70,
+	SKY1_IOMUXC_I2S3_DATA_IN1  = 71,
+	SKY1_IOMUXC_I2S3_DATA_OUT0 = 72,
+	SKY1_IOMUXC_I2S3_DATA_OUT1 = 73,
+	SKY1_IOMUXC_I2S4_MCLK_LB  = 74,
+	SKY1_IOMUXC_I2S4_SCK_LB	  = 75,
+	SKY1_IOMUXC_I2S4_WS_LB	  = 76,
+	SKY1_IOMUXC_I2S4_DATA_IN_LB  = 77,
+	SKY1_IOMUXC_I2S4_DATA_OUT_LB = 78,
+	SKY1_IOMUXC_UART0_TXD	  = 79,
+	SKY1_IOMUXC_UART0_RXD	  = 80,
+	SKY1_IOMUXC_UART0_CTS	  = 81,
+	SKY1_IOMUXC_UART0_RTS	  = 82,
+	SKY1_IOMUXC_UART1_TXD	  = 83,
+	SKY1_IOMUXC_UART1_RXD	  = 84,
+	SKY1_IOMUXC_UART1_CTS	  = 85,
+	SKY1_IOMUXC_UART1_RTS	  = 86,
+	SKY1_IOMUXC_UART2_TXD	  = 87,
+	SKY1_IOMUXC_UART2_RXD	  = 88,
+	SKY1_IOMUXC_UART3_TXD	  = 89,
+	SKY1_IOMUXC_UART3_RXD	  = 90,
+	SKY1_IOMUXC_UART3_CTS	  = 91,
+	SKY1_IOMUXC_UART3_RTS	  = 92,
+	SKY1_IOMUXC_UART4_CSU_PM_TXD = 93,
+	SKY1_IOMUXC_UART4_CSU_PM_RXD = 94,
+	SKY1_IOMUXC_UART5_CSU_SE_TXD = 95,
+	SKY1_IOMUXC_UART5_CSU_SE_RXD = 96,
+	SKY1_IOMUXC_UART6_CSU_SE_RXD = 97,
+	SKY1_IOMUXC_CLK_REQ0_L	     = 98,
+	SKY1_IOMUXC_CLK_REQ2_L	     = 99,
+	SKY1_IOMUXC_CLK_REQ4_L	     = 100,
+	SKY1_IOMUXC_CSI0_MCLK0	     = 101,
+	SKY1_IOMUXC_CSI0_MCLK1	     = 102,
+	SKY1_IOMUXC_CSI1_MCLK0	     = 103,
+	SKY1_IOMUXC_CSI1_MCLK1	     = 104,
+	SKY1_IOMUXC_GMAC0_REFCLK_25M = 105,
+	SKY1_IOMUXC_GMAC0_TX_CTL     = 106,
+	SKY1_IOMUXC_GMAC0_TXD0	     = 107,
+	SKY1_IOMUXC_GMAC0_TXD1	     = 108,
+	SKY1_IOMUXC_GMAC0_TXD2	     = 109,
+	SKY1_IOMUXC_GMAC0_TXD3	     = 110,
+	SKY1_IOMUXC_GMAC0_TX_CLK     = 111,
+	SKY1_IOMUXC_GMAC0_RX_CTL     = 112,
+	SKY1_IOMUXC_GMAC0_RXD0	     = 113,
+	SKY1_IOMUXC_GMAC0_RXD1	     = 114,
+	SKY1_IOMUXC_GMAC0_RXD2	     = 115,
+	SKY1_IOMUXC_GMAC0_RXD3	     = 116,
+	SKY1_IOMUXC_GMAC0_RX_CLK     = 117,
+	SKY1_IOMUXC_GMAC0_MDC	     = 118,
+	SKY1_IOMUXC_GMAC0_MDIO	     = 119,
+	SKY1_IOMUXC_GMAC1_REFCLK_25M = 120,
+	SKY1_IOMUXC_GMAC1_TX_CTL     = 121,
+	SKY1_IOMUXC_GMAC1_TXD0	     = 122,
+	SKY1_IOMUXC_GMAC1_TXD1	     = 123,
+	SKY1_IOMUXC_GMAC1_TXD2	     = 124,
+	SKY1_IOMUXC_GMAC1_TXD3	     = 125,
+	SKY1_IOMUXC_GMAC1_TX_CLK     = 126,
+	SKY1_IOMUXC_GMAC1_RX_CTL     = 127,
+	SKY1_IOMUXC_GMAC1_RXD0	     = 128,
+	SKY1_IOMUXC_GMAC1_RXD1	     = 129,
+	SKY1_IOMUXC_GMAC1_RXD2	     = 130,
+	SKY1_IOMUXC_GMAC1_RXD3	     = 131,
+	SKY1_IOMUXC_GMAC1_RX_CLK     = 132,
+	SKY1_IOMUXC_GMAC1_MDC	     = 133,
+	SKY1_IOMUXC_GMAC1_MDIO	     = 134,
+	SKY1_IOMUXC_PM_GPIO0	     = 135,
+	SKY1_IOMUXC_PM_GPIO1	     = 136,
+	SKY1_IOMUXC_PM_GPIO2	     = 137,
+};
+
+/* Pad names for the s5 domain pinmux subsystem */
+static const struct pinctrl_pin_desc sky1_pinctrl_s5_pads[] = {
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO4),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO5),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO6),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO7),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO8),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO9),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO10),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO11),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO12),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO13),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO14),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_RSMRST_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SRST_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SLP_S3_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SLP_S5_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PWRGD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PWROK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PWRBTN_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_VDD_DDRIO_GATE),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_JTAG_GPIO_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_JTAG_TCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_JTAG_TDI),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_JTAG_TDO),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_TMS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_TRST_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_I2C0_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_I2C0_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_I2C1_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_I2C1_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO4),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO5),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO6),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO7),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO8),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SFI_GPIO9),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SPI1_MISO),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SPI1_CS0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SPI1_CS1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SPI1_MOSI),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SPI1_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC0_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC1_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC2_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC3_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC4_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC5_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC6_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC7_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC8_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_USB_OC9_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DRIVE_VBUS0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DRIVE_VBUS4),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DRIVE_VBUS5),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_CS_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_DATA0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_DATA1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_DATA2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_SE_QSPI_DATA3),
+};
+
+/* Pad names for the s0 domain pinmux subsystem */
+static const struct pinctrl_pin_desc sky1_pinctrl_pads[] = {
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO43),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO44),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO45),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GPIO46),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_RESET_IN_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PLT_RESET_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_THERMRIP_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PROCHOT_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C0_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C0_DATA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C1_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C1_DATA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C2_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C2_DATA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C3_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_I2C3_DATA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_STRAP0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_STRAP1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DP2_DIGON),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DP2_BLON),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_DP2_VARY_BL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C7_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C7_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART6_CSU_SE_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CLK_REQ1_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CLK_REQ3_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C5_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C5_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C6_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C6_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C0_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C0_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C1_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C1_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C2_SCL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C2_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I3C0_PUR_EN_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C3_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C3_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I3C1_PUR_EN_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C4_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2C4_SDA),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_BITCLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_RST_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_SDIN0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_SDOUT0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_SYNC),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_SDIN1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_HDA_SDOUT1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S1_MCLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S1_SCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S1_WS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S1_DATA_IN),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S1_DATA_OUT),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_MCLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_RSCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_RWS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_TSCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_TWS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_IN0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_IN1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_OUT0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_OUT1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_OUT2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S2_DATA_OUT3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_MCLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_RSCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_RWS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_TSCK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_TWS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_DATA_IN0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_DATA_IN1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_DATA_OUT0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S3_DATA_OUT1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S4_MCLK_LB),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S4_SCK_LB),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S4_WS_LB),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S4_DATA_IN_LB),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_I2S4_DATA_OUT_LB),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART0_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART0_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART0_CTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART0_RTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART1_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART1_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART1_CTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART1_RTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART2_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART2_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART3_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART3_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART3_CTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART3_RTS),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART4_CSU_PM_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART4_CSU_PM_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART5_CSU_SE_TXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART5_CSU_SE_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_UART6_CSU_SE_RXD),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CLK_REQ0_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CLK_REQ2_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CLK_REQ4_L),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CSI0_MCLK0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CSI0_MCLK1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CSI1_MCLK0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_CSI1_MCLK1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_REFCLK_25M),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TX_CTL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TXD0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TXD1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TXD2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TXD3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_TX_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RX_CTL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RXD0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RXD1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RXD2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RXD3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_RX_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_MDC),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC0_MDIO),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_REFCLK_25M),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TX_CTL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TXD0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TXD1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TXD2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TXD3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_TX_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RX_CTL),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RXD0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RXD1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RXD2),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RXD3),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_RX_CLK),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_MDC),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_GMAC1_MDIO),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_GPIO0),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_GPIO1),
+	SKY1_PINCTRL_PIN(SKY1_IOMUXC_PM_GPIO2),
+};
+
+static const struct sky1_pinctrl_soc_info sky1_pinctrl_s5_info = {
+	.pins = sky1_pinctrl_s5_pads,
+	.npins = ARRAY_SIZE(sky1_pinctrl_s5_pads),
+};
+
+static const struct sky1_pinctrl_soc_info sky1_pinctrl_info = {
+	.pins = sky1_pinctrl_pads,
+	.npins = ARRAY_SIZE(sky1_pinctrl_pads),
+};
+
+static const struct of_device_id sky1_pinctrl_of_match[] = {
+	{ .compatible = "cix,sky1-iomuxc-s5", .data = &sky1_pinctrl_s5_info, },
+	{ .compatible = "cix,sky1-iomuxc", .data = &sky1_pinctrl_info, },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sky1_pinctrl_of_match);
+
+static int __maybe_unused sky1_pinctrl_suspend(struct device *dev)
+{
+	struct sky1_pinctrl *spctl = dev_get_drvdata(dev);
+
+	return pinctrl_force_sleep(spctl->pctl);
+}
+
+static int __maybe_unused sky1_pinctrl_resume(struct device *dev)
+{
+	struct sky1_pinctrl *spctl = dev_get_drvdata(dev);
+
+	return pinctrl_force_default(spctl->pctl);
+}
+
+const struct dev_pm_ops sky1_pinctrl_pm_ops = {
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(sky1_pinctrl_suspend,
+					sky1_pinctrl_resume)
+};
+EXPORT_SYMBOL_GPL(sky1_pinctrl_pm_ops);
+
+static int sky1_pinctrl_probe(struct platform_device *pdev)
+{
+	const struct sky1_pinctrl_soc_info *pinctrl_info;
+
+	pinctrl_info = device_get_match_data(&pdev->dev);
+	if (!pinctrl_info)
+		return -ENODEV;
+
+	return sky1_base_pinctrl_probe(pdev, pinctrl_info);
+}
+
+static struct platform_driver sky1_pinctrl_driver = {
+	.driver = {
+		.name = "sky1-pinctrl",
+		.of_match_table = of_match_ptr(sky1_pinctrl_of_match),
+		.pm = &sky1_pinctrl_pm_ops,
+	},
+	.probe = sky1_pinctrl_probe,
+};
+
+static int __init sky1_pinctrl_init(void)
+{
+	return platform_driver_register(&sky1_pinctrl_driver);
+}
+arch_initcall(sky1_pinctrl_init);
+
+MODULE_AUTHOR("Jerry Zhu <Jerry.Zhu@cixtech.com>");
+MODULE_DESCRIPTION("Cix Sky1 pinctrl driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pinctrl/cix/pinctrl-sky1.h b/drivers/pinctrl/cix/pinctrl-sky1.h
new file mode 100644
index 000000000000..09b25dbb6db3
--- /dev/null
+++ b/drivers/pinctrl/cix/pinctrl-sky1.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Author: Jerry Zhu <Jerry.Zhu@cixtech.com>
+ */
+
+#ifndef __DRIVERS_PINCTRL_SKY1_H
+#define __DRIVERS_PINCTRL_SKY1_H
+
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinmux.h>
+
+/**
+ * struct sky1_pin - describes a single SKY1 pin
+ * @pin_id: the pin id of this pin
+ * @offest: the iomux register offset
+ * @configs: the mux and config value for pin
+ */
+struct sky1_pin {
+	unsigned int pin_id;
+	unsigned int offset;
+	unsigned long configs;
+};
+
+/**
+ * sky1_pin_reg contains 32 bits
+ * bit7:bit8 for function select
+ * bit0:bit6 for pad configuration
+ */
+typedef u32 sky1_pin_reg;
+
+/**
+ * @dev: a pointer back to containing device
+ * @base: the offset to the controller in virtual memory
+ */
+struct sky1_pinctrl {
+	struct device *dev;
+	struct pinctrl_dev *pctl;
+	void __iomem *base;
+	const struct sky1_pinctrl_soc_info *info;
+	sky1_pin_reg *pin_regs;
+	unsigned int group_index;
+	struct mutex mutex;
+};
+
+struct sky1_pinctrl_soc_info {
+	const struct pinctrl_pin_desc *pins;
+	unsigned int npins;
+};
+
+#define SKY1_PINCTRL_PIN(pin) PINCTRL_PIN(pin, #pin)
+
+int sky1_base_pinctrl_probe(struct platform_device *pdev,
+			const struct sky1_pinctrl_soc_info *info);
+
+#endif /* __DRIVERS_PINCTRL_SKY1_H */
-- 
2.49.0

Re: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 27/08/2025 04:42, Gary Yang wrote:
> +
> +static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
> +				struct sky1_pinctrl *spctl)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *child;
> +	struct pinctrl_dev *pctl = spctl->pctl;
> +	u32 nfuncs = 0;
> +	u32 i = 0;
> +	bool flat_funcs;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	flat_funcs = sky1_pinctrl_dt_is_flat_functions(np);
> +	if (flat_funcs) {
> +		nfuncs = 1;
> +	} else {
> +		nfuncs = of_get_child_count(np);
> +		if (nfuncs == 0) {
> +			dev_err(&pdev->dev, "no functions defined\n");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	for (i = 0; i < nfuncs; i++) {
> +		struct function_desc *function;
> +
> +		function = devm_kzalloc(&pdev->dev, sizeof(*function),
> +					GFP_KERNEL);
> +		if (!function)
> +			return -ENOMEM;
> +
> +		mutex_lock(&spctl->mutex);
> +		radix_tree_insert(&pctl->pin_function_tree, i, function);
> +		mutex_unlock(&spctl->mutex);
> +	}
> +	pctl->num_functions = nfuncs;
> +
> +	spctl->group_index = 0;
> +	if (flat_funcs) {
> +		pctl->num_groups = of_get_child_count(np);
> +	} else {
> +		pctl->num_groups = 0;
> +		for_each_child_of_node(np, child)
> +			pctl->num_groups += of_get_child_count(child);
> +	}
> +
> +	if (flat_funcs) {
> +		sky1_pinctrl_parse_functions(np, spctl, 0);
> +	} else {
> +		i = 0;
> +		for_each_child_of_node(np, child)
> +			sky1_pinctrl_parse_functions(child, spctl, i++);
> +	}
> +
> +	return 0;
> +}
> +
> +int sky1_base_pinctrl_probe(struct platform_device *pdev,
> +		      const struct sky1_pinctrl_soc_info *info)
> +{
> +	struct pinctrl_desc *sky1_pinctrl_desc;
> +	struct sky1_pinctrl *spctl;
> +	int ret, i;
> +
> +	if (!info || !info->pins || !info->npins) {
> +		dev_err(&pdev->dev, "wrong pinctrl info\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Create state holders etc for this driver */
> +	spctl = devm_kzalloc(&pdev->dev, sizeof(*spctl), GFP_KERNEL);
> +	if (!spctl)
> +		return -ENOMEM;
> +
> +	spctl->pin_regs = devm_kmalloc_array(&pdev->dev, info->npins,
> +					    sizeof(*spctl->pin_regs),
> +					    GFP_KERNEL);
> +	if (!spctl->pin_regs)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < info->npins; i++)
> +		spctl->pin_regs[i] = -1;
> +
> +	spctl->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(spctl->base))
> +		return PTR_ERR(spctl->base);
> +
> +	sky1_pinctrl_desc = devm_kzalloc(&pdev->dev, sizeof(*sky1_pinctrl_desc),
> +					GFP_KERNEL);
> +	if (!sky1_pinctrl_desc)
> +		return -ENOMEM;
> +
> +	sky1_pinctrl_desc->name = dev_name(&pdev->dev);
> +	sky1_pinctrl_desc->pins = info->pins;
> +	sky1_pinctrl_desc->npins = info->npins;
> +	sky1_pinctrl_desc->pctlops = &sky1_pctrl_ops;
> +	sky1_pinctrl_desc->pmxops = &sky1_pmx_ops;
> +	sky1_pinctrl_desc->confops = &sky1_pinconf_ops;
> +	sky1_pinctrl_desc->owner = THIS_MODULE;
> +
> +	mutex_init(&spctl->mutex);
> +
> +	spctl->info = info;
> +	spctl->dev = &pdev->dev;
> +	platform_set_drvdata(pdev, spctl);
> +	ret = devm_pinctrl_register_and_init(&pdev->dev,
> +					     sky1_pinctrl_desc, spctl,
> +					     &spctl->pctl);
> +	if (ret) {
> +		dev_err(&pdev->dev, "could not register SKY1 pinctrl driver\n");
> +		return ret;
> +	}
> +
> +	ret = sky1_pinctrl_probe_dt(pdev, spctl);
> +

No blank line here.

> +	if (ret) {
> +		dev_err(&pdev->dev, "fail to probe dt properties\n");

You are printing same error twice. Drop this and just handle error
printing in sky1_pinctrl_probe_dt().
Especially that you now print errors on ENOMEM.

> +		return ret;
> +	}
> +
> +	pinctrl_provide_dummies();
> +	dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");


No, please drop. Drivers should be silent on success.

> +
> +	return pinctrl_enable(spctl->pctl);
> +}
> +EXPORT_SYMBOL_GPL(sky1_base_pinctrl_probe);
> +



..

> +
> +static struct platform_driver sky1_pinctrl_driver = {
> +	.driver = {
> +		.name = "sky1-pinctrl",
> +		.of_match_table = of_match_ptr(sky1_pinctrl_of_match),


You have a warning here - please drop of_match_ptr.

> +		.pm = &sky1_pinctrl_pm_ops,
> +	},
> +	.probe = sky1_pinctrl_probe,
> +};
> +
> +static int __init sky1_pinctrl_init(void)
> +{
> +	return platform_driver_register(&sky1_pinctrl_driver);
> +}
> +arch_initcall(sky1_pinctrl_init);
> +
> +MODULE_AUTHOR("Jerry Zhu <Jerry.Zhu@cixtech.com>");
> +MODULE_DESCRIPTION("Cix Sky1 pinctrl driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/pinctrl/cix/pinctrl-sky1.h b/drivers/pinctrl/cix/pinctrl-sky1.h
> new file mode 100644
> index 000000000000..09b25dbb6db3
> --- /dev/null
> +++ b/drivers/pinctrl/cix/pinctrl-sky1.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Author: Jerry Zhu <Jerry.Zhu@cixtech.com>
> + */
> +
> +#ifndef __DRIVERS_PINCTRL_SKY1_H
> +#define __DRIVERS_PINCTRL_SKY1_H
> +
> +#include <linux/pinctrl/pinconf-generic.h>
> +#include <linux/pinctrl/pinmux.h>

Are you sure you use both headers in this header?

Best regards,
Krzysztof

回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Gary Yang]

Hi Krzysztof,
> 
> On 27/08/2025 04:42, Gary Yang wrote:
> > +
> > +static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
> > +                             struct sky1_pinctrl *spctl) {
> > +     struct device_node *np = pdev->dev.of_node;
> > +     struct device_node *child;
> > +     struct pinctrl_dev *pctl = spctl->pctl;
> > +     u32 nfuncs = 0;
> > +     u32 i = 0;
> > +     bool flat_funcs;
> > +
> > +     if (!np)
> > +             return -ENODEV;
> > +
> > +     flat_funcs = sky1_pinctrl_dt_is_flat_functions(np);
> > +     if (flat_funcs) {
> > +             nfuncs = 1;
> > +     } else {
> > +             nfuncs = of_get_child_count(np);
> > +             if (nfuncs == 0) {
> > +                     dev_err(&pdev->dev, "no functions defined\n");
> > +                     return -EINVAL;
> > +             }
> > +     }
> > +
> > +     for (i = 0; i < nfuncs; i++) {
> > +             struct function_desc *function;
> > +
> > +             function = devm_kzalloc(&pdev->dev, sizeof(*function),
> > +                                     GFP_KERNEL);
> > +             if (!function)
> > +                     return -ENOMEM;
> > +
> > +             mutex_lock(&spctl->mutex);
> > +             radix_tree_insert(&pctl->pin_function_tree, i, function);
> > +             mutex_unlock(&spctl->mutex);
> > +     }
> > +     pctl->num_functions = nfuncs;
> > +
> > +     spctl->group_index = 0;
> > +     if (flat_funcs) {
> > +             pctl->num_groups = of_get_child_count(np);
> > +     } else {
> > +             pctl->num_groups = 0;
> > +             for_each_child_of_node(np, child)
> > +                     pctl->num_groups += of_get_child_count(child);
> > +     }
> > +
> > +     if (flat_funcs) {
> > +             sky1_pinctrl_parse_functions(np, spctl, 0);
> > +     } else {
> > +             i = 0;
> > +             for_each_child_of_node(np, child)
> > +                     sky1_pinctrl_parse_functions(child, spctl, i++);
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +int sky1_base_pinctrl_probe(struct platform_device *pdev,
> > +                   const struct sky1_pinctrl_soc_info *info) {
> > +     struct pinctrl_desc *sky1_pinctrl_desc;
> > +     struct sky1_pinctrl *spctl;
> > +     int ret, i;
> > +
> > +     if (!info || !info->pins || !info->npins) {
> > +             dev_err(&pdev->dev, "wrong pinctrl info\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     /* Create state holders etc for this driver */
> > +     spctl = devm_kzalloc(&pdev->dev, sizeof(*spctl), GFP_KERNEL);
> > +     if (!spctl)
> > +             return -ENOMEM;
> > +
> > +     spctl->pin_regs = devm_kmalloc_array(&pdev->dev, info->npins,
> > +                                         sizeof(*spctl->pin_regs),
> > +                                         GFP_KERNEL);
> > +     if (!spctl->pin_regs)
> > +             return -ENOMEM;
> > +
> > +     for (i = 0; i < info->npins; i++)
> > +             spctl->pin_regs[i] = -1;
> > +
> > +     spctl->base = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(spctl->base))
> > +             return PTR_ERR(spctl->base);
> > +
> > +     sky1_pinctrl_desc = devm_kzalloc(&pdev->dev,
> sizeof(*sky1_pinctrl_desc),
> > +                                     GFP_KERNEL);
> > +     if (!sky1_pinctrl_desc)
> > +             return -ENOMEM;
> > +
> > +     sky1_pinctrl_desc->name = dev_name(&pdev->dev);
> > +     sky1_pinctrl_desc->pins = info->pins;
> > +     sky1_pinctrl_desc->npins = info->npins;
> > +     sky1_pinctrl_desc->pctlops = &sky1_pctrl_ops;
> > +     sky1_pinctrl_desc->pmxops = &sky1_pmx_ops;
> > +     sky1_pinctrl_desc->confops = &sky1_pinconf_ops;
> > +     sky1_pinctrl_desc->owner = THIS_MODULE;
> > +
> > +     mutex_init(&spctl->mutex);
> > +
> > +     spctl->info = info;
> > +     spctl->dev = &pdev->dev;
> > +     platform_set_drvdata(pdev, spctl);
> > +     ret = devm_pinctrl_register_and_init(&pdev->dev,
> > +                                          sky1_pinctrl_desc, spctl,
> > +                                          &spctl->pctl);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "could not register SKY1 pinctrl
> driver\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = sky1_pinctrl_probe_dt(pdev, spctl);
> > +
> 
> No blank line here.

OK, we will delete this blank line next version

> 
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "fail to probe dt properties\n");
> 
> You are printing same error twice. Drop this and just handle error printing in
> sky1_pinctrl_probe_dt().
> Especially that you now print errors on ENOMEM.
> 

Sorry, this print message is only once, not twice, please give more information

> > +             return ret;
> > +     }
> > +
> > +     pinctrl_provide_dummies();
> > +     dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
> 
> 
> No, please drop. Drivers should be silent on success.
> 

Ok, replace dev_info with dev_dbg next version

> > +
> > +     return pinctrl_enable(spctl->pctl); }
> > +EXPORT_SYMBOL_GPL(sky1_base_pinctrl_probe);
> > +
> 
> 
> 
> ..
> 
> > +
> > +static struct platform_driver sky1_pinctrl_driver = {
> > +     .driver = {
> > +             .name = "sky1-pinctrl",
> > +             .of_match_table = of_match_ptr(sky1_pinctrl_of_match),
> 
> 
> You have a warning here - please drop of_match_ptr.
> 

We will delete it on next version

> > +             .pm = &sky1_pinctrl_pm_ops,
> > +     },
> > +     .probe = sky1_pinctrl_probe,
> > +};
> > +
> > +static int __init sky1_pinctrl_init(void) {
> > +     return platform_driver_register(&sky1_pinctrl_driver);
> > +}
> > +arch_initcall(sky1_pinctrl_init);
> > +
> > +MODULE_AUTHOR("Jerry Zhu <Jerry.Zhu@cixtech.com>");
> > +MODULE_DESCRIPTION("Cix Sky1 pinctrl driver"); MODULE_LICENSE("GPL");
> > diff --git a/drivers/pinctrl/cix/pinctrl-sky1.h
> > b/drivers/pinctrl/cix/pinctrl-sky1.h
> > new file mode 100644
> > index 000000000000..09b25dbb6db3
> > --- /dev/null
> > +++ b/drivers/pinctrl/cix/pinctrl-sky1.h
> > @@ -0,0 +1,55 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +/*
> > + * Author: Jerry Zhu <Jerry.Zhu@cixtech.com>  */
> > +
> > +#ifndef __DRIVERS_PINCTRL_SKY1_H
> > +#define __DRIVERS_PINCTRL_SKY1_H
> > +
> > +#include <linux/pinctrl/pinconf-generic.h> #include
> > +<linux/pinctrl/pinmux.h>
> 
> Are you sure you use both headers in this header?
> 

We will delete them on next version

> Best regards,
> Krzysztof

Best wishes
Gary

Re: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 28/08/2025 08:44, Gary Yang wrote:
>>
>>> +     if (ret) {
>>> +             dev_err(&pdev->dev, "fail to probe dt properties\n");
>>
>> You are printing same error twice. Drop this and just handle error printing in
>> sky1_pinctrl_probe_dt().
>> Especially that you now print errors on ENOMEM.
>>
> 
> Sorry, this print message is only once, not twice, please give more information

Trigger the error and check how many error messages you see. I see two.
You should know your code better than me...

> 
>>> +             return ret;
>>> +     }
>>> +
>>> +     pinctrl_provide_dummies();
>>> +     dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
>>
>>
>> No, please drop. Drivers should be silent on success.
>>
> 
> Ok, replace dev_info with dev_dbg next version


No, it is completely redundant. You are duplicating existing mechanism
for no gain at all.

Best regards,
Krzysztof

回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Gary Yang]

Hi Krzysztof,

> 
> On 28/08/2025 08:44, Gary Yang wrote:
> >>
> >>> +     if (ret) {
> >>> +             dev_err(&pdev->dev, "fail to probe dt properties\n");
> >>
> >> You are printing same error twice. Drop this and just handle error
> >> printing in sky1_pinctrl_probe_dt().
> >> Especially that you now print errors on ENOMEM.
> >>
> >
> > Sorry, this print message is only once, not twice, please give more
> > information
> 
> Trigger the error and check how many error messages you see. I see two.
> You should know your code better than me...
> 

There are two pin-controller on sky1. They share the same driver. The probe is called twice. 

So we see the print message twice.

> >
> >>> +             return ret;
> >>> +     }
> >>> +
> >>> +     pinctrl_provide_dummies();
> >>> +     dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
> >>
> >>
> >> No, please drop. Drivers should be silent on success.
> >>
> >
> > Ok, replace dev_info with dev_dbg next version
> 
> 
> No, it is completely redundant. You are duplicating existing mechanism for no
> gain at all.
> 

OK, we delete it on next version

> Best regards,
> Krzysztof

Best wishes
Gary

Re: 回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 28/08/2025 10:32, Gary Yang wrote:
> Hi Krzysztof,
> 
>>
>> On 28/08/2025 08:44, Gary Yang wrote:
>>>>
>>>>> +     if (ret) {
>>>>> +             dev_err(&pdev->dev, "fail to probe dt properties\n");
>>>>
>>>> You are printing same error twice. Drop this and just handle error
>>>> printing in sky1_pinctrl_probe_dt().
>>>> Especially that you now print errors on ENOMEM.
>>>>
>>>
>>> Sorry, this print message is only once, not twice, please give more
>>> information
>>
>> Trigger the error and check how many error messages you see. I see two.
>> You should know your code better than me...
>>
> 
> There are two pin-controller on sky1. They share the same driver. The probe is called twice. 
> 
> So we see the print message twice.


No, you don't really understand how this works. Test your code and its
error paths and you will see FOR ONE BIND more than one error message.
Plus my second comment which you completely ignored.

I am sorry, but this is basic C.

Best regards,
Krzysztof

回复: 回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Gary Yang]

Hi Krzysztof,
> 
> On 28/08/2025 10:32, Gary Yang wrote:
> > Hi Krzysztof,
> >
> >>
> >> On 28/08/2025 08:44, Gary Yang wrote:
> >>>>
> >>>>> +     if (ret) {
> >>>>> +             dev_err(&pdev->dev, "fail to probe dt
> >>>>> + properties\n");
> >>>>
> >>>> You are printing same error twice. Drop this and just handle error
> >>>> printing in sky1_pinctrl_probe_dt().
> >>>> Especially that you now print errors on ENOMEM.
> >>>>
> >>>
> >>> Sorry, this print message is only once, not twice, please give more
> >>> information
> >>
> >> Trigger the error and check how many error messages you see. I see two.
> >> You should know your code better than me...
> >>
> >
> > There are two pin-controller on sky1. They share the same driver. The probe
> is called twice.
> >
> > So we see the print message twice.
> 
> 
> No, you don't really understand how this works. Test your code and its error
> paths and you will see FOR ONE BIND more than one error message.
> Plus my second comment which you completely ignored.
> 
> I am sorry, but this is basic C.
> 

In order to trigger a error, we add a sentence in sky1_pinctrl_probe_dt() as follow:

static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
                                 struct sky1_pinctrl *spctl)
{

+         return -ENODEV;
          .......
}

dmesg shows as following:

[    0.812780] /soc@0/pinctrl@4170000: Fixed dependency cycle(s) with /soc@0/pinctrl@4170000/hog-pins
[    0.821920] sky1-pinctrl 4170000.pinctrl: fail to probe dt properties
[    0.828503] /soc@0/pinctrl@16007000: Fixed dependency cycle(s) with /soc@0/pinctrl@16007000/hog-s5-pins
[    0.838058] sky1-pinctrl 16007000.pinctrl: fail to probe dt properties

I don't see the error message twice per one. There are two pin-controller. One is /soc@0/pinctrl@4170000. Other is /soc@0/pinctrl@16007000.

So you see the twice, once per one pin-controller. BTW as you suggested before, we will print the value of ret in the error message.

If I miss any information, please kindly remind me. Thanks

> Best regards,
> Krzysztof

Best wishes
Gary

Re: 回复: 回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 29/08/2025 06:33, Gary Yang wrote:
> Hi Krzysztof,
>>
>> On 28/08/2025 10:32, Gary Yang wrote:
>>> Hi Krzysztof,
>>>
>>>>
>>>> On 28/08/2025 08:44, Gary Yang wrote:
>>>>>>
>>>>>>> +     if (ret) {
>>>>>>> +             dev_err(&pdev->dev, "fail to probe dt
>>>>>>> + properties\n");
>>>>>>
>>>>>> You are printing same error twice. Drop this and just handle error
>>>>>> printing in sky1_pinctrl_probe_dt().
>>>>>> Especially that you now print errors on ENOMEM.
>>>>>>
>>>>>
>>>>> Sorry, this print message is only once, not twice, please give more
>>>>> information
>>>>
>>>> Trigger the error and check how many error messages you see. I see two.
>>>> You should know your code better than me...
>>>>
>>>
>>> There are two pin-controller on sky1. They share the same driver. The probe
>> is called twice.
>>>
>>> So we see the print message twice.
>>
>>
>> No, you don't really understand how this works. Test your code and its error
>> paths and you will see FOR ONE BIND more than one error message.
>> Plus my second comment which you completely ignored.
>>
>> I am sorry, but this is basic C.
>>
> 
> In order to trigger a error, we add a sentence in sky1_pinctrl_probe_dt() as follow:
> 
> static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
>                                  struct sky1_pinctrl *spctl)
> {
> 
> +         return -ENODEV;
>           .......
> }
> 
> dmesg shows as following:
> 
> [    0.812780] /soc@0/pinctrl@4170000: Fixed dependency cycle(s) with /soc@0/pinctrl@4170000/hog-pins
> [    0.821920] sky1-pinctrl 4170000.pinctrl: fail to probe dt properties
> [    0.828503] /soc@0/pinctrl@16007000: Fixed dependency cycle(s) with /soc@0/pinctrl@16007000/hog-s5-pins
> [    0.838058] sky1-pinctrl 16007000.pinctrl: fail to probe dt properties
> 
> I don't see the error message twice per one. There are two pin-controller. One is /soc@0/pinctrl@4170000. Other is /soc@0/pinctrl@16007000.

And the next error case from sky1_pinctrl_probe_dt? ... and then the
next one? And another one?

Really, either you didn't read your own code or you just push the same
poor code, regardless of review, because you want it to get merged?

This will lead you nowhere.

You have:

+static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
+				struct sky1_pinctrl *spctl)
...
+		if (!function)
+			return -ENOMEM;
...
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		return ret;
+	}

That's a clear NAK.

Then you have:

+		if (nfuncs == 0) {
+			dev_err(&pdev->dev, "no functions defined\n");
+			return -EINVAL;
...
+	if (ret) {
+		dev_err(&pdev->dev, "fail to probe dt properties\n");
+		return ret;
+	}

that's useless duplicated message. TWICE.

You could easily spot it yourself instead of keep bugging the reviewer
for such trivial stuff.

NAK, please remember to never waste reviewers time.

> 
> So you see the twice, once per one pin-controller. BTW as you suggested before, we will print the value of ret in the error message.
> 
> If I miss any information, please kindly remind me. Thanks

You still ignored my second comment.


Best regards,
Krzysztof

回复: 回复: 回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Gary Yang]

Hi Krzysztof,

> 
> On 29/08/2025 06:33, Gary Yang wrote:
> > Hi Krzysztof,
> >>
> >> On 28/08/2025 10:32, Gary Yang wrote:
> >>> Hi Krzysztof,
> >>>
> >>>>
> >>>> On 28/08/2025 08:44, Gary Yang wrote:
> >>>>>>
> >>>>>>> +     if (ret) {
> >>>>>>> +             dev_err(&pdev->dev, "fail to probe dt
> >>>>>>> + properties\n");
> >>>>>>
> >>>>>> You are printing same error twice. Drop this and just handle
> >>>>>> error printing in sky1_pinctrl_probe_dt().
> >>>>>> Especially that you now print errors on ENOMEM.
> >>>>>>
> >>>>>
> >>>>> Sorry, this print message is only once, not twice, please give
> >>>>> more information
> >>>>
> >>>> Trigger the error and check how many error messages you see. I see two.
> >>>> You should know your code better than me...
> >>>>
> >>>
> >>> There are two pin-controller on sky1. They share the same driver.
> >>> The probe
> >> is called twice.
> >>>
> >>> So we see the print message twice.
> >>
> >>
> >> No, you don't really understand how this works. Test your code and
> >> its error paths and you will see FOR ONE BIND more than one error
> message.
> >> Plus my second comment which you completely ignored.
> >>
> >> I am sorry, but this is basic C.
> >>
> >
> > In order to trigger a error, we add a sentence in sky1_pinctrl_probe_dt() as
> follow:
> >
> > static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
> >                                  struct sky1_pinctrl *spctl) {
> >
> > +         return -ENODEV;
> >           .......
> > }
> >
> > dmesg shows as following:
> >
> > [    0.812780] /soc@0/pinctrl@4170000: Fixed dependency cycle(s) with
> /soc@0/pinctrl@4170000/hog-pins
> > [    0.821920] sky1-pinctrl 4170000.pinctrl: fail to probe dt properties
> > [    0.828503] /soc@0/pinctrl@16007000: Fixed dependency cycle(s) with
> /soc@0/pinctrl@16007000/hog-s5-pins
> > [    0.838058] sky1-pinctrl 16007000.pinctrl: fail to probe dt properties
> >
> > I don't see the error message twice per one. There are two pin-controller.
> One is /soc@0/pinctrl@4170000. Other is /soc@0/pinctrl@16007000.
> 
> And the next error case from sky1_pinctrl_probe_dt? ... and then the next one?
> And another one?
> 
> Really, either you didn't read your own code or you just push the same poor
> code, regardless of review, because you want it to get merged?
> 
> This will lead you nowhere.
> 
> You have:
> 
> +static int sky1_pinctrl_probe_dt(struct platform_device *pdev,
> +                               struct sky1_pinctrl *spctl)
> ...
> +               if (!function)
> +                       return -ENOMEM;
> ...
> +       if (ret) {
> +               dev_err(&pdev->dev, "fail to probe dt properties\n");
> +               return ret;
> +       }
> 
> That's a clear NAK.
> 
> Then you have:
> 
> +               if (nfuncs == 0) {
> +                       dev_err(&pdev->dev, "no functions defined\n");
> +                       return -EINVAL;
> ...
> +       if (ret) {
> +               dev_err(&pdev->dev, "fail to probe dt properties\n");
> +               return ret;
> +       }
> 
> that's useless duplicated message. TWICE.
> 
> You could easily spot it yourself instead of keep bugging the reviewer for such
> trivial stuff.
> 
> NAK, please remember to never waste reviewers time.
> 
> >
> > So you see the twice, once per one pin-controller. BTW as you suggested
> before, we will print the value of ret in the error message.
> >
> > If I miss any information, please kindly remind me. Thanks
> 
> You still ignored my second comment.
> 

First you wrote " You are printing same error twice ", please pay attention to the "same error" strings, it makes me confuse, I misunderstand your thinking 
until you took the example above. So the discussion is needed, not waste our time.

Second the return value of sky1_pinctrl_probe_dt() is not only ENOMEM, it can also return ENODEV and EINVAL. Although I don't think this is a bug, take our 
time consider, I will delete the print sentence in sky1_pinctrl_probe_dt(). It makes the error message only once.

Third Although the author is not me, I really read all driver codes before take my courage to submit this patch. Maybe the codes comes from old example, 
we can update these codes to support the newest standard.

Fourth I work as an kernel engineer many years. But I first push patches to Linux kernel. If I make some stupid mistake, please kindly remind me. Maybe your kind make me 
more courage on doing this valuable work. 

If miss any information, please let me know. Thanks for your understanding.

> 
> Best regards,
> Krzysztof

Best wishes
Gary

Re: 回复: 回复: 回复: 回复: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 29/08/2025 12:18, Gary Yang wrote:
>>>
>>> So you see the twice, once per one pin-controller. BTW as you suggested
>> before, we will print the value of ret in the error message.
>>>
>>> If I miss any information, please kindly remind me. Thanks
>>
>> You still ignored my second comment.

^^^ That was second time here I reminded about ENOMEM comment.

>>
> 
> First you wrote " You are printing same error twice ", please pay attention to the "same error" strings, it makes me confuse, I misunderstand your thinking 
> until you took the example above. So the discussion is needed, not waste our time.
> 
> Second the return value of sky1_pinctrl_probe_dt() is not only ENOMEM, it can also return ENODEV and EINVAL. Although I don't think this is a bug, take our

The problem is that you ignored the ENOMEM comment and it clearly should
direct you to the problem and solution ("If I cannot have error on
ENOMEM, I will move my error msgs to the called function").

Best regards,
Krzysztof

Re: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Linus Walleij]

On Wed, Aug 27, 2025 at 11:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 27/08/2025 04:42, Gary Yang wrote:

> > +     pinctrl_provide_dummies();
> > +     dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
>
>
> No, please drop. Drivers should be silent on success.

I usually think this is a matter of taste.

I suppose the reason why a lot of drivers have some "hello world" message
is that missing probe calls is sometimes an issue.

If you mistakenly disable the driver in Kconfig (or due to other Kconfig
changes that just happen ...), how do you know from the dmesg
what error you made, when comparing it to a successful boot. There
are no *error* messages from the driver either, just the same silence as
when it's enabled. With pinctrl, random completely unrelated stuff just
stops working.

If you see that the "hello world" from that driver is missing, you know it
isn't probing, instead of finding it out after combing through the .config
for the third time.

But I know a lot of people are dmesg minimalists, because it's just too
much information and they just want errors there. It makes sense in a
way too.

So as subsystem maintainer I have no hard opinion on it.

Yours,
Linus Walleij

Re: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Krzysztof Kozlowski]

On 28/08/2025 19:51, Linus Walleij wrote:
> On Wed, Aug 27, 2025 at 11:07 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>> On 27/08/2025 04:42, Gary Yang wrote:
> 
>>> +     pinctrl_provide_dummies();
>>> +     dev_info(&pdev->dev, "initialized SKY1 pinctrl driver\n");
>>
>>
>> No, please drop. Drivers should be silent on success.
> 
> I usually think this is a matter of taste.

It's actually coding style:
https://elixir.bootlin.com/linux/v6.15-rc1/source/Documentation/process/coding-style.rst#L913

https://elixir.bootlin.com/linux/v6.15-rc1/source/Documentation/process/debugging/driver_development_debugging_guide.rst#L79

https://lore.kernel.org/all/20191210143706.3928480-6-gregkh@linuxfoundation.org/

> 
> I suppose the reason why a lot of drivers have some "hello world" message
> is that missing probe calls is sometimes an issue.
> 
> If you mistakenly disable the driver in Kconfig (or due to other Kconfig
> changes that just happen ...), how do you know from the dmesg
> what error you made, when comparing it to a successful boot. There
> are no *error* messages from the driver either, just the same silence as
> when it's enabled. With pinctrl, random completely unrelated stuff just
> stops working.
> 
> If you see that the "hello world" from that driver is missing, you know it
> isn't probing, instead of finding it out after combing through the .config
> for the third time.

Any tests for driver success should be checking in sysfs, not in dmesg.
Most of platforms have smaller or bigger tests for that.

> 
> But I know a lot of people are dmesg minimalists, because it's just too
> much information and they just want errors there. It makes sense in a
> way too.
> 
> So as subsystem maintainer I have no hard opinion on it.

Best regards,
Krzysztof

Re: [PATCH 1/3] pinctrl: cix: Add pin-controller support for sky1

[Linus Walleij]

On Thu, Aug 28, 2025 at 8:02 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:

> > If you see that the "hello world" from that driver is missing, you know it
> > isn't probing, instead of finding it out after combing through the .config
> > for the third time.
>
> Any tests for driver success should be checking in sysfs, not in dmesg.
> Most of platforms have smaller or bigger tests for that.

Yeah I understand the thinking.

The typical symptom of a non-probed pin control driver is however
eg that the system does not mount root because some pins
connected to the eMMC are not muxed right.

People find the problem sooner or later anyway, it's just that the
print makes it sooner. And the mistake is pretty common (or
at least for me, but I'm not the best developer...)

Yours,
Linus Walleij

[PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Gary Yang]

Add dt-bindings docs

Signed-off-by: Gary Yang <gary.yang@cixtech.com>
---
 .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
 include/dt-bindings/pinctrl/pads-sky1.h       | 592 ++++++++++++++++++
 2 files changed, 669 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
 create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h

diff --git a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
new file mode 100644
index 000000000000..10a4a292188e
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
@@ -0,0 +1,77 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cix Sky1 Pin Controller
+
+maintainers:
+  - Gary Yang <gary.yang@cixtech.com>
+
+description:
+  Please refer to pinctrl-bindings.txt in this directory for common
+  binding part and usage.
+
+properties:
+  compatible:
+    enum:
+      - cix,sky1-iomuxc
+      - cix,sky1-iomuxc-s5
+
+  reg:
+    maxItems: 1
+
+# Client device subnode's properties
+patternProperties:
+  '-pins$':
+    type: object
+    description:
+      Pinctrl node's client devices use subnodes for desired pin configuration.
+      Client device subnodes use below standard properties.
+
+    properties:
+      cix,pins:
+        description:
+          each entry consists of 3 integers and represents the mux and config
+          setting for one pin. The first 2 integers <mux_reg func_num> are
+          specified using a CIX_PAD_* macro.The last integer CONFIG is the pad
+          setting value like pull-up on this pin.
+        $ref: /schemas/types.yaml#/definitions/uint32-matrix
+        items:
+          items:
+            - description: |
+                "mux_reg" indicates the offset of register.
+            - description: |
+                "func_num" indicates the mux value to be applied.
+            - description: |
+                "pad_setting" indicates the pad configuration value to be
+                applied.
+
+    required:
+      - cix,pins
+
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: pinctrl.yaml#
+
+additionalProperties: false
+
+examples:
+  # Pinmux controller node
+  - |
+    #include <dt-bindings/pinctrl/pads-sky1.h>
+    iomuxc: pinctrl@4170000 {
+        compatible = "cix,sky1-iomuxc";
+        reg = <0x4170000 0x1000>;
+
+        pinctrl_hog: hog-pins {
+            cix,pins =
+                <CIX_PAD_GPIO144_FUNC_GPIO144 (PULL_DOWN|DS_LEVEL4)>;
+        };
+    };
diff --git a/include/dt-bindings/pinctrl/pads-sky1.h b/include/dt-bindings/pinctrl/pads-sky1.h
new file mode 100644
index 000000000000..44550e4105b3
--- /dev/null
+++ b/include/dt-bindings/pinctrl/pads-sky1.h
@@ -0,0 +1,592 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright 2024-2025 Cix Technology Group Co., Ltd.
+ */
+
+#ifndef __SKY1_PADS_H
+#define __SKY1_PADS_H
+
+#define CIX_PAD_GPIO001_OFFSET			0x0
+#define CIX_PAD_GPIO002_OFFSET			0x4
+#define CIX_PAD_GPIO003_OFFSET			0x8
+#define CIX_PAD_GPIO004_OFFSET			0xc
+#define CIX_PAD_GPIO005_OFFSET			0x10
+#define CIX_PAD_GPIO006_OFFSET			0x14
+#define CIX_PAD_GPIO007_OFFSET			0x18
+#define CIX_PAD_GPIO008_OFFSET			0x1c
+#define CIX_PAD_GPIO009_OFFSET			0x20
+#define CIX_PAD_GPIO010_OFFSET			0x24
+#define CIX_PAD_GPIO011_OFFSET			0x28
+#define CIX_PAD_GPIO012_OFFSET			0x2c
+#define CIX_PAD_GPIO013_OFFSET			0x30
+#define CIX_PAD_GPIO014_OFFSET			0x34
+#define CIX_PAD_SFI_I2C0_SCL_OFFSET		0x70
+#define CIX_PAD_SFI_I2C0_SDA_OFFSET		0x74
+#define CIX_PAD_SFI_I2C1_SCL_OFFSET		0x78
+#define CIX_PAD_SFI_I2C1_SDA_OFFSET		0x7c
+#define CIX_PAD_SFI_GPIO0_OFFSET		0x80
+#define CIX_PAD_SFI_GPIO1_OFFSET		0x84
+#define CIX_PAD_SFI_GPIO2_OFFSET		0x88
+#define CIX_PAD_GPIO018_OFFSET			0x8c
+#define CIX_PAD_GPIO019_OFFSET			0x90
+#define CIX_PAD_GPIO020_OFFSET			0x94
+#define CIX_PAD_GPIO021_OFFSET			0x98
+#define CIX_PAD_GPIO022_OFFSET			0x9c
+#define CIX_PAD_GPIO023_OFFSET			0xa0
+#define CIX_PAD_GPIO024_OFFSET			0xa4
+#define CIX_PAD_SPI1_MISO_OFFSET		0xa8
+#define CIX_PAD_SPI1_CS0_OFFSET			0xac
+#define CIX_PAD_SPI1_CS1_OFFSET			0xb0
+#define CIX_PAD_SPI1_MOSI_OFFSET		0xb4
+#define CIX_PAD_SPI1_CLK_OFFSET			0xb8
+#define CIX_PAD_GPIO030_OFFSET			0xbc
+#define CIX_PAD_GPIO031_OFFSET			0xc0
+#define CIX_PAD_GPIO032_OFFSET			0xc4
+#define CIX_PAD_GPIO033_OFFSET			0xc8
+#define CIX_PAD_GPIO034_OFFSET			0xcc
+#define CIX_PAD_GPIO035_OFFSET			0xd0
+#define CIX_PAD_GPIO036_OFFSET			0xd4
+#define CIX_PAD_GPIO037_OFFSET			0xd8
+#define CIX_PAD_GPIO038_OFFSET			0xdc
+#define CIX_PAD_GPIO039_OFFSET			0xe0
+#define CIX_PAD_GPIO040_OFFSET			0xe4
+#define CIX_PAD_GPIO041_OFFSET			0xe8
+#define CIX_PAD_GPIO042_OFFSET			0xec
+#define CIX_PAD_SE_QSPI_CLK_OFFSET		0xf0
+#define CIX_PAD_SE_QSPI_CS_L_OFFSET		0xf4
+#define CIX_PAD_SE_QSPI_DATA0_OFFSET		0xf8
+#define CIX_PAD_SE_QSPI_DATA1_OFFSET		0xfc
+#define CIX_PAD_SE_QSPI_DATA2_OFFSET		0x100
+#define CIX_PAD_SE_QSPI_DATA3_OFFSET		0x104
+#define CIX_PAD_GPIO043_OFFSET			0x0
+#define CIX_PAD_GPIO044_OFFSET			0x4
+#define CIX_PAD_GPIO045_OFFSET			0x8
+#define CIX_PAD_GPIO046_OFFSET			0xc
+#define CIX_PAD_DP2_DIGON_OFFSET		0x48
+#define CIX_PAD_DP2_BLON_OFFSET			0x4c
+#define CIX_PAD_DP2_VARY_BL_OFFSET		0x50
+#define CIX_PAD_I2C7_SCL_OFFSET			0x54
+#define CIX_PAD_I2C7_SDA_OFFSET			0x58
+#define CIX_PAD_I2C5_SCL_OFFSET			0x68
+#define CIX_PAD_I2C5_SDA_OFFSET			0x6c
+#define CIX_PAD_I2C6_SCL_OFFSET			0x70
+#define CIX_PAD_I2C6_SDA_OFFSET			0x74
+#define CIX_PAD_I2C0_CLK_OFFSET			0x78
+#define CIX_PAD_I2C0_SDA_OFFSET			0x7c
+#define CIX_PAD_I2C1_CLK_OFFSET			0x80
+#define CIX_PAD_I2C1_SDA_OFFSET			0x84
+#define CIX_PAD_I2C2_SCL_OFFSET			0x88
+#define CIX_PAD_I2C2_SDA_OFFSET			0x8c
+#define CIX_PAD_GPIO057_OFFSET			0x90
+#define CIX_PAD_I2C3_CLK_OFFSET			0x94
+#define CIX_PAD_I2C3_SDA_OFFSET			0x98
+#define CIX_PAD_GPIO060_OFFSET			0x9c
+#define CIX_PAD_I2C4_CLK_OFFSET			0xa0
+#define CIX_PAD_I2C4_SDA_OFFSET			0xa4
+#define CIX_PAD_HDA_BITCLK_OFFSET		0xa8
+#define CIX_PAD_HDA_RST_L_OFFSET		0xac
+#define CIX_PAD_HDA_SDIN0_OFFSET		0xb0
+#define CIX_PAD_HDA_SDOUT0_OFFSET		0xb4
+#define CIX_PAD_HDA_SYNC_OFFSET			0xb8
+#define CIX_PAD_HDA_SDIN1_OFFSET		0xbc
+#define CIX_PAD_HDA_SDOUT1_OFFSET		0xc0
+#define CIX_PAD_I2S1_MCLK_OFFSET		0xc4
+#define CIX_PAD_I2S1_SCK_OFFSET			0xc8
+#define CIX_PAD_I2S1_WS_OFFSET			0xcc
+#define CIX_PAD_I2S1_DATA_IN_OFFSET		0xd0
+#define CIX_PAD_I2S1_DATA_OUT_OFFSET		0xd4
+#define CIX_PAD_I2S2_MCLK_OFFSET		0xd8
+#define CIX_PAD_I2S2_RSCK_OFFSET		0xdc
+#define CIX_PAD_I2S2_RWS_OFFSET			0xe0
+#define CIX_PAD_I2S2_TSCK_OFFSET		0xe4
+#define CIX_PAD_I2S2_TWS_OFFSET			0xe8
+#define CIX_PAD_I2S2_DATA_IN0_OFFSET		0xec
+#define CIX_PAD_I2S2_DATA_IN1_OFFSET		0xf0
+#define CIX_PAD_I2S2_DATA_OUT0_OFFSET		0xf4
+#define CIX_PAD_I2S2_DATA_OUT1_OFFSET		0xf8
+#define CIX_PAD_I2S2_DATA_OUT2_OFFSET		0xfc
+#define CIX_PAD_I2S2_DATA_OUT3_OFFSET		0x100
+#define CIX_PAD_I2S3_MCLK_OFFSET		0x104
+#define CIX_PAD_I2S3_RSCK_OFFSET		0x108
+#define CIX_PAD_I2S3_RWS_OFFSET			0x10c
+#define CIX_PAD_I2S3_TSCK_OFFSET		0x110
+#define CIX_PAD_I2S3_TWS_OFFSET			0x114
+#define CIX_PAD_I2S3_DATA_IN0_OFFSET		0x118
+#define CIX_PAD_I2S3_DATA_IN1_OFFSET		0x11c
+#define CIX_PAD_I2S3_DATA_OUT0_OFFSET		0x120
+#define CIX_PAD_I2S3_DATA_OUT1_OFFSET		0x124
+#define CIX_PAD_GPIO090_OFFSET			0x128
+#define CIX_PAD_GPIO091_OFFSET			0x12c
+#define CIX_PAD_GPIO092_OFFSET			0x130
+#define CIX_PAD_GPIO093_OFFSET			0x134
+#define CIX_PAD_GPIO094_OFFSET			0x138
+#define CIX_PAD_UART0_TXD_OFFSET		0x13c
+#define CIX_PAD_UART0_RXD_OFFSET		0x140
+#define CIX_PAD_UART0_CTS_OFFSET		0x144
+#define CIX_PAD_UART0_RTS_OFFSET		0x148
+#define CIX_PAD_UART1_TXD_OFFSET		0x14c
+#define CIX_PAD_UART1_RXD_OFFSET		0x150
+#define CIX_PAD_UART1_CTS_OFFSET		0x154
+#define CIX_PAD_UART1_RTS_OFFSET		0x158
+#define CIX_PAD_UART2_TXD_OFFSET		0x15c
+#define CIX_PAD_UART2_RXD_OFFSET		0x160
+#define CIX_PAD_UART3_TXD_OFFSET		0x164
+#define CIX_PAD_UART3_RXD_OFFSET		0x168
+#define CIX_PAD_UART3_CTS_OFFSET		0x16c
+#define CIX_PAD_UART3_RTS_OFFSET		0x170
+#define CIX_PAD_UART4_CSU_PM_TXD_OFFSET		0x174
+#define CIX_PAD_UART4_CSU_PM_RXD_OFFSET		0x178
+#define CIX_PAD_UART5_CSU_SE_TXD_OFFSET		0x17c
+#define CIX_PAD_UART5_CSU_SE_RXD_OFFSET		0x180
+#define CIX_PAD_UART6_CSU_SE_RXD_OFFSET		0x184
+#define CIX_PAD_CLK_REQ0_L_OFFSET		0x188
+#define CIX_PAD_CLK_REQ2_L_OFFSET		0x18c
+#define CIX_PAD_CLK_REQ4_L_OFFSET		0x190
+#define CIX_PAD_CSI0_MCLK0_OFFSET		0x194
+#define CIX_PAD_CSI0_MCLK1_OFFSET		0x198
+#define CIX_PAD_CSI1_MCLK0_OFFSET		0x19c
+#define CIX_PAD_CSI1_MCLK1_OFFSET		0x1a0
+#define CIX_PAD_GPIO121_OFFSET			0x1a4
+#define CIX_PAD_GPIO122_OFFSET			0x1a8
+#define CIX_PAD_GPIO123_OFFSET			0x1ac
+#define CIX_PAD_GPIO124_OFFSET			0x1b0
+#define CIX_PAD_GPIO125_OFFSET			0x1b4
+#define CIX_PAD_GPIO126_OFFSET			0x1b8
+#define CIX_PAD_GPIO127_OFFSET			0x1bc
+#define CIX_PAD_GPIO128_OFFSET			0x1c0
+#define CIX_PAD_GPIO129_OFFSET			0x1c4
+#define CIX_PAD_GPIO130_OFFSET			0x1c8
+#define CIX_PAD_GPIO131_OFFSET			0x1cc
+#define CIX_PAD_GPIO132_OFFSET			0x1d0
+#define CIX_PAD_GPIO133_OFFSET			0x1d4
+#define CIX_PAD_GPIO134_OFFSET			0x1d8
+#define CIX_PAD_GPIO135_OFFSET			0x1dc
+#define CIX_PAD_GPIO136_OFFSET			0x1e0
+#define CIX_PAD_GPIO137_OFFSET			0x1e4
+#define CIX_PAD_GPIO138_OFFSET			0x1e8
+#define CIX_PAD_GPIO139_OFFSET			0x1ec
+#define CIX_PAD_GPIO140_OFFSET			0x1f0
+#define CIX_PAD_GPIO141_OFFSET			0x1f4
+#define CIX_PAD_GPIO142_OFFSET			0x1f8
+#define CIX_PAD_GPIO143_OFFSET			0x1fc
+#define CIX_PAD_GPIO144_OFFSET			0x200
+#define CIX_PAD_GPIO145_OFFSET			0x204
+#define CIX_PAD_GPIO146_OFFSET			0x208
+#define CIX_PAD_GPIO147_OFFSET			0x20c
+#define CIX_PAD_GPIO148_OFFSET			0x210
+#define CIX_PAD_GPIO149_OFFSET			0x214
+#define CIX_PAD_GPIO150_OFFSET			0x218
+#define CIX_PAD_GPIO151_OFFSET			0x21c
+#define CIX_PAD_GPIO152_OFFSET			0x220
+#define CIX_PAD_GPIO153_OFFSET			0x224
+
+#define CIX_PAD_GPIO001_FUNC_GPIO001				   0x0
+#define CIX_PAD_GPIO002_FUNC_GPIO002				   0x0
+#define CIX_PAD_GPIO003_FUNC_GPIO003				   0x0
+#define CIX_PAD_GPIO004_FUNC_GPIO004				   0x0
+#define CIX_PAD_GPIO005_FUNC_GPIO005				   0x0
+#define CIX_PAD_GPIO006_FUNC_GPIO006				   0x0
+#define CIX_PAD_GPIO007_FUNC_GPIO007				   0x0
+#define CIX_PAD_GPIO008_FUNC_GPIO008				   0x0
+#define CIX_PAD_GPIO009_FUNC_GPIO009				   0x0
+#define CIX_PAD_GPIO010_FUNC_GPIO010				   0x0
+#define CIX_PAD_GPIO011_FUNC_GPIO011				   0x0
+#define CIX_PAD_GPIO012_FUNC_GPIO012				   0x0
+#define CIX_PAD_GPIO013_FUNC_GPIO013				   0x0
+#define CIX_PAD_GPIO014_FUNC_GPIO014				   0x0
+#define CIX_PAD_SFI_I2C0_SCL_FUNC_SFI_I2C0_SCL			   0x0
+#define CIX_PAD_SFI_I2C0_SCL_FUNC_SFI_I3C0_SCL			   0x1
+#define CIX_PAD_SFI_I2C0_SDA_FUNC_SFI_I2C0_SDA			   0x0
+#define CIX_PAD_SFI_I2C0_SDA_FUNC_SFI_I3C0_SDA			   0x1
+#define CIX_PAD_SFI_I2C1_SCL_FUNC_SFI_I2C1_SCL			   0x0
+#define CIX_PAD_SFI_I2C1_SCL_FUNC_SFI_I3C1_SCL			   0x1
+#define CIX_PAD_SFI_I2C1_SCL_FUNC_SFI_SPI_CS0			   0x2
+#define CIX_PAD_SFI_I2C1_SDA_FUNC_SFI_I2C1_SDA			   0x0
+#define CIX_PAD_SFI_I2C1_SDA_FUNC_SFI_I3C1_SDA			   0x1
+#define CIX_PAD_SFI_I2C1_SDA_FUNC_SFI_SPI_CS1			   0x2
+#define CIX_PAD_SFI_GPIO0_FUNC_GPIO015				   0x0
+#define CIX_PAD_SFI_GPIO0_FUNC_SFI_SPI_SCK			   0x1
+#define CIX_PAD_SFI_GPIO0_FUNC_SFI_GPIO0			   0x2
+#define CIX_PAD_SFI_GPIO1_FUNC_GPIO016				   0x0
+#define CIX_PAD_SFI_GPIO1_FUNC_SFI_SPI_MOSI			   0x1
+#define CIX_PAD_SFI_GPIO1_FUNC_SFI_GPIO1			   0x2
+#define CIX_PAD_SFI_GPIO2_FUNC_GPIO017				   0x0
+#define CIX_PAD_SFI_GPIO2_FUNC_SFI_SPI_MISO			   0x1
+#define CIX_PAD_SFI_GPIO2_FUNC_SFI_GPIO2			   0x2
+#define CIX_PAD_GPIO018_FUNC_SFI_GPIO3				   0x0
+#define CIX_PAD_GPIO018_FUNC_GPIO018				   0x1
+#define CIX_PAD_GPIO019_FUNC_SFI_GPIO4				   0x0
+#define CIX_PAD_GPIO019_FUNC_GPIO019				   0x1
+#define CIX_PAD_GPIO020_FUNC_SFI_GPIO5				   0x0
+#define CIX_PAD_GPIO020_FUNC_GPIO020				   0x1
+#define CIX_PAD_GPIO021_FUNC_SFI_GPIO6				   0x0
+#define CIX_PAD_GPIO021_FUNC_GPIO021				   0x1
+#define CIX_PAD_GPIO022_FUNC_SFI_GPIO7				   0x0
+#define CIX_PAD_GPIO022_FUNC_GPIO022				   0x1
+#define CIX_PAD_GPIO023_FUNC_SFI_GPIO8				   0x0
+#define CIX_PAD_GPIO023_FUNC_GPIO023				   0x1
+#define CIX_PAD_GPIO023_FUNC_SFI_I3C0_PUR_EN_L			   0x2
+#define CIX_PAD_GPIO024_FUNC_SFI_GPIO9				   0x0
+#define CIX_PAD_GPIO024_FUNC_GPIO024				   0x1
+#define CIX_PAD_GPIO024_FUNC_SFI_I3C1_PUR_EN_L			   0x2
+#define CIX_PAD_SPI1_MISO_FUNC_SPI1_MISO			   0x0
+#define CIX_PAD_SPI1_MISO_FUNC_GPIO025				   0x1
+#define CIX_PAD_SPI1_CS0_FUNC_SPI1_CS0				   0x0
+#define CIX_PAD_SPI1_CS0_FUNC_GPIO026				   0x1
+#define CIX_PAD_SPI1_CS1_FUNC_SPI1_CS1				   0x0
+#define CIX_PAD_SPI1_CS1_FUNC_GPIO027				   0x1
+#define CIX_PAD_SPI1_MOSI_FUNC_SPI1_MOSI			   0x0
+#define CIX_PAD_SPI1_MOSI_FUNC_GPIO028				   0x1
+#define CIX_PAD_SPI1_CLK_FUNC_SPI1_CLK				   0x0
+#define CIX_PAD_SPI1_CLK_FUNC_GPIO029				   0x1
+#define CIX_PAD_GPIO030_FUNC_GPIO030				   0x0
+#define CIX_PAD_GPIO030_FUNC_USB_OC0_L				   0x1
+#define CIX_PAD_GPIO031_FUNC_GPIO031				   0x0
+#define CIX_PAD_GPIO031_FUNC_USB_OC1_L				   0x1
+#define CIX_PAD_GPIO032_FUNC_GPIO032				   0x0
+#define CIX_PAD_GPIO032_FUNC_USB_OC2_L				   0x1
+#define CIX_PAD_GPIO033_FUNC_GPIO033				   0x0
+#define CIX_PAD_GPIO033_FUNC_USB_OC3_L				   0x1
+#define CIX_PAD_GPIO034_FUNC_GPIO034				   0x0
+#define CIX_PAD_GPIO034_FUNC_USB_OC4_L				   0x1
+#define CIX_PAD_GPIO035_FUNC_GPIO035				   0x0
+#define CIX_PAD_GPIO035_FUNC_USB_OC5_L				   0x1
+#define CIX_PAD_GPIO036_FUNC_GPIO036				   0x0
+#define CIX_PAD_GPIO036_FUNC_USB_OC6_L				   0x1
+#define CIX_PAD_GPIO037_FUNC_GPIO037				   0x0
+#define CIX_PAD_GPIO037_FUNC_USB_OC7_L				   0x1
+#define CIX_PAD_GPIO038_FUNC_GPIO038				   0x0
+#define CIX_PAD_GPIO038_FUNC_USB_OC8_L				   0x1
+#define CIX_PAD_GPIO039_FUNC_GPIO039				   0x0
+#define CIX_PAD_GPIO039_FUNC_USB_OC9_L				   0x1
+#define CIX_PAD_GPIO040_FUNC_GPIO040				   0x0
+#define CIX_PAD_GPIO040_FUNC_USB_DRIVE_VBUS0			   0x1
+#define CIX_PAD_GPIO041_FUNC_GPIO041				   0x0
+#define CIX_PAD_GPIO041_FUNC_USB_DRIVE_VBUS4			   0x1
+#define CIX_PAD_GPIO042_FUNC_GPIO042				   0x0
+#define CIX_PAD_GPIO042_FUNC_USB_DRIVE_VBUS5			   0x1
+#define CIX_PAD_SE_QSPI_CLK_FUNC_SE_QSPI_CLK			   0x0
+#define CIX_PAD_SE_QSPI_CLK_FUNC_QSPI_CLK			   0x1
+#define CIX_PAD_SE_QSPI_CS_L_FUNC_SE_QSPI_CS_L			   0x0
+#define CIX_PAD_SE_QSPI_CS_L_FUNC_QSPI_CS_L			   0x1
+#define CIX_PAD_SE_QSPI_DATA0_FUNC_SE_QSPI_DATA0		   0x0
+#define CIX_PAD_SE_QSPI_DATA0_FUNC_QSPI_DATA0			   0x1
+#define CIX_PAD_SE_QSPI_DATA1_FUNC_SE_QSPI_DATA1		   0x0
+#define CIX_PAD_SE_QSPI_DATA1_FUNC_QSPI_DATA1			   0x1
+#define CIX_PAD_SE_QSPI_DATA2_FUNC_SE_QSPI_DATA2		   0x0
+#define CIX_PAD_SE_QSPI_DATA2_FUNC_QSPI_DATA2			   0x1
+#define CIX_PAD_SE_QSPI_DATA3_FUNC_SE_QSPI_DATA3		   0x0
+#define CIX_PAD_SE_QSPI_DATA3_FUNC_QSPI_DATA3			   0x1
+#define CIX_PAD_GPIO043_FUNC_GPIO043				   0x0
+#define CIX_PAD_GPIO044_FUNC_GPIO044				   0x0
+#define CIX_PAD_GPIO045_FUNC_GPIO045				   0x0
+#define CIX_PAD_GPIO046_FUNC_GPIO046				   0x0
+#define CIX_PAD_DP2_DIGON_FUNC_DP2_DIGON			   0x0
+#define CIX_PAD_DP2_BLON_FUNC_DP2_BLON				   0x0
+#define CIX_PAD_DP2_VARY_BL_FUNC_DP2_VARY_BL			   0x0
+#define CIX_PAD_I2C7_SCL_FUNC_I2C7_SCL				   0x0
+#define CIX_PAD_I2C7_SDA_FUNC_I2C7_SDA				   0x0
+#define CIX_PAD_I2C5_SCL_FUNC_I2C5_SCL				   0x0
+#define CIX_PAD_I2C5_SCL_FUNC_GPIO047				   0x1
+#define CIX_PAD_I2C5_SDA_FUNC_I2C5_SDA				   0x0
+#define CIX_PAD_I2C5_SDA_FUNC_GPIO048				   0x1
+#define CIX_PAD_I2C6_SCL_FUNC_I2C6_SCL				   0x0
+#define CIX_PAD_I2C6_SCL_FUNC_GPIO049				   0x1
+#define CIX_PAD_I2C6_SDA_FUNC_I2C6_SDA				   0x0
+#define CIX_PAD_I2C6_SDA_FUNC_GPIO050				   0x1
+#define CIX_PAD_I2C0_CLK_FUNC_I2C0_CLK				   0x0
+#define CIX_PAD_I2C0_CLK_FUNC_GPIO051				   0x1
+#define CIX_PAD_I2C0_SDA_FUNC_I2C0_SDA				   0x0
+#define CIX_PAD_I2C0_SDA_FUNC_GPIO052				   0x1
+#define CIX_PAD_I2C1_CLK_FUNC_I2C1_CLK				   0x0
+#define CIX_PAD_I2C1_CLK_FUNC_GPIO053				   0x1
+#define CIX_PAD_I2C1_SDA_FUNC_I2C1_SDA				   0x0
+#define CIX_PAD_I2C1_SDA_FUNC_GPIO054				   0x1
+#define CIX_PAD_I2C2_SCL_FUNC_I2C2_SCL				   0x0
+#define CIX_PAD_I2C2_SCL_FUNC_I3C0_SCL				   0x1
+#define CIX_PAD_I2C2_SCL_FUNC_GPIO055				   0x2
+#define CIX_PAD_I2C2_SDA_FUNC_I2C2_SDA				   0x0
+#define CIX_PAD_I2C2_SDA_FUNC_I3C0_SDA				   0x1
+#define CIX_PAD_I2C2_SDA_FUNC_GPIO056				   0x2
+#define CIX_PAD_GPIO057_FUNC_GPIO057				   0x0
+#define CIX_PAD_GPIO057_FUNC_I3C0_PUR_EN_L			   0x1
+#define CIX_PAD_I2C3_CLK_FUNC_I2C3_CLK				   0x0
+#define CIX_PAD_I2C3_CLK_FUNC_I3C1_CLK				   0x1
+#define CIX_PAD_I2C3_CLK_FUNC_GPIO058				   0x2
+#define CIX_PAD_I2C3_SDA_FUNC_I2C3_SDA				   0x0
+#define CIX_PAD_I2C3_SDA_FUNC_I3C1_SDA				   0x1
+#define CIX_PAD_I2C3_SDA_FUNC_GPIO059				   0x2
+#define CIX_PAD_GPIO060_FUNC_GPIO060				   0x0
+#define CIX_PAD_GPIO060_FUNC_I3C1_PUR_EN_L			   0x1
+#define CIX_PAD_I2C4_CLK_FUNC_I2C4_CLK				   0x0
+#define CIX_PAD_I2C4_CLK_FUNC_GPIO061				   0x1
+#define CIX_PAD_I2C4_SDA_FUNC_I2C4_SDA				   0x0
+#define CIX_PAD_I2C4_SDA_FUNC_GPIO062				   0x1
+#define CIX_PAD_HDA_BITCLK_FUNC_HDA_BITCLK			   0x0
+#define CIX_PAD_HDA_BITCLK_FUNC_I2S0_SCK			   0x1
+#define CIX_PAD_HDA_BITCLK_FUNC_I2S9_RSCK_DBG			   0x2
+#define CIX_PAD_HDA_RST_L_FUNC_HDA_RST_L			   0x0
+#define CIX_PAD_HDA_RST_L_FUNC_I2S0_DATA_IN			   0x1
+#define CIX_PAD_HDA_RST_L_FUNC_I2S9_DATA_IN0_DBG		   0x2
+#define CIX_PAD_HDA_SDIN0_FUNC_HDA_SDIN0			   0x0
+#define CIX_PAD_HDA_SDIN0_FUNC_I2S0_MCLK			   0x1
+#define CIX_PAD_HDA_SDIN0_FUNC_I2S9_TSCK_DBG			   0x2
+#define CIX_PAD_HDA_SDOUT0_FUNC_HDA_SDOUT0			   0x0
+#define CIX_PAD_HDA_SDOUT0_FUNC_I2S0_DATA_OUT			   0x1
+#define CIX_PAD_HDA_SDOUT0_FUNC_I2S9_TWS_DBG		           0x2
+#define CIX_PAD_HDA_SYNC_FUNC_HDA_SYNC				   0x0
+#define CIX_PAD_HDA_SYNC_FUNC_I2S0_WS				   0x1
+#define CIX_PAD_HDA_SYNC_FUNC_I2S9_RWS_DBG			   0x2
+#define CIX_PAD_HDA_SDIN1_FUNC_HDA_SDIN1			   0x0
+#define CIX_PAD_HDA_SDIN1_FUNC_GPIO063				   0x1
+#define CIX_PAD_HDA_SDIN1_FUNC_I2S9_DATA_IN1_DBG		   0x2
+#define CIX_PAD_HDA_SDOUT1_FUNC_HDA_SDOUT1			   0x0
+#define CIX_PAD_HDA_SDOUT1_FUNC_GPIO064				   0x1
+#define CIX_PAD_HDA_SDOUT1_FUNC_I2S9_DATA_OUT0_DBG		   0x2
+#define CIX_PAD_I2S1_MCLK_FUNC_I2S1_MCLK			   0x0
+#define CIX_PAD_I2S1_MCLK_FUNC_GPIO065				   0x1
+#define CIX_PAD_I2S1_SCK_FUNC_I2S1_SCK				   0x0
+#define CIX_PAD_I2S1_SCK_FUNC_GPIO066				   0x1
+#define CIX_PAD_I2S1_WS_FUNC_I2S1_WS				   0x0
+#define CIX_PAD_I2S1_WS_FUNC_GPIO067				   0x1
+#define CIX_PAD_I2S1_DATA_IN_FUNC_I2S1_DATA_IN			   0x0
+#define CIX_PAD_I2S1_DATA_IN_FUNC_GPIO068			   0x1
+#define CIX_PAD_I2S1_DATA_OUT_FUNC_I2S1_DATA_OUT		   0x0
+#define CIX_PAD_I2S1_DATA_OUT_FUNC_GPIO069			   0x1
+#define CIX_PAD_I2S2_MCLK_FUNC_I2S2_MCLK			   0x0
+#define CIX_PAD_I2S2_MCLK_FUNC_GPIO070				   0x1
+#define CIX_PAD_I2S2_RSCK_FUNC_I2S2_RSCK			   0x0
+#define CIX_PAD_I2S2_RSCK_FUNC_GPIO071				   0x1
+#define CIX_PAD_I2S2_RSCK_FUNC_I2S5_RSCK_DBG			   0x2
+#define CIX_PAD_I2S2_RSCK_FUNC_I2S6_RSCK_DBG			   0x3
+#define CIX_PAD_I2S2_RWS_FUNC_I2S2_RWS				   0x0
+#define CIX_PAD_I2S2_RWS_FUNC_GPIO072				   0x1
+#define CIX_PAD_I2S2_RWS_FUNC_I2S5_RWS_DBG			   0x2
+#define CIX_PAD_I2S2_RWS_FUNC_I2S6_RWS_DBG			   0x3
+#define CIX_PAD_I2S2_TSCK_FUNC_I2S2_TSCK			   0x0
+#define CIX_PAD_I2S2_TSCK_FUNC_GPIO073				   0x1
+#define CIX_PAD_I2S2_TSCK_FUNC_I2S5_TSCK_DBG			   0x2
+#define CIX_PAD_I2S2_TSCK_FUNC_I2S6_TSCK_DBG			   0x3
+#define CIX_PAD_I2S2_TWS_FUNC_I2S2_TWS				   0x0
+#define CIX_PAD_I2S2_TWS_FUNC_GPIO074				   0x1
+#define CIX_PAD_I2S2_TWS_FUNC_I2S5_TWS_DBG			   0x2
+#define CIX_PAD_I2S2_TWS_FUNC_I2S6_TWS_DBG			   0x3
+#define CIX_PAD_I2S2_DATA_IN0_FUNC_I2S2_DATA_IN0		   0x0
+#define CIX_PAD_I2S2_DATA_IN0_FUNC_GPIO075			   0x1
+#define CIX_PAD_I2S2_DATA_IN0_FUNC_I2S5_DATA_IN0_DBG		   0x2
+#define CIX_PAD_I2S2_DATA_IN0_FUNC_I2S6_DATA_IN0_DBG		   0x3
+#define CIX_PAD_I2S2_DATA_IN1_FUNC_I2S2_DATA_IN1		   0x0
+#define CIX_PAD_I2S2_DATA_IN1_FUNC_GPIO076			   0x1
+#define CIX_PAD_I2S2_DATA_IN1_FUNC_I2S5_DATA_IN1_DBG		   0x2
+#define CIX_PAD_I2S2_DATA_IN1_FUNC_I2S6_DATA_IN1_DBG		   0x3
+#define CIX_PAD_I2S2_DATA_OUT0_FUNC_I2S2_DATA_OUT0		   0x0
+#define CIX_PAD_I2S2_DATA_OUT0_FUNC_GPIO077			   0x1
+#define CIX_PAD_I2S2_DATA_OUT0_FUNC_I2S5_DATA_OUT0_DBG		   0x2
+#define CIX_PAD_I2S2_DATA_OUT0_FUNC_I2S6_DATA_OUT0_DBG		   0x3
+#define CIX_PAD_I2S2_DATA_OUT1_FUNC_I2S2_DATA_OUT1		   0x0
+#define CIX_PAD_I2S2_DATA_OUT1_FUNC_GPIO078			   0x1
+#define CIX_PAD_I2S2_DATA_OUT1_FUNC_I2S5_DATA_OUT1_DBG		   0x2
+#define CIX_PAD_I2S2_DATA_OUT1_FUNC_I2S6_DATA_OUT1_DBG		   0x3
+#define CIX_PAD_I2S2_DATA_OUT2_FUNC_I2S2_DATA_OUT2		   0x0
+#define CIX_PAD_I2S2_DATA_OUT2_FUNC_GPIO079			   0x1
+#define CIX_PAD_I2S2_DATA_OUT3_FUNC_I2S2_DATA_OUT3		   0x0
+#define CIX_PAD_I2S2_DATA_OUT3_FUNC_GPIO080			   0x1
+#define CIX_PAD_I2S2_DATA_OUT3_FUNC_I2S9_DATA_OUT1_DBG		   0x2
+#define CIX_PAD_I2S3_MCLK_FUNC_I2S3_MCLK			   0x0
+#define CIX_PAD_I2S3_MCLK_FUNC_GPIO081				   0x1
+#define CIX_PAD_I2S3_RSCK_FUNC_I2S3_RSCK			   0x0
+#define CIX_PAD_I2S3_RSCK_FUNC_GPIO082				   0x1
+#define CIX_PAD_I2S3_RSCK_FUNC_I2S7_RSCK_DBG			   0x2
+#define CIX_PAD_I2S3_RSCK_FUNC_I2S8_RSCK_DBG			   0x3
+#define CIX_PAD_I2S3_RWS_FUNC_I2S3_RWS				   0x0
+#define CIX_PAD_I2S3_RWS_FUNC_GPIO083				   0x1
+#define CIX_PAD_I2S3_RWS_FUNC_I2S7_RWS_DBG			   0x2
+#define CIX_PAD_I2S3_RWS_FUNC_I2S8_RWS_DBG			   0x3
+#define CIX_PAD_I2S3_TSCK_FUNC_I2S3_TSCK			   0x0
+#define CIX_PAD_I2S3_TSCK_FUNC_GPIO084				   0x1
+#define CIX_PAD_I2S3_TSCK_FUNC_I2S7_TSCK_DBG			   0x2
+#define CIX_PAD_I2S3_TSCK_FUNC_I2S8_TSCK_DBG			   0x3
+#define CIX_PAD_I2S3_TWS_FUNC_I2S3_TWS				   0x0
+#define CIX_PAD_I2S3_TWS_FUNC_GPIO085				   0x1
+#define CIX_PAD_I2S3_TWS_FUNC_I2S7_TWS_DBG			   0x2
+#define CIX_PAD_I2S3_TWS_FUNC_I2S8_TWS_DBG			   0x3
+#define CIX_PAD_I2S3_DATA_IN0_FUNC_I2S3_DATA_IN0		   0x0
+#define CIX_PAD_I2S3_DATA_IN0_FUNC_GPIO086			   0x1
+#define CIX_PAD_I2S3_DATA_IN0_FUNC_I2S7_DATA_IN0_DBG		   0x2
+#define CIX_PAD_I2S3_DATA_IN0_FUNC_I2S8_DATA_IN0_DBG		   0x3
+#define CIX_PAD_I2S3_DATA_IN1_FUNC_I2S3_DATA_IN1		   0x0
+#define CIX_PAD_I2S3_DATA_IN1_FUNC_GPIO087			   0x1
+#define CIX_PAD_I2S3_DATA_IN1_FUNC_I2S7_DATA_IN1_DBG		   0x2
+#define CIX_PAD_I2S3_DATA_IN1_FUNC_I2S8_DATA_IN1_DBG		   0x3
+#define CIX_PAD_I2S3_DATA_OUT0_FUNC_I2S3_DATA_OUT0		   0x0
+#define CIX_PAD_I2S3_DATA_OUT0_FUNC_GPIO088			   0x1
+#define CIX_PAD_I2S3_DATA_OUT0_FUNC_I2S7_DATA_OUT0_DBG		   0x2
+#define CIX_PAD_I2S3_DATA_OUT0_FUNC_I2S8_DATA_OUT0_DBG		   0x3
+#define CIX_PAD_I2S3_DATA_OUT1_FUNC_I2S3_DATA_OUT1		   0x0
+#define CIX_PAD_I2S3_DATA_OUT1_FUNC_GPIO089			   0x1
+#define CIX_PAD_I2S3_DATA_OUT1_FUNC_I2S7_DATA_OUT1_DBG		   0x2
+#define CIX_PAD_I2S3_DATA_OUT1_FUNC_I2S8_DATA_OUT1_DBG		   0x3
+#define CIX_PAD_GPIO090_FUNC_GPIO090				   0x0
+#define CIX_PAD_GPIO090_FUNC_I2S4_MCLK_LB			   0x1
+#define CIX_PAD_GPIO091_FUNC_GPIO091				   0x0
+#define CIX_PAD_GPIO091_FUNC_I2S4_SCK_LB			   0x1
+#define CIX_PAD_GPIO092_FUNC_GPIO092				   0x0
+#define CIX_PAD_GPIO092_FUNC_I2S4_WS_LB				   0x1
+#define CIX_PAD_GPIO093_FUNC_GPIO093				   0x0
+#define CIX_PAD_GPIO093_FUNC_I2S4_DATA_IN_LB			   0x1
+#define CIX_PAD_GPIO094_FUNC_GPIO094				   0x0
+#define CIX_PAD_GPIO094_FUNC_I2S4_DATA_OUT_LB			   0x1
+#define CIX_PAD_UART0_TXD_FUNC_UART0_TXD			   0x0
+#define CIX_PAD_UART0_TXD_FUNC_PWM0				   0x1
+#define CIX_PAD_UART0_TXD_FUNC_GPIO095				   0x2
+#define CIX_PAD_UART0_RXD_FUNC_UART0_RXD			   0x0
+#define CIX_PAD_UART0_RXD_FUNC_PWM1				   0x1
+#define CIX_PAD_UART0_RXD_FUNC_GPIO096				   0x2
+#define CIX_PAD_UART0_CTS_FUNC_UART0_CTS			   0x0
+#define CIX_PAD_UART0_CTS_FUNC_FAN_OUT2				   0x1
+#define CIX_PAD_UART0_CTS_FUNC_GPIO097				   0x2
+#define CIX_PAD_UART0_RTS_FUNC_UART0_RTS			   0x0
+#define CIX_PAD_UART0_RTS_FUNC_FAN_TACH2			   0x1
+#define CIX_PAD_UART0_RTS_FUNC_GPIO098				   0x2
+#define CIX_PAD_UART1_TXD_FUNC_UART1_TXD			   0x0
+#define CIX_PAD_UART1_TXD_FUNC_FAN_OUT0				   0x1
+#define CIX_PAD_UART1_TXD_FUNC_GPIO099				   0x2
+#define CIX_PAD_UART1_RXD_FUNC_UART1_RXD			   0x0
+#define CIX_PAD_UART1_RXD_FUNC_FAN_TACH0			   0x1
+#define CIX_PAD_UART1_RXD_FUNC_GPIO100				   0x2
+#define CIX_PAD_UART1_CTS_FUNC_UART1_CTS			   0x0
+#define CIX_PAD_UART1_CTS_FUNC_FAN_OUT1				   0x1
+#define CIX_PAD_UART1_CTS_FUNC_GPIO101				   0x2
+#define CIX_PAD_UART1_RTS_FUNC_UART1_RTS			   0x0
+#define CIX_PAD_UART1_RTS_FUNC_FAN_TACH1			   0x1
+#define CIX_PAD_UART1_RTS_FUNC_GPIO102				   0x2
+#define CIX_PAD_UART2_TXD_FUNC_UART2_TXD			   0x0
+#define CIX_PAD_UART2_TXD_FUNC_GPIO103				   0x1
+#define CIX_PAD_UART2_RXD_FUNC_UART2_RXD			   0x0
+#define CIX_PAD_UART2_RXD_FUNC_GPIO104				   0x1
+#define CIX_PAD_UART3_TXD_FUNC_UART3_TXD			   0x0
+#define CIX_PAD_UART3_TXD_FUNC_GPIO105				   0x1
+#define CIX_PAD_UART3_RXD_FUNC_UART3_RXD			   0x0
+#define CIX_PAD_UART3_RXD_FUNC_GPIO106				   0x1
+#define CIX_PAD_UART3_CTS_FUNC_UART3_CTS			   0x0
+#define CIX_PAD_UART3_CTS_FUNC_GPIO107				   0x1
+#define CIX_PAD_UART3_CTS_FUNC_TRIGIN0				   0x2
+#define CIX_PAD_UART3_RTS_FUNC_UART3_RTS			   0x0
+#define CIX_PAD_UART3_RTS_FUNC_GPIO108				   0x1
+#define CIX_PAD_UART3_RTS_FUNC_TRIGIN1				   0x2
+#define CIX_PAD_UART4_CSU_PM_TXD_FUNC_UART4_CSU_PM_TXD		   0x0
+#define CIX_PAD_UART4_CSU_PM_TXD_FUNC_GPIO109			   0x1
+#define CIX_PAD_UART4_CSU_PM_RXD_FUNC_UART4_CSU_PM_RXD		   0x0
+#define CIX_PAD_UART4_CSU_PM_RXD_FUNC_GPIO110			   0x1
+#define CIX_PAD_UART5_CSU_SE_TXD_FUNC_UART5_CSU_SE_TXD		   0x0
+#define CIX_PAD_UART5_CSU_SE_TXD_FUNC_GPIO111			   0x1
+#define CIX_PAD_UART5_CSU_SE_RXD_FUNC_UART5_CSU_SE_RXD		   0x0
+#define CIX_PAD_UART5_CSU_SE_RXD_FUNC_GPIO112			   0x1
+#define CIX_PAD_UART6_CSU_SE_RXD_FUNC_UART6_CSU_SE_RXD		   0x0
+#define CIX_PAD_UART6_CSU_SE_RXD_FUNC_GPIO113			   0x1
+#define CIX_PAD_CLK_REQ0_L_FUNC_CLK_REQ0_L			   0x0
+#define CIX_PAD_CLK_REQ0_L_FUNC_GPIO114				   0x1
+#define CIX_PAD_CLK_REQ2_L_FUNC_CLK_REQ2_L			   0x0
+#define CIX_PAD_CLK_REQ2_L_FUNC_GPIO115				   0x1
+#define CIX_PAD_CLK_REQ4_L_FUNC_CLK_REQ4_L			   0x0
+#define CIX_PAD_CLK_REQ4_L_FUNC_GPIO116				   0x1
+#define CIX_PAD_CSI0_MCLK0_FUNC_CSI0_MCLK0			   0x0
+#define CIX_PAD_CSI0_MCLK0_FUNC_GPIO117				   0x1
+#define CIX_PAD_CSI0_MCLK1_FUNC_CSI0_MCLK1			   0x0
+#define CIX_PAD_CSI0_MCLK1_FUNC_GPIO118				   0x1
+#define CIX_PAD_CSI1_MCLK0_FUNC_CSI1_MCLK0			   0x0
+#define CIX_PAD_CSI1_MCLK0_FUNC_GPIO119				   0x1
+#define CIX_PAD_CSI1_MCLK1_FUNC_CSI1_MCLK1			   0x0
+#define CIX_PAD_CSI1_MCLK1_FUNC_GPIO120				   0x1
+#define CIX_PAD_GPIO121_FUNC_GPIO121				   0x0
+#define CIX_PAD_GPIO121_FUNC_GMAC0_REFCLK_25M			   0x1
+#define CIX_PAD_GPIO122_FUNC_GPIO122				   0x0
+#define CIX_PAD_GPIO122_FUNC_GMAC0_TX_CTL			   0x1
+#define CIX_PAD_GPIO123_FUNC_GPIO123				   0x0
+#define CIX_PAD_GPIO123_FUNC_GMAC0_TXD0				   0x1
+#define CIX_PAD_GPIO124_FUNC_GPIO124				   0x0
+#define CIX_PAD_GPIO124_FUNC_GMAC0_TXD1				   0x1
+#define CIX_PAD_GPIO125_FUNC_GPIO125				   0x0
+#define CIX_PAD_GPIO125_FUNC_GMAC0_TXD2				   0x1
+#define CIX_PAD_GPIO126_FUNC_GPIO126				   0x0
+#define CIX_PAD_GPIO126_FUNC_GMAC0_TXD3				   0x1
+#define CIX_PAD_GPIO127_FUNC_GPIO127				   0x0
+#define CIX_PAD_GPIO127_FUNC_GMAC0_TX_CLK			   0x1
+#define CIX_PAD_GPIO128_FUNC_GPIO128				   0x0
+#define CIX_PAD_GPIO128_FUNC_GMAC0_RX_CTL			   0x1
+#define CIX_PAD_GPIO129_FUNC_GPIO129				   0x0
+#define CIX_PAD_GPIO129_FUNC_GMAC0_RXD0				   0x1
+#define CIX_PAD_GPIO130_FUNC_GPIO130				   0x0
+#define CIX_PAD_GPIO130_FUNC_GMAC0_RXD1				   0x1
+#define CIX_PAD_GPIO131_FUNC_GPIO131				   0x0
+#define CIX_PAD_GPIO131_FUNC_GMAC0_RXD2				   0x1
+#define CIX_PAD_GPIO132_FUNC_GPIO132				   0x0
+#define CIX_PAD_GPIO132_FUNC_GMAC0_RXD3				   0x1
+#define CIX_PAD_GPIO133_FUNC_GPIO133				   0x0
+#define CIX_PAD_GPIO133_FUNC_GMAC0_RX_CLK			   0x1
+#define CIX_PAD_GPIO134_FUNC_GPIO134				   0x0
+#define CIX_PAD_GPIO134_FUNC_GMAC0_MDC				   0x1
+#define CIX_PAD_GPIO135_FUNC_GPIO135				   0x0
+#define CIX_PAD_GPIO135_FUNC_GMAC0_MDIO				   0x1
+#define CIX_PAD_GPIO136_FUNC_GPIO136				   0x0
+#define CIX_PAD_GPIO136_FUNC_GMAC1_REFCLK_25M			   0x1
+#define CIX_PAD_GPIO137_FUNC_GPIO137				   0x0
+#define CIX_PAD_GPIO137_FUNC_GMAC1_TX_CTL			   0x1
+#define CIX_PAD_GPIO138_FUNC_GPIO138				   0x0
+#define CIX_PAD_GPIO138_FUNC_GMAC1_TXD0				   0x1
+#define CIX_PAD_GPIO138_FUNC_SPI2_MISO				   0x2
+#define CIX_PAD_GPIO139_FUNC_GPIO139				   0x0
+#define CIX_PAD_GPIO139_FUNC_GMAC1_TXD1				   0x1
+#define CIX_PAD_GPIO139_FUNC_SPI2_CS0				   0x2
+#define CIX_PAD_GPIO140_FUNC_GPIO140				   0x0
+#define CIX_PAD_GPIO140_FUNC_GMAC1_TXD2				   0x1
+#define CIX_PAD_GPIO140_FUNC_SPI2_CS1				   0x2
+#define CIX_PAD_GPIO141_FUNC_GPIO141				   0x0
+#define CIX_PAD_GPIO141_FUNC_GMAC1_TXD3				   0x1
+#define CIX_PAD_GPIO141_FUNC_SPI2_MOSI				   0x2
+#define CIX_PAD_GPIO142_FUNC_GPIO142				   0x0
+#define CIX_PAD_GPIO142_FUNC_GMAC1_TX_CLK			   0x1
+#define CIX_PAD_GPIO142_FUNC_SPI2_CLK				   0x2
+#define CIX_PAD_GPIO143_FUNC_GPIO143				   0x0
+#define CIX_PAD_GPIO143_FUNC_GMAC1_RX_CTL			   0x1
+#define CIX_PAD_GPIO144_FUNC_GPIO144				   0x0
+#define CIX_PAD_GPIO144_FUNC_GMAC1_RXD0				   0x1
+#define CIX_PAD_GPIO145_FUNC_GPIO145				   0x0
+#define CIX_PAD_GPIO145_FUNC_GMAC1_RXD1				   0x1
+#define CIX_PAD_GPIO146_FUNC_GPIO146				   0x0
+#define CIX_PAD_GPIO146_FUNC_GMAC1_RXD2				   0x1
+#define CIX_PAD_GPIO147_FUNC_GPIO147				   0x0
+#define CIX_PAD_GPIO147_FUNC_GMAC1_RXD3				   0x1
+#define CIX_PAD_GPIO148_FUNC_GPIO148				   0x0
+#define CIX_PAD_GPIO148_FUNC_GMAC1_RX_CLK			   0x1
+#define CIX_PAD_GPIO149_FUNC_GPIO149				   0x0
+#define CIX_PAD_GPIO149_FUNC_GMAC1_MDC				   0x1
+#define CIX_PAD_GPIO150_FUNC_GPIO150				   0x0
+#define CIX_PAD_GPIO150_FUNC_GMAC1_MDIO				   0x1
+#define CIX_PAD_GPIO151_FUNC_GPIO151				   0x0
+#define CIX_PAD_GPIO151_FUNC_PM_GPIO0				   0x1
+#define CIX_PAD_GPIO152_FUNC_GPIO152				   0x0
+#define CIX_PAD_GPIO152_FUNC_PM_GPIO1				   0x1
+#define CIX_PAD_GPIO153_FUNC_GPIO153				   0x0
+#define CIX_PAD_GPIO153_FUNC_PM_GPIO2				   0x1
+
+#define PULL_UP		(1 << 6)
+#define PULL_DOWN	(1 << 5)
+#define ST		(1 << 4)
+#define DS_LEVEL1	0x1
+#define DS_LEVEL2	0x2
+#define DS_LEVEL3	0x3
+#define DS_LEVEL4	0x4
+#define DS_LEVEL5	0x5
+#define DS_LEVEL6	0x6
+#define DS_LEVEL7	0x7
+#define DS_LEVEL8	0x8
+#define DS_LEVEL9	0x9
+#define DS_LEVEL10	0xa
+#define DS_LEVEL11	0xb
+#define DS_LEVEL12	0xc
+#define DS_LEVEL13	0xd
+#define DS_LEVEL14	0xe
+#define DS_LEVEL15	0xf
+
+#endif
-- 
2.49.0

Re: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Krzysztof Kozlowski]

On 27/08/2025 04:42, Gary Yang wrote:
> Add dt-bindings docs

For what? Describe the hardware here in one, two sentences.

> 
> Signed-off-by: Gary Yang <gary.yang@cixtech.com>
> ---
>  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
>  include/dt-bindings/pinctrl/pads-sky1.h       | 592 ++++++++++++++++++
>  2 files changed, 669 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> new file mode 100644
> index 000000000000..10a4a292188e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> @@ -0,0 +1,77 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cix Sky1 Pin Controller
> +
> +maintainers:
> +  - Gary Yang <gary.yang@cixtech.com>
> +
> +description:
> +  Please refer to pinctrl-bindings.txt in this directory for common
> +  binding part and usage.

Drop description, not desired really.

> +
> +properties:
> +  compatible:
> +    enum:
> +      - cix,sky1-iomuxc
> +      - cix,sky1-iomuxc-s5

Whats the difference between? You have entire description field to
explain this but instead you said something obvious there.

> +
> +  reg:
> +    maxItems: 1
> +
> +# Client device subnode's properties
> +patternProperties:
> +  '-pins$':
> +    type: object
> +    description:
> +      Pinctrl node's client devices use subnodes for desired pin configuration.
> +      Client device subnodes use below standard properties.
> +
> +    properties:
> +      cix,pins:

No, use generic properties from pinmux schema.

You should also reference it.

> +        description:
> +          each entry consists of 3 integers and represents the mux and config
> +          setting for one pin. The first 2 integers <mux_reg func_num> are
> +          specified using a CIX_PAD_* macro.The last integer CONFIG is the pad
> +          setting value like pull-up on this pin.
> +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +        items:
> +          items:
> +            - description: |
> +                "mux_reg" indicates the offset of register.
> +            - description: |
> +                "func_num" indicates the mux value to be applied.
> +            - description: |
> +                "pad_setting" indicates the pad configuration value to be
> +                applied.
> +
> +    required:
> +      - cix,pins
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: pinctrl.yaml#
> +
> +additionalProperties: false
> +
> +examples:
> +  # Pinmux controller node
> +  - |
> +    #include <dt-bindings/pinctrl/pads-sky1.h>
> +    iomuxc: pinctrl@4170000 {
> +        compatible = "cix,sky1-iomuxc";
> +        reg = <0x4170000 0x1000>;
> +
> +        pinctrl_hog: hog-pins {

Don't use hog.

> +            cix,pins =
> +                <CIX_PAD_GPIO144_FUNC_GPIO144 (PULL_DOWN|DS_LEVEL4)>;
> +        };
> +    };
> diff --git a/include/dt-bindings/pinctrl/pads-sky1.h b/include/dt-bindings/pinctrl/pads-sky1.h
> new file mode 100644
> index 000000000000..44550e4105b3
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/pads-sky1.h

Bindings follow compatible naming. See writing bindings.

> @@ -0,0 +1,592 @@
> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> +/*
> + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
> + */
> +
> +#ifndef __SKY1_PADS_H
> +#define __SKY1_PADS_H
> +
> +#define CIX_PAD_GPIO001_OFFSET			0x0
> +#define CIX_PAD_GPIO002_OFFSET			0x4

Not bindings. Drop all this.



Best regards,
Krzysztof

回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Gary Yang]

Hi Krzysztof,

Thanks for your comments

> 
> On 27/08/2025 04:42, Gary Yang wrote:
> > Add dt-bindings docs
> 
> For what? Describe the hardware here in one, two sentences.
> 

OK, we will add some description for it next version

> >
> > Signed-off-by: Gary Yang <gary.yang@cixtech.com>
> > ---
> >  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
> >  include/dt-bindings/pinctrl/pads-sky1.h       | 592
> ++++++++++++++++++
> >  2 files changed, 669 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
> >
> > diff --git
> > a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> > b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> > new file mode 100644
> > index 000000000000..10a4a292188e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> > @@ -0,0 +1,77 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Cix Sky1 Pin Controller
> > +
> > +maintainers:
> > +  - Gary Yang <gary.yang@cixtech.com>
> > +
> > +description:
> > +  Please refer to pinctrl-bindings.txt in this directory for common
> > +  binding part and usage.
> 
> Drop description, not desired really.
> 

Ok, this yaml file comes from other yaml file. If not needed, we remove it next version

> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - cix,sky1-iomuxc
> > +      - cix,sky1-iomuxc-s5
> 
> Whats the difference between? You have entire description field to explain this
> but instead you said something obvious there.
> 
Cix sky1 has three power states. S0 means work state. S3 means STR state. S5 means SD state.

The pin-controller on sky1 has two power states. They are S0 and S5.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +# Client device subnode's properties
> > +patternProperties:
> > +  '-pins$':
> > +    type: object
> > +    description:
> > +      Pinctrl node's client devices use subnodes for desired pin
> configuration.
> > +      Client device subnodes use below standard properties.
> > +
> > +    properties:
> > +      cix,pins:
> 
> No, use generic properties from pinmux schema.
> 
> You should also reference it.

Did you suggest us to refer to Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?

Make us support drive-strength, bias-pull-down properties?

> 
> > +        description:
> > +          each entry consists of 3 integers and represents the mux and
> config
> > +          setting for one pin. The first 2 integers <mux_reg func_num> are
> > +          specified using a CIX_PAD_* macro.The last integer CONFIG is
> the pad
> > +          setting value like pull-up on this pin.
> > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +        items:
> > +          items:
> > +            - description: |
> > +                "mux_reg" indicates the offset of register.
> > +            - description: |
> > +                "func_num" indicates the mux value to be applied.
> > +            - description: |
> > +                "pad_setting" indicates the pad configuration value to be
> > +                applied.
> > +
> > +    required:
> > +      - cix,pins
> > +
> > +    additionalProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - $ref: pinctrl.yaml#
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  # Pinmux controller node
> > +  - |
> > +    #include <dt-bindings/pinctrl/pads-sky1.h>
> > +    iomuxc: pinctrl@4170000 {
> > +        compatible = "cix,sky1-iomuxc";
> > +        reg = <0x4170000 0x1000>;
> > +
> > +        pinctrl_hog: hog-pins {
> 
> Don't use hog.

OK, we use other group instead of it

> 
> > +            cix,pins =
> > +                <CIX_PAD_GPIO144_FUNC_GPIO144
> (PULL_DOWN|DS_LEVEL4)>;
> > +        };
> > +    };
> > diff --git a/include/dt-bindings/pinctrl/pads-sky1.h
> > b/include/dt-bindings/pinctrl/pads-sky1.h
> > new file mode 100644
> > index 000000000000..44550e4105b3
> > --- /dev/null
> > +++ b/include/dt-bindings/pinctrl/pads-sky1.h
> 
> Bindings follow compatible naming. See writing bindings.
> 

Did you suggest rename it to pinctrl-sky1.h ?

> > @@ -0,0 +1,592 @@
> > +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> > +/*
> > + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
> > + */
> > +
> > +#ifndef __SKY1_PADS_H
> > +#define __SKY1_PADS_H
> > +
> > +#define CIX_PAD_GPIO001_OFFSET                       0x0
> > +#define CIX_PAD_GPIO002_OFFSET                       0x4
> 
> Not bindings. Drop all this.
> 

Do you mean those macros not used need to delete?

> 
> 
> Best regards,
> Krzysztof

Best wishes
Gary

Re: 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Krzysztof Kozlowski]

On 28/08/2025 07:37, Gary Yang wrote:
> Hi Krzysztof,
> 
> Thanks for your comments
> 
>>
>> On 27/08/2025 04:42, Gary Yang wrote:
>>> Add dt-bindings docs
>>
>> For what? Describe the hardware here in one, two sentences.
>>
> 
> OK, we will add some description for it next version
> 
>>>
>>> Signed-off-by: Gary Yang <gary.yang@cixtech.com>
>>> ---
>>>  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
>>>  include/dt-bindings/pinctrl/pads-sky1.h       | 592
>> ++++++++++++++++++
>>>  2 files changed, 669 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>>>  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>>> b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>>> new file mode 100644
>>> index 000000000000..10a4a292188e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>>> @@ -0,0 +1,77 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Cix Sky1 Pin Controller
>>> +
>>> +maintainers:
>>> +  - Gary Yang <gary.yang@cixtech.com>
>>> +
>>> +description:
>>> +  Please refer to pinctrl-bindings.txt in this directory for common
>>> +  binding part and usage.
>>
>> Drop description, not desired really.
>>
> 
> Ok, this yaml file comes from other yaml file. If not needed, we remove it next version
> 
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - cix,sky1-iomuxc
>>> +      - cix,sky1-iomuxc-s5
>>
>> Whats the difference between? You have entire description field to explain this
>> but instead you said something obvious there.
>>
> Cix sky1 has three power states. S0 means work state. S3 means STR state. S5 means SD state.
> 
> The pin-controller on sky1 has two power states. They are S0 and S5.


State != device. Please create bindings for devices, not states.

> 
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +# Client device subnode's properties
>>> +patternProperties:
>>> +  '-pins$':
>>> +    type: object
>>> +    description:
>>> +      Pinctrl node's client devices use subnodes for desired pin
>> configuration.
>>> +      Client device subnodes use below standard properties.
>>> +
>>> +    properties:
>>> +      cix,pins:
>>
>> No, use generic properties from pinmux schema.
>>
>> You should also reference it.
> 
> Did you suggest us to refer to Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?
> 
> Make us support drive-strength, bias-pull-down properties?

and pinmux. There is a standard pins property.


...

>>> diff --git a/include/dt-bindings/pinctrl/pads-sky1.h
>>> b/include/dt-bindings/pinctrl/pads-sky1.h
>>> new file mode 100644
>>> index 000000000000..44550e4105b3
>>> --- /dev/null
>>> +++ b/include/dt-bindings/pinctrl/pads-sky1.h
>>
>> Bindings follow compatible naming. See writing bindings.
>>
> 
> Did you suggest rename it to pinctrl-sky1.h ?

No. I suggest to be named EXACTLY like compatible.

> 
>>> @@ -0,0 +1,592 @@
>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>> +/*
>>> + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
>>> + */
>>> +
>>> +#ifndef __SKY1_PADS_H
>>> +#define __SKY1_PADS_H
>>> +
>>> +#define CIX_PAD_GPIO001_OFFSET                       0x0
>>> +#define CIX_PAD_GPIO002_OFFSET                       0x4
>>
>> Not bindings. Drop all this.
>>
> 
> Do you mean those macros not used need to delete?

Really, what is unlcear in "drop all this"? Drop means to remove.

You ask for confirmation for some really obvious comments.

BTW, if you disagree provide arguments (in terms of bindings) why these
are bindings.

回复: 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Gary Yang]

Hi Krzysztof,

> On 28/08/2025 07:37, Gary Yang wrote:
> > Hi Krzysztof,
> >
> > Thanks for your comments
> >
> >>
> >> On 27/08/2025 04:42, Gary Yang wrote:
> >>> Add dt-bindings docs
> >>
> >> For what? Describe the hardware here in one, two sentences.
> >>
> >
> > OK, we will add some description for it next version
> >
> >>>
> >>> Signed-off-by: Gary Yang <gary.yang@cixtech.com>
> >>> ---
> >>>  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
> >>>  include/dt-bindings/pinctrl/pads-sky1.h       | 592
> >> ++++++++++++++++++
> >>>  2 files changed, 669 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>>  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
> >>>
> >>> diff --git
> >>> a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>> b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> >>> new file mode 100644
> >>> index 000000000000..10a4a292188e
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yam
> >>> +++ l
> >>> @@ -0,0 +1,77 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) %YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/pinctrl/cix,sky1-pinctrl.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: Cix Sky1 Pin Controller
> >>> +
> >>> +maintainers:
> >>> +  - Gary Yang <gary.yang@cixtech.com>
> >>> +
> >>> +description:
> >>> +  Please refer to pinctrl-bindings.txt in this directory for common
> >>> +  binding part and usage.
> >>
> >> Drop description, not desired really.
> >>
> >
> > Ok, this yaml file comes from other yaml file. If not needed, we
> > remove it next version
> >
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - cix,sky1-iomuxc
> >>> +      - cix,sky1-iomuxc-s5
> >>
> >> Whats the difference between? You have entire description field to
> >> explain this but instead you said something obvious there.
> >>
> > Cix sky1 has three power states. S0 means work state. S3 means STR state.
> S5 means SD state.
> >
> > The pin-controller on sky1 has two power states. They are S0 and S5.
> 
> 
> State != device. Please create bindings for devices, not states.
> 

Sorry, maybe I didn't explain it correctly before, and then make you misunderstand

There are two pin-controller on sky1. One is used under s0 state, other is used under s5 state.

They are two devices

> >
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +# Client device subnode's properties
> >>> +patternProperties:
> >>> +  '-pins$':
> >>> +    type: object
> >>> +    description:
> >>> +      Pinctrl node's client devices use subnodes for desired pin
> >> configuration.
> >>> +      Client device subnodes use below standard properties.
> >>> +
> >>> +    properties:
> >>> +      cix,pins:
> >>
> >> No, use generic properties from pinmux schema.
> >>
> >> You should also reference it.
> >
> > Did you suggest us to refer to
> Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?
> >
> > Make us support drive-strength, bias-pull-down properties?
> 
> and pinmux. There is a standard pins property.
> 

Ok, I see, try our best to support standard

> 
> ...
> 
> >>> diff --git a/include/dt-bindings/pinctrl/pads-sky1.h
> >>> b/include/dt-bindings/pinctrl/pads-sky1.h
> >>> new file mode 100644
> >>> index 000000000000..44550e4105b3
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/pinctrl/pads-sky1.h
> >>
> >> Bindings follow compatible naming. See writing bindings.
> >>
> >
> > Did you suggest rename it to pinctrl-sky1.h ?
> 
> No. I suggest to be named EXACTLY like compatible.
> 

OK, I see

> >
> >>> @@ -0,0 +1,592 @@
> >>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
> >>> +/*
> >>> + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
> >>> + */
> >>> +
> >>> +#ifndef __SKY1_PADS_H
> >>> +#define __SKY1_PADS_H
> >>> +
> >>> +#define CIX_PAD_GPIO001_OFFSET                       0x0
> >>> +#define CIX_PAD_GPIO002_OFFSET                       0x4
> >>
> >> Not bindings. Drop all this.
> >>
> >
> > Do you mean those macros not used need to delete?
> 
> Really, what is unlcear in "drop all this"? Drop means to remove.
> 
> You ask for confirmation for some really obvious comments.
> 
> BTW, if you disagree provide arguments (in terms of bindings) why these are
> bindings.

Sorry, make you misunderstand again. I know drop means remove. I want to know whether "all this" mean all macros in the file or not.
These macros will be used by client in the dtsi and dts file. If remove them, maybe add them again when they are used in the further.
It looks like the file which locate include/dt-bindings/pinctrl/pads-imx8qm.h

Best wishes
Gary

Re: 回复: 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Krzysztof Kozlowski]

On 28/08/2025 10:58, Gary Yang wrote:
>>>
>>>>> @@ -0,0 +1,592 @@
>>>>> +/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
>>>>> +/*
>>>>> + * Copyright 2024-2025 Cix Technology Group Co., Ltd.
>>>>> + */
>>>>> +
>>>>> +#ifndef __SKY1_PADS_H
>>>>> +#define __SKY1_PADS_H
>>>>> +
>>>>> +#define CIX_PAD_GPIO001_OFFSET                       0x0
>>>>> +#define CIX_PAD_GPIO002_OFFSET                       0x4
>>>>
>>>> Not bindings. Drop all this.
>>>>
>>>
>>> Do you mean those macros not used need to delete?
>>
>> Really, what is unlcear in "drop all this"? Drop means to remove.
>>
>> You ask for confirmation for some really obvious comments.
>>
>> BTW, if you disagree provide arguments (in terms of bindings) why these are
>> bindings.
> 
> Sorry, make you misunderstand again. I know drop means remove. I want to know whether "all this" mean all macros in the file or not.

All of these defines are not suitable for bindings, because they do not
represent any ABI for Linux.

> These macros will be used by client in the dtsi and dts file. If remove them, maybe add them again when they are used in the further.
> It looks like the file which locate include/dt-bindings/pinctrl/pads-imx8qm.h

Please take the newest contributions as example, not something 10 year old.


Best regards,
Krzysztof

Re: 回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Linus Walleij]

Hi Gary,

thanks for your patch!

On Thu, Aug 28, 2025 at 10:58 AM Gary Yang <gary.yang@cixtech.com> wrote:
> > On 28/08/2025 07:37, Gary Yang wrote:

> > >> Whats the difference between? You have entire description field to
> > >> explain this but instead you said something obvious there.
> > >>
> > > Cix sky1 has three power states. S0 means work state. S3 means STR state.
> > S5 means SD state.
> > >
> > > The pin-controller on sky1 has two power states. They are S0 and S5.
> >
> >
> > State != device. Please create bindings for devices, not states.
> >
>
> Sorry, maybe I didn't explain it correctly before, and then make you misunderstand
>
> There are two pin-controller on sky1. One is used under s0 state, other is used under s5 state.
>
> They are two devices

Just explain this in the description: and everyone will understand what
is going on. Since "S0" and "S5" can be easy to confuse for "states"
it is extra helpful with some extended descriptions.

> > >>> +    properties:
> > >>> +      cix,pins:
> > >>
> > >> No, use generic properties from pinmux schema.
> > >>
> > >> You should also reference it.
> > >
> > > Did you suggest us to refer to
> > Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?
> > >
> > > Make us support drive-strength, bias-pull-down properties?
> >
> > and pinmux. There is a standard pins property.
>
> Ok, I see, try our best to support standard

Unfortunately many pin controllers have forged ahead
with custom foo,pins = <....>; settings where they set up
mux and electrical config by OR:in together different bits,
and then they just poke this into some registers.

This isn't very helpful for users.

I initially wanted all functions and groups to be strings
and then to associate groups with functions using
strings in the device tree.

But I have realized (though much pain) that many developers
don't like this. They want a magic number to write to
a register to configure a pin, because their hardware
has one (or several) register for each pin.

So nowadays the most common is to use a compromise.

A magic number in the pinmux property to set up the muxing.

For example:

arch/arm/boot/dts/mediatek/mt7623.dtsi:
pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
               <MT7623_PIN_76_SCL0_FUNC_SCL0>;

Then the electric properties like bias-pull-down; to set
these on the state:

        i2c0_pins_a: i2c0-default {
                pins-i2c0 {
                        pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
                                 <MT7623_PIN_76_SCL0_FUNC_SCL0>;
                        bias-disable;
                };
        };

This is a good compromis becaus it looks similar on all
SoCs and you see immediately what is going on: we enable
SDA0 And SCL0 and disable bias, so there must be external
pull-up resistors on this bus since I2C is open drain. Very
easy for an electronics engineer to grasp, they don't need
to be computer engineers or device tree experts.

Yours,
Linus Walleij

回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Gary Yang]

Hi Linus,

Thanks for your comments!

Sorry for delay reply

> 
> On Thu, Aug 28, 2025 at 10:58 AM Gary Yang <gary.yang@cixtech.com> wrote:
> > > On 28/08/2025 07:37, Gary Yang wrote:
> 
> > > >> Whats the difference between? You have entire description field
> > > >> to explain this but instead you said something obvious there.
> > > >>
> > > > Cix sky1 has three power states. S0 means work state. S3 means STR
> state.
> > > S5 means SD state.
> > > >
> > > > The pin-controller on sky1 has two power states. They are S0 and S5.
> > >
> > >
> > > State != device. Please create bindings for devices, not states.
> > >
> >
> > Sorry, maybe I didn't explain it correctly before, and then make you
> > misunderstand
> >
> > There are two pin-controller on sky1. One is used under s0 state, other is
> used under s5 state.
> >
> > They are two devices
> 
> Just explain this in the description: and everyone will understand what is going
> on. Since "S0" and "S5" can be easy to confuse for "states"
> it is extra helpful with some extended descriptions.
> 

Yes, I have realized this problem, Thanks for your remind.

> > > >>> +    properties:
> > > >>> +      cix,pins:
> > > >>
> > > >> No, use generic properties from pinmux schema.
> > > >>
> > > >> You should also reference it.
> > > >
> > > > Did you suggest us to refer to
> > > Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml?
> > > >
> > > > Make us support drive-strength, bias-pull-down properties?
> > >
> > > and pinmux. There is a standard pins property.
> >
> > Ok, I see, try our best to support standard
> 
> Unfortunately many pin controllers have forged ahead with custom foo,pins =
> <....>; settings where they set up mux and electrical config by OR:in together
> different bits, and then they just poke this into some registers.
> 
> This isn't very helpful for users.
> 
> I initially wanted all functions and groups to be strings and then to associate
> groups with functions using strings in the device tree.
> 
> But I have realized (though much pain) that many developers don't like this.
> They want a magic number to write to a register to configure a pin, because
> their hardware has one (or several) register for each pin.
> 
> So nowadays the most common is to use a compromise.
> 
> A magic number in the pinmux property to set up the muxing.
> 
> For example:
> 
> arch/arm/boot/dts/mediatek/mt7623.dtsi:
> pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
>                <MT7623_PIN_76_SCL0_FUNC_SCL0>;
> 
> Then the electric properties like bias-pull-down; to set these on the state:
> 
>         i2c0_pins_a: i2c0-default {
>                 pins-i2c0 {
>                         pinmux = <MT7623_PIN_75_SDA0_FUNC_SDA0>,
>                                  <MT7623_PIN_76_SCL0_FUNC_SCL0>;
>                         bias-disable;
>                 };
>         };
> 
> This is a good compromis becaus it looks similar on all SoCs and you see
> immediately what is going on: we enable
> SDA0 And SCL0 and disable bias, so there must be external pull-up resistors on
> this bus since I2C is open drain. Very easy for an electronics engineer to grasp,
> they don't need to be computer engineers or device tree experts.
> 

I appreciate your comments. You are very kind and nice.

I understand your thinking and try to support the standard referred to above.

I only need to spend some time to research this scheme and debug it on Radax O6 board.

If miss any information, please remind me. Thanks for your kind again.

> Yours,
> Linus Walleij

Best wishes
Gary

Re: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Linus Walleij]

On Sat, Aug 30, 2025 at 3:20 PM Gary Yang <gary.yang@cixtech.com> wrote:

> I understand your thinking and try to support the standard referred to above.
>
> I only need to spend some time to research this scheme and debug it on Radax O6 board.

Thanks Gary, I have this board too so I hope to be able to test it
directly.

I haven't figured out how to boot it using device tree, everything
I have going is using ACPI right now, but once I know how to
boot it with device tree, I will be happy to test!

Yours,
Linus Walleij

回复: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Gary Yang]

Hi Linus,
> 
> On Sat, Aug 30, 2025 at 3:20 PM Gary Yang <gary.yang@cixtech.com> wrote:
> 
> > I understand your thinking and try to support the standard referred to above.
> >
> > I only need to spend some time to research this scheme and debug it on
> Radax O6 board.
> 
> Thanks Gary, I have this board too so I hope to be able to test it directly.
> 
> I haven't figured out how to boot it using device tree, everything I have going is
> using ACPI right now, but once I know how to boot it with device tree, I will be
> happy to test!
> 

I'm glad to see your interesting on Radax board.

I have understanded the new scheme. I start to debug it on Radax board yesterday.

Of course, If you want to test, please follow these steps:

First select "Cix Sky1 on Orion O6 (Device Tree) " option on grub UI

Second press "Ctrl+x" to continue boot, and use "root" user to login

Third plug ethernet and execute shell commands:
1) mount /dev/nvme0n1p1 /mnt
2) scp arch/arm64/boot/Image /mnt/Image-pinctrl
3) scp arch/arm64/boot/dts/cix/sky1-orion-o6.dtb /mnt/sky1-orion-o6-pinctrl.dtb
4) umount /mnt
5) reboot

Fourth select "Cix Sky1 on Orion O6 (Device Tree) " option on grub UI and edit it, 

and then replace your dtb and image with sky1-orion-o6-pinctrl.dtb and Image-pinctrl

Please note: add clk_ignore_unused=1 config in grub

Finally press "Ctrl+x" to continue boot, then you can see what expected.

If you have any questions, please ping me any time. Thanks

> Yours,
> Linus Walleij

Best wishes
Gary

Re: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Krzysztof Kozlowski]

On Wed, Aug 27, 2025 at 10:42:21AM +0800, Gary Yang wrote:
> Add dt-bindings docs
> 
> Signed-off-by: Gary Yang <gary.yang@cixtech.com>
> ---
>  .../bindings/pinctrl/cix,sky1-pinctrl.yaml    |  77 +++
>  include/dt-bindings/pinctrl/pads-sky1.h       | 592 ++++++++++++++++++
>  2 files changed, 669 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
>  create mode 100644 include/dt-bindings/pinctrl/pads-sky1.h
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml
> new file mode 100644
> index 000000000000..10a4a292188e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/cix,sky1-pinctrl.yaml

Also: Filename based on compatible

Bot reports here some warnings, so please confirm: did you test your
bindings and there are no errors reported by the toolset?

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: pinctrl: Add cix,sky1-pinctrl

[Linus Walleij]

On Wed, Aug 27, 2025 at 4:42 AM Gary Yang <gary.yang@cixtech.com> wrote:

> Add dt-bindings docs
>
> Signed-off-by: Gary Yang <gary.yang@cixtech.com>

I saw Krzysztof reacted to this:

> +++ b/include/dt-bindings/pinctrl/pads-sky1.h
(...)
> +#define CIX_PAD_GPIO001_OFFSET                 0x0
> +#define CIX_PAD_GPIO002_OFFSET                 0x4
(...)
> +#define CIX_PAD_GPIO001_FUNC_GPIO001                              0x0
> +#define CIX_PAD_GPIO002_FUNC_GPIO002                              0x0
(...)
> +#define PULL_UP                (1 << 6)
> +#define PULL_DOWN      (1 << 5)
> +#define ST             (1 << 4)
> +#define DS_LEVEL1      0x1
> +#define DS_LEVEL2      0x2
(...)

As stated, this isn't part of bindings so it should not be
include/dt-bindings/pinctrl/*.

If you are using the pinmux = <...>: property, what you can
do however is to put the same defines into
arch/arm64/boot/dts/cix/sky1-pinmux-props.dtsi
and use it the same way.

Then it is not bindings, just some DT data.

Sometimes this distinction isn't clear, and the kernel contain
many offenders to this rule.

Yours,
Linus Walleij

[PATCH 3/3] arm64: dts: cix: Add pinctrl nodes for sky1

[Gary Yang]

Add the pin-controller nodes for Sky1 platform.

Signed-off-by: Gary Yang <gary.yang@cixtech.com>
---
 arch/arm64/boot/dts/cix/sky1-orion-o6.dts | 28 +++++++++++++++++++++++
 arch/arm64/boot/dts/cix/sky1.dtsi         | 10 ++++++++
 2 files changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
index d74964d53c3b..8fab0c3b36b3 100644
--- a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
+++ b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts
@@ -7,6 +7,9 @@
 /dts-v1/;
 
 #include "sky1.dtsi"
+
+#include <dt-bindings/pinctrl/pads-sky1.h>
+
 / {
 	model = "Radxa Orion O6";
 	compatible = "radxa,orion-o6", "cix,sky1";
@@ -34,6 +37,31 @@ linux,cma {
 
 };
 
+&iomuxc {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog>;
+
+	pinctrl_hog: hog-pins {
+		cix,pins = <
+			CIX_PAD_GPIO144_OFFSET CIX_PAD_GPIO144_FUNC_GPIO144 (PULL_DOWN|DS_LEVEL4)
+			CIX_PAD_GPIO145_OFFSET CIX_PAD_GPIO145_FUNC_GPIO145 (PULL_DOWN|DS_LEVEL4)
+			CIX_PAD_GPIO146_OFFSET CIX_PAD_GPIO146_FUNC_GPIO146 (PULL_DOWN|DS_LEVEL4)
+			CIX_PAD_GPIO147_OFFSET CIX_PAD_GPIO147_FUNC_GPIO147 (PULL_DOWN|DS_LEVEL4)
+		>;
+	};
+};
+
+&iomuxc_s5 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_hog_s5>;
+
+	pinctrl_hog_s5: hog-s5-pins {
+		cix,pins = <
+			CIX_PAD_GPIO014_OFFSET CIX_PAD_GPIO014_FUNC_GPIO014  (PULL_UP|DS_LEVEL4)
+		>;
+	};
+};
+
 &uart2 {
 	status = "okay";
 };
diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
index 7dfe7677e649..bfe34e5311e8 100644
--- a/arch/arm64/boot/dts/cix/sky1.dtsi
+++ b/arch/arm64/boot/dts/cix/sky1.dtsi
@@ -316,6 +316,16 @@ ppi_partition1: interrupt-partition-1 {
 				};
 			};
 		};
+
+		iomuxc: pinctrl@4170000 {
+			compatible = "cix,sky1-iomuxc";
+			reg = <0x0 0x04170000 0x0 0x1000>;
+		};
+
+		iomuxc_s5: pinctrl@16007000 {
+			compatible = "cix,sky1-iomuxc-s5";
+			reg = <0x0 0x16007000 0x0 0x1000>;
+		};
 	};
 
 	timer {
-- 
2.49.0

Re: [PATCH 3/3] arm64: dts: cix: Add pinctrl nodes for sky1

[Krzysztof Kozlowski]

On 27/08/2025 04:42, Gary Yang wrote:
>  			};
>  		};
> +
> +		iomuxc: pinctrl@4170000 {
> +			compatible = "cix,sky1-iomuxc";
> +			reg = <0x0 0x04170000 0x0 0x1000>;
> +		};
> +
> +		iomuxc_s5: pinctrl@16007000 {

Are you sure you follow DTS coding style for ordering of nodes? Looks
like you just keep adding things to the end...

> +			compatible = "cix,sky1-iomuxc-s5";
> +			reg = <0x0 0x16007000 0x0 0x1000>;
> +		};
>  	};
>  
>  	timer {


Best regards,
Krzysztof

回复: [PATCH 3/3] arm64: dts: cix: Add pinctrl nodes for sky1

[Gary Yang]

Hi Krzysztof,

> On 27/08/2025 04:42, Gary Yang wrote:
> >                       };
> >               };
> > +
> > +             iomuxc: pinctrl@4170000 {
> > +                     compatible = "cix,sky1-iomuxc";
> > +                     reg = <0x0 0x04170000 0x0 0x1000>;
> > +             };
> > +
> > +             iomuxc_s5: pinctrl@16007000 {
> 
> Are you sure you follow DTS coding style for ordering of nodes? Looks like you
> just keep adding things to the end...

Sorry, I see, we will correct it next version. Thanks for your comments

> 
> > +                     compatible = "cix,sky1-iomuxc-s5";
> > +                     reg = <0x0 0x16007000 0x0 0x1000>;
> > +             };
> >       };
> >
> >       timer {
> 
> 
> Best regards,
> Krzysztof

Best wishes
Gary