[PATCH v5 00/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[PATCH v5 00/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

This patch series adds support for pinmuxing, gpios, and leds on the Kendyte
K210.

This patch series was previously part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

This patch series depends on
https://patchwork.ozlabs.org/project/uboot/list/?series=178480

Changes in v5:
- Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
- Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
  by commit 2548493ab4.
- Rebase onto u-boot/master

Changes in v4:
- Add sandbox dt-binding headers to MAINTAINERS
- Add test for led behavior
- Add test/dm/pinmux.c to patch
- Move sandbox_* variables into a priv structure. This resets them to the
  default state every time we re-probe.
- Reformat documentation in dm/pinctrl.h

Changes in v3:
- Add dt-bindings/pinctrl/sandbox-pinmux.h to patch

Changes in v2:
- Add test for pinmuxing
- Don't clear existing pinctrl settings on probe
- Re-order GPIOs to match the defaults more closely
- Rebase onto v13 of "riscv: Add Sipeed Maix support"
- Rewrite FPIOA driver to use pinmux property
- Support muxing the output enable signal for each function in the FPIOA
- Support output and input inversion in the pinmux driver
- Support pinmux property in pinctrl-generic

Sean Anderson (11):
  pinctrl: Add pinmux property support to pinctrl-generic
  pinctrl: Reformat documentation in dm/pinctrl.h
  test: pinmux: Add test for pin muxing
  pinctrl: Add support for Kendryte K210 FPIOA
  gpio: dw: Fix warnings about casting int to pointer
  gpio: dw: Add a trailing underscore to generated name
  gpio: dw: Return output value when direction is out
  led: gpio: Default to using node name if label is absent
  test: dm: Test for default led naming
  riscv: Add pinmux and gpio bindings for Kendryte K210
  riscv: Add FPIOA and GPIO support for Kendryte K210

 MAINTAINERS                                   |   3 +
 arch/riscv/dts/k210-maix-bit.dts              | 104 +++
 arch/riscv/dts/k210.dtsi                      |  12 +
 arch/sandbox/dts/test.dts                     |  47 +-
 board/sipeed/maix/Kconfig                     |   9 +
 configs/sipeed_maix_bitm_defconfig            |   2 +
 doc/board/sipeed/maix.rst                     |  64 +-
 .../pinctrl/kendryte,k210-fpioa.txt           | 102 +++
 .../pinctrl/pinctrl-bindings.txt              |  65 +-
 drivers/gpio/dwapb_gpio.c                     |  33 +-
 drivers/led/led_gpio.c                        |   7 +-
 drivers/pinctrl/Kconfig                       |   1 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/kendryte/Kconfig              |   7 +
 drivers/pinctrl/kendryte/Makefile             |   1 +
 drivers/pinctrl/kendryte/pinctrl.c            | 678 ++++++++++++++++++
 drivers/pinctrl/kendryte/pinctrl.h            |  70 ++
 drivers/pinctrl/pinctrl-generic.c             | 125 +++-
 drivers/pinctrl/pinctrl-sandbox.c             | 186 +++--
 include/dm/pinctrl.h                          | 360 +++++++---
 include/dt-bindings/pinctrl/k210-pinctrl.h    | 277 +++++++
 include/dt-bindings/pinctrl/sandbox-pinmux.h  |  19 +
 test/dm/Makefile                              |   3 +
 test/dm/led.c                                 |   3 +-
 test/dm/pinmux.c                              |  57 ++
 test/py/tests/test_pinmux.py                  |  36 +-
 26 files changed, 2028 insertions(+), 244 deletions(-)
 create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
 create mode 100644 drivers/pinctrl/kendryte/Kconfig
 create mode 100644 drivers/pinctrl/kendryte/Makefile
 create mode 100644 drivers/pinctrl/kendryte/pinctrl.c
 create mode 100644 drivers/pinctrl/kendryte/pinctrl.h
 create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h
 create mode 100644 include/dt-bindings/pinctrl/sandbox-pinmux.h
 create mode 100644 test/dm/pinmux.c

-- 
2.28.0

[PATCH v5 01/11] pinctrl: Add pinmux property support to pinctrl-generic

[Sean Anderson]

The pinmux property allows for smaller and more compact device trees,
especially when there are many pins which need to be assigned individually.
Instead of specifying an array of strings to be parsed as pins and a
function property, the pinmux property contains an array of integers
representing pinmux groups. A pinmux group consists of the pin identifier
and mux settings represented as a single integer or an array of integers.
Each individual pin controller driver specifies the exact format of a
pinmux group. As specified in the Linux documentation, a pinmux group may
be multiple integers long. However, no existing drivers use multi-integer
pinmux groups, so I have chosen to omit this feature. This makes the
implementation easier, since there is no need to allocate a buffer to do
endian conversions.

Support for the pinmux property is done differently than in Linux.  As far
as I can tell, inversion of control is used when implementing support for
the pins and groups properties to avoid allocating. This results in some
duplication of effort; every property in a config node is parsed once for
each pin in that node. This is not such an overhead with pins and groups
properties, since having multiple pins in one config node does not occur
especially often. However, the semantics of the pinmux property make such a
configuration much more appealing. A future patch could parse all config
properties at once and store them in an array. This would make it easier to
create drivers which do not function solely as callbacks from
pinctrl-generic.

This commit increases the size of the sandbox build by approximately 48
bytes.  However, it also decreases the size of the K210 device tree by 2
KiB from the previous version of this series.

The documentation has been updated from the last Linux commit before it was
split off into yaml files.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- New

 .../pinctrl/pinctrl-bindings.txt              |  65 ++++++++-
 drivers/pinctrl/pinctrl-generic.c             | 125 ++++++++++++++----
 include/dm/pinctrl.h                          |  21 +--
 3 files changed, 168 insertions(+), 43 deletions(-)

diff --git a/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt b/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt
index b73c96d24f..603796f169 100644
--- a/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt
+++ b/doc/device-tree-bindings/pinctrl/pinctrl-bindings.txt
@@ -119,7 +119,8 @@ For example:
 
 The contents of each of those pin configuration child nodes is defined
 entirely by the binding for the individual pin controller device. There
-exists no common standard for this content.
+exists no common standard for this content. The pinctrl framework only
+provides generic helper bindings that the pin controller driver can use.
 
 The pin configuration nodes need not be direct children of the pin controller
 device; they may be grandchildren, for example. Whether this is legal, and
@@ -156,6 +157,29 @@ state_2_node_a {
 	pins = "mfio29", "mfio30";
 };
 
+For hardware where pin multiplexing configurations have to be specified for
+each single pin the number of required sub-nodes containing "pin" and
+"function" properties can quickly escalate and become hard to write and
+maintain.
+
+For cases like this, the pin controller driver may use the pinmux helper
+property, where the pin identifier is provided with mux configuration settings
+in a pinmux group. A pinmux group consists of the pin identifier and mux
+settings represented as a single integer or an array of integers.
+
+The pinmux property accepts an array of pinmux groups, each of them describing
+a single pin multiplexing configuration.
+
+pincontroller {
+	state_0_node_a {
+		pinmux = <PINMUX_GROUP>, <PINMUX_GROUP>, ...;
+	};
+};
+
+Each individual pin controller driver bindings documentation shall specify
+how pin IDs and pin multiplexing configuration are defined and assembled
+together in a pinmux group.
+
 == Generic pin configuration node content ==
 
 Many data items that are represented in a pin configuration node are common
@@ -168,12 +192,15 @@ structure of the DT nodes that contain these properties.
 Supported generic properties are:
 
 pins			- the list of pins that properties in the node
-			  apply to (either this or "group" has to be
+			  apply to (either this, "group" or "pinmux" has to be
 			  specified)
 group			- the group to apply the properties to, if the driver
 			  supports configuration of whole groups rather than
-			  individual pins (either this or "pins" has to be
-			  specified)
+			  individual pins (either this, "pins" or "pinmux" has
+			  to be specified)
+pinmux			- the list of numeric pin ids and their mux settings
+			  that properties in the node apply to (either this,
+			  "pins" or "groups" have to be specified)
 bias-disable		- disable any pin bias
 bias-high-impedance	- high impedance mode ("third-state", "floating")
 bias-bus-hold		- latch weakly
@@ -184,17 +211,30 @@ drive-push-pull		- drive actively high and low
 drive-open-drain	- drive with open drain
 drive-open-source	- drive with open source
 drive-strength		- sink or source at most X mA
-input-enable		- enable input on pin (no effect on output)
-input-disable		- disable input on pin (no effect on output)
+drive-strength-microamp	- sink or source at most X uA
+input-enable		- enable input on pin (no effect on output, such as
+			  enabling an input buffer)
+input-disable		- disable input on pin (no effect on output, such as
+			  disabling an input buffer)
 input-schmitt-enable	- enable schmitt-trigger mode
 input-schmitt-disable	- disable schmitt-trigger mode
 input-debounce		- debounce mode with debound time X
 power-source		- select between different power supplies
 low-power-enable	- enable low power mode
 low-power-disable	- disable low power mode
+output-disable		- disable output on a pin (such as disable an output
+			  buffer)
+output-enable		- enable output on a pin without actively driving it
+			  (such as enabling an output buffer)
 output-low		- set the pin to output mode with low level
 output-high		- set the pin to output mode with high level
+sleep-hardware-state	- indicate this is sleep related state which will be programmed
+			  into the registers for the sleep state.
 slew-rate		- set the slew rate
+skew-delay		- this affects the expected clock skew on input pins
+			  and the delay before latching a value to an output
+			  pin. Typically indicates how many double-inverters are
+			  used to delay the signal.
 
 For example:
 
@@ -216,6 +256,12 @@ state_2_node_a {
 		bias-pull-up;
 	};
 };
+state_3_node_a {
+	mux {
+		pinmux = <GPIOx_PINm_MUXn>, <GPIOx_PINj_MUXk)>;
+		input-enable;
+	};
+};
 
 Some of the generic properties take arguments. For those that do, the
 arguments are described below.
@@ -224,11 +270,18 @@ arguments are described below.
   binding for the hardware defines:
   - Whether the entries are integers or strings, and their meaning.
 
+- pinmux takes a list of pin IDs and mux settings as required argument. The
+  specific bindings for the hardware defines:
+  - How pin IDs and mux settings are defined and assembled together in a single
+    integer or an array of integers.
+
 - bias-pull-up, -down and -pin-default take as optional argument on hardware
   supporting it the pull strength in Ohm. bias-disable will disable the pull.
 
 - drive-strength takes as argument the target strength in mA.
 
+- drive-strength-microamp takes as argument the target strength in uA.
+
 - input-debounce takes the debounce time in usec as argument
   or 0 to disable debouncing
 
diff --git a/drivers/pinctrl/pinctrl-generic.c b/drivers/pinctrl/pinctrl-generic.c
index 313aeccb1e..3c8e24088c 100644
--- a/drivers/pinctrl/pinctrl-generic.c
+++ b/drivers/pinctrl/pinctrl-generic.c
@@ -227,6 +227,13 @@ static int pinconf_enable_setting(struct udevice *dev, bool is_group,
 }
 #endif
 
+enum pinmux_subnode_type {
+	PST_NONE = 0,
+	PST_PIN,
+	PST_GROUP,
+	PST_PINMUX,
+};
+
 /**
  * pinctrl_generic_set_state_one() - set state for a certain pin/group
  * Apply all pin multiplexing and pin configurations specified by @config
@@ -234,13 +241,15 @@ static int pinconf_enable_setting(struct udevice *dev, bool is_group,
  *
  * @dev: pin controller device
  * @config: pseudo device pointing to config node
- * @is_group: target of operation (true: pin group, false: pin)
- * @selector: pin selector or group selector, depending on @is_group
+ * @subnode_type: target of operation (pin, group, or pin specified by a pinmux
+ * group)
+ * @selector: pin selector or group selector, depending on @subnode_type
  * @return: 0 on success, or negative error code on failure
  */
 static int pinctrl_generic_set_state_one(struct udevice *dev,
 					 struct udevice *config,
-					 bool is_group, unsigned selector)
+					 enum pinmux_subnode_type subnode_type,
+					 unsigned selector)
 {
 	const char *propname;
 	const void *value;
@@ -248,17 +257,22 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 	int len, func_selector, param, ret;
 	u32 arg, default_val;
 
+	assert(subnode_type != PST_NONE);
+
 	dev_for_each_property(property, config) {
 		value = dev_read_prop_by_prop(&property, &propname, &len);
 		if (!value)
 			return -EINVAL;
 
-		if (!strcmp(propname, "function")) {
+		/* pinmux subnodes already have their muxing set */
+		if (subnode_type != PST_PINMUX &&
+		    !strcmp(propname, "function")) {
 			func_selector = pinmux_func_name_to_selector(dev,
 								     value);
 			if (func_selector < 0)
 				return func_selector;
-			ret = pinmux_enable_setting(dev, is_group,
+			ret = pinmux_enable_setting(dev,
+						    subnode_type == PST_GROUP,
 						    selector,
 						    func_selector);
 		} else {
@@ -272,7 +286,8 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 			else
 				arg = default_val;
 
-			ret = pinconf_enable_setting(dev, is_group,
+			ret = pinconf_enable_setting(dev,
+						     subnode_type == PST_GROUP,
 						     selector, param, arg);
 		}
 
@@ -283,6 +298,41 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 	return 0;
 }
 
+/**
+ * pinctrl_generic_get_subnode_type() - determine whether there is a valid
+ * pins, groups, or pinmux property in the config node
+ *
+ * @dev: pin controller device
+ * @config: pseudo device pointing to config node
+ * @count: number of specifiers contained within the property
+ * @return: the type of the subnode, or PST_NONE
+ */
+static enum pinmux_subnode_type pinctrl_generic_get_subnode_type(struct udevice *dev,
+								 struct udevice *config,
+								 int *count)
+{
+	const struct pinctrl_ops *ops = pinctrl_get_ops(dev);
+
+	*count = dev_read_string_count(config, "pins");
+	if (*count >= 0)
+		return PST_PIN;
+
+	*count = dev_read_string_count(config, "groups");
+	if (*count >= 0)
+		return PST_GROUP;
+
+	if (ops->pinmux_property_set) {
+		*count = dev_read_size(config, "pinmux");
+		if (*count >= 0 && !(*count % sizeof(u32))) {
+			*count /= sizeof(u32);
+			return PST_PINMUX;
+		}
+	}
+
+	*count = 0;
+	return PST_NONE;
+}
+
 /**
  * pinctrl_generic_set_state_subnode() - apply all settings in config node
  *
@@ -293,38 +343,55 @@ static int pinctrl_generic_set_state_one(struct udevice *dev,
 static int pinctrl_generic_set_state_subnode(struct udevice *dev,
 					     struct udevice *config)
 {
-	const char *subnode_target_type = "pins";
-	bool is_group = false;
+	enum pinmux_subnode_type subnode_type;
 	const char *name;
-	int strings_count, selector, i, ret;
+	int count, selector, i, ret, scratch;
+	const u32 *pinmux_groups = NULL; /* prevent use-uninitialized warning */
 
-	strings_count = dev_read_string_count(config, subnode_target_type);
-	if (strings_count < 0) {
-		subnode_target_type = "groups";
-		is_group = true;
-		strings_count = dev_read_string_count(config,
-						      subnode_target_type);
-		if (strings_count < 0) {
+	subnode_type = pinctrl_generic_get_subnode_type(dev, config, &count);
+
+	debug("%s(%s, %s): count=%d\n", __func__, dev->name, config->name,
+	      count);
+
+	if (subnode_type == PST_PINMUX) {
+		pinmux_groups = dev_read_prop(config, "pinmux", &scratch);
+		if (!pinmux_groups)
+			return -EINVAL;
+	}
+
+	for (i = 0; i < count; i++) {
+		switch (subnode_type) {
+		case PST_PIN:
+			ret = dev_read_string_index(config, "pins", i, &name);
+			if (ret)
+				return ret;
+			selector = pinctrl_pin_name_to_selector(dev, name);
+			break;
+		case PST_GROUP:
+			ret = dev_read_string_index(config, "groups", i, &name);
+			if (ret)
+				return ret;
+			selector = pinctrl_group_name_to_selector(dev, name);
+			break;
+		case PST_PINMUX: {
+			const struct pinctrl_ops *ops = pinctrl_get_ops(dev);
+			u32 pinmux_group = fdt32_to_cpu(pinmux_groups[i]);
+
+			/* Checked for in pinctrl_generic_get_subnode_type */
+			selector = ops->pinmux_property_set(dev, pinmux_group);
+			break;
+		}
+		case PST_NONE:
+		default:
 			/* skip this node; may contain config child nodes */
 			return 0;
 		}
-	}
 
-	for (i = 0; i < strings_count; i++) {
-		ret = dev_read_string_index(config, subnode_target_type, i,
-					    &name);
-		if (ret)
-			return ret;
-
-		if (is_group)
-			selector = pinctrl_group_name_to_selector(dev, name);
-		else
-			selector = pinctrl_pin_name_to_selector(dev, name);
 		if (selector < 0)
 			return selector;
 
-		ret = pinctrl_generic_set_state_one(dev, config,
-						    is_group, selector);
+		ret = pinctrl_generic_set_state_one(dev, config, subnode_type,
+						    selector);
 		if (ret)
 			return ret;
 	}
diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
index 692e5fc8cb..d50af1ce38 100644
--- a/include/dm/pinctrl.h
+++ b/include/dm/pinctrl.h
@@ -34,29 +34,33 @@ struct pinconf_param {
  * depending on your necessity.
  *
  * @get_pins_count: return number of selectable named pins available
- *	in this driver.  (necessary to parse "pins" property in DTS)
+ *	in this driver. (necessary to parse "pins" property in DTS)
  * @get_pin_name: return the pin name of the pin selector,
  *	called by the core to figure out which pin it shall do
- *	operations to.  (necessary to parse "pins" property in DTS)
+ *	operations to. (necessary to parse "pins" property in DTS)
  * @get_groups_count: return number of selectable named groups available
- *	in this driver.  (necessary to parse "groups" property in DTS)
+ *	in this driver. (necessary to parse "groups" property in DTS)
  * @get_group_name: return the group name of the group selector,
  *	called by the core to figure out which pin group it shall do
- *	operations to.  (necessary to parse "groups" property in DTS)
+ *	operations to. (necessary to parse "groups" property in DTS)
  * @get_functions_count: return number of selectable named functions available
- *	in this driver.  (necessary for pin-muxing)
+ *	in this driver. (necessary for pin-muxing)
  * @get_function_name: return the function name of the muxing selector,
  *	called by the core to figure out which mux setting it shall map a
- *	certain device to.  (necessary for pin-muxing)
+ *	certain device to. (necessary for pin-muxing)
  * @pinmux_set: enable a certain muxing function with a certain pin.
  *	The @func_selector selects a certain function whereas @pin_selector
  *	selects a certain pin to be used. On simple controllers one of them
- *	may be ignored.  (necessary for pin-muxing against a single pin)
+ *	may be ignored. (necessary for pin-muxing against a single pin)
  * @pinmux_group_set: enable a certain muxing function with a certain pin
- *	group.  The @func_selector selects a certain function whereas
+ *	group. The @func_selector selects a certain function whereas
  *	@group_selector selects a certain set of pins to be used. On simple
  *	controllers one of them may be ignored.
  *	(necessary for pin-muxing against a pin group)
+ * @pinmux_property_set: enable a pinmux group. @pinmux_group should specify the
+ *      pin identifier and mux settings. The exact format of a pinmux group is
+ *      left up to the driver. The pin selector for the mux-ed pin should be
+ *      returned on success. (necessary to parse the "pinmux" property in DTS)
  * @pinconf_num_params: number of driver-specific parameters to be parsed
  *	from device trees  (necessary for pin-configuration)
  * @pinconf_params: list of driver_specific parameters to be parsed from
@@ -90,6 +94,7 @@ struct pinctrl_ops {
 			  unsigned func_selector);
 	int (*pinmux_group_set)(struct udevice *dev, unsigned group_selector,
 				unsigned func_selector);
+	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);
 	unsigned int pinconf_num_params;
 	const struct pinconf_param *pinconf_params;
 	int (*pinconf_set)(struct udevice *dev, unsigned pin_selector,
-- 
2.28.0

[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h

[Sean Anderson]

This normalizes the documentatation to conform to kernel-doc style [1]. It
also moves the documentation for pinctrl_ops inline, and adds argument and
return-value documentation. I have kept the usual function style for these
comments. I could not find any existing examples of function documentation
inside structs.

[1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- New

 include/dm/pinctrl.h | 363 ++++++++++++++++++++++++++++---------------
 1 file changed, 242 insertions(+), 121 deletions(-)

diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
index d50af1ce38..5b994f22cc 100644
--- a/include/dm/pinctrl.h
+++ b/include/dm/pinctrl.h
@@ -11,11 +11,10 @@
 
 /**
  * struct pinconf_param - pin config parameters
- *
- * @property: property name in DT nodes
- * @param: ID for this config parameter
- * @default_value: default value for this config parameter used in case
- *	no value is specified in DT nodes
+ * @property:		Property name in DT nodes
+ * @param:		ID for this config parameter
+ * @default_value:	default value for this config parameter used in case
+ *			no value is specified in DT nodes
  */
 struct pinconf_param {
 	const char * const property;
@@ -27,111 +26,228 @@ struct pinconf_param {
  * struct pinctrl_ops - pin control operations, to be implemented by
  * pin controller drivers.
  *
- * The @set_state is the only mandatory operation.  You can implement your
- * pinctrl driver with its own @set_state.  In this case, the other callbacks
- * are not required.  Otherwise, generic pinctrl framework is also available;
- * use pinctrl_generic_set_state for @set_state, and implement other operations
+ * set_state() is the only mandatory operation. You can implement your pinctrl
+ * driver with its own @set_state. In this case, the other callbacks are not
+ * required. Otherwise, generic pinctrl framework is also available; use
+ * pinctrl_generic_set_state for @set_state, and implement other operations
  * depending on your necessity.
- *
- * @get_pins_count: return number of selectable named pins available
- *	in this driver. (necessary to parse "pins" property in DTS)
- * @get_pin_name: return the pin name of the pin selector,
- *	called by the core to figure out which pin it shall do
- *	operations to. (necessary to parse "pins" property in DTS)
- * @get_groups_count: return number of selectable named groups available
- *	in this driver. (necessary to parse "groups" property in DTS)
- * @get_group_name: return the group name of the group selector,
- *	called by the core to figure out which pin group it shall do
- *	operations to. (necessary to parse "groups" property in DTS)
- * @get_functions_count: return number of selectable named functions available
- *	in this driver. (necessary for pin-muxing)
- * @get_function_name: return the function name of the muxing selector,
- *	called by the core to figure out which mux setting it shall map a
- *	certain device to. (necessary for pin-muxing)
- * @pinmux_set: enable a certain muxing function with a certain pin.
- *	The @func_selector selects a certain function whereas @pin_selector
- *	selects a certain pin to be used. On simple controllers one of them
- *	may be ignored. (necessary for pin-muxing against a single pin)
- * @pinmux_group_set: enable a certain muxing function with a certain pin
- *	group. The @func_selector selects a certain function whereas
- *	@group_selector selects a certain set of pins to be used. On simple
- *	controllers one of them may be ignored.
- *	(necessary for pin-muxing against a pin group)
- * @pinmux_property_set: enable a pinmux group. @pinmux_group should specify the
- *      pin identifier and mux settings. The exact format of a pinmux group is
- *      left up to the driver. The pin selector for the mux-ed pin should be
- *      returned on success. (necessary to parse the "pinmux" property in DTS)
- * @pinconf_num_params: number of driver-specific parameters to be parsed
- *	from device trees  (necessary for pin-configuration)
- * @pinconf_params: list of driver_specific parameters to be parsed from
- *	device trees  (necessary for pin-configuration)
- * @pinconf_set: configure an individual pin with a given parameter.
- *	(necessary for pin-configuration against a single pin)
- * @pinconf_group_set: configure all pins in a group with a given parameter.
- *	(necessary for pin-configuration against a pin group)
- * @set_state: do pinctrl operations specified by @config, a pseudo device
- *	pointing a config node. (necessary for pinctrl_full)
- * @set_state_simple: do needed pinctrl operations for a peripherl @periph.
- *	(necessary for pinctrl_simple)
- * @get_pin_muxing: display the muxing of a given pin.
- * @gpio_request_enable: requests and enables GPIO on a certain pin.
- *	Implement this only if you can mux every pin individually as GPIO. The
- *	affected GPIO range is passed along with an offset(pin number) into that
- *	specific GPIO range - function selectors and pin groups are orthogonal
- *	to this, the core will however make sure the pins do not collide.
- * @gpio_disable_free: free up GPIO muxing on a certain pin, the reverse of
- *	@gpio_request_enable
  */
 struct pinctrl_ops {
+	/**
+	 * get_pins_count() - Get the number of selectable pins
+	 * @dev:	Pinctrl device to use
+	 *
+	 * This function is necessary to parse the "pins" property in DTS.
+	 *
+	 * Return: number of selectable named pins available in this driver
+	 */
 	int (*get_pins_count)(struct udevice *dev);
+
+	/**
+	 * get_pin_name() - Get the name of a pin
+	 * @dev:	Pinctrl device of the pin
+	 * @selector:	The pin selector
+	 *
+	 * This function is called by the core to figure out which pin it shall
+	 * do operations to. This function is necessary to parse the "pins"
+	 * property in DTS.
+	 *
+	 * Return: const pointer to the name of the pin
+	 */
 	const char *(*get_pin_name)(struct udevice *dev, unsigned selector);
+
+	/**
+	 * get_groups_count() - Get the number of selectable groups
+	 * @dev:	Pinctrl device to use
+	 *
+	 * This function is necessary to parse the "groups" property in DTS.
+	 *
+	 * Return: number of selectable named groups available in the driver
+	 */
 	int (*get_groups_count)(struct udevice *dev);
+
+	/**
+	 * get_group_name() - Get the name of a group
+	 * @dev:	Pinctrl device of the group
+	 * @selector:	The group selector
+	 *
+	 * This function is called by the core to figure out which group it
+	 * shall do operations to. This function is necessary to parse the
+	 * "groups" property in DTS.
+	 *
+	 * Return: const pointer to the name of the group
+	 */
 	const char *(*get_group_name)(struct udevice *dev, unsigned selector);
+
+	/**
+	 * get_functions_count() - Get the number of selectable functions
+	 * @dev:	Pinctrl device to use
+	 *
+	 * This function is necessary for pin-muxing.
+	 *
+	 * Return: number of selectable named functions available in this driver
+	 */
 	int (*get_functions_count)(struct udevice *dev);
+
+	/**
+	 * get_function_name() - Get the name of a function
+	 * @dev:	Pinmux device of the function
+	 * @selector:	The function selector
+	 *
+	 * This function is called by the core to figure out which mux setting
+	 * it shall map a certain device to. This function is necessary for
+	 * pin-muxing.
+	 *
+	 * Return: const pointer to the function name of the muxing selector
+	 */
 	const char *(*get_function_name)(struct udevice *dev,
 					 unsigned selector);
+
+	/**
+	 * pinmux_set() - Mux a pin to a function
+	 * @dev:		Pinctrl device to use
+	 * @pin_selector:	The pin selector
+	 * @func_selector:	The func selector
+	 *
+	 * On simple controllers one of @pin_selector or @func_selector may be
+	 * ignored. This function is necessary for pin-muxing against a single
+	 * pin.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*pinmux_set)(struct udevice *dev, unsigned pin_selector,
 			  unsigned func_selector);
+
+	/**
+	 * pinmux_set() - Mux a group of pins to a function
+	 * @dev:		Pinctrl device to use
+	 * @group_selector:	The group selector
+	 * @func_selector:	The func selector
+	 *
+	 * On simple controllers one of @group_selector or @func_selector may be
+	 * ignored. This function is necessary for pin-muxing against a group of
+	 * pins.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*pinmux_group_set)(struct udevice *dev, unsigned group_selector,
 				unsigned func_selector);
+
+	/**
+	 * pinmux_property_set() - Enable a pinmux group
+	 * @dev:		Pinctrl device to use
+	 * @pinmux_group:	A u32 representing the pin identifier and mux
+	 *			settings. The exact format of a pinmux group is
+	 *			left up to the driver.
+	 *
+	 * Mux a single pin to a single function based on a driver-specific
+	 * pinmux group. This function is necessary for parsing the "pinmux"
+	 * property in DTS, and for pin-muxing against a pinmux group.
+	 *
+	 * Return: pin selector for the muxed pin if OK, or negative error code
+	 * on failure
+	 */
 	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);
+
+	/**
+	 * @pinconf_num_params:	Number of driver-specific parameters to be
+	 *			parsed from device trees. This member is
+	 *			necessary for pin configuration.
+	 */
 	unsigned int pinconf_num_params;
+
+	/**
+	 * @pinconf_params:	List of driver-specific parameters to be parsed
+	 *			from the device tree. This member is necessary
+	 *			for pin configuration.
+	 */
 	const struct pinconf_param *pinconf_params;
+
+	/**
+	 * pinconf_set() - configure an individual pin with a parameter
+	 * @dev:		Pinctrl device to use
+	 * @pin_selector:	The pin selector
+	 * @param:		A &enum pin_config_param from @pinconf_params
+	 * @argument:		The argument to this param from the device tree
+	 *
+	 * This function is necessary for pin configuration against a single
+	 * pin.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*pinconf_set)(struct udevice *dev, unsigned pin_selector,
 			   unsigned param, unsigned argument);
+
+	/**
+	 * pinconf_group_set() - configure all pins in a group with a parameter
+	 * @dev:		Pinctrl device to use
+	 * @pin_selector:	The group selector
+	 * @param:		A &enum pin_config_param from @pinconf_params
+	 * @argument:		The argument to this param from the device tree
+	 *
+	 * This function is necessary for pin configuration against a group of
+	 * pins.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*pinconf_group_set)(struct udevice *dev, unsigned group_selector,
 				 unsigned param, unsigned argument);
+
+	/**
+	 * set_state() - Configure a pinctrl device
+	 * @dev:	Pinctrl device to use
+	 * @config:	Pseudo device pointing a config node
+	 *
+	 * This function is required to be implemented by all pinctrl drivers.
+	 * Drivers may set this member to pinctrl_generic_set_state(), which
+	 * will call other functions in &struct pinctrl_ops to parse @config.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*set_state)(struct udevice *dev, struct udevice *config);
 
-	/* for pinctrl-simple */
+	/**
+	 * set_state_simple() - Configure a pinctrl device
+	 * @dev:	Pinctrl device to use
+	 * @config:	Pseudo-device pointing a config node
+	 *
+	 * This function is usually a simpler version of set_state(). Only the
+	 * first pinctrl device on the system is supported by this function.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
+	 */
 	int (*set_state_simple)(struct udevice *dev, struct udevice *periph);
+
 	/**
 	 * request() - Request a particular pinctrl function
+	 * @dev:	Device to adjust (UCLASS_PINCTRL)
+	 * @func:	Function number (driver-specific)
 	 *
 	 * This activates the selected function.
 	 *
-	 * @dev:	Device to adjust (UCLASS_PINCTRL)
-	 * @func:	Function number (driver-specific)
-	 * @return 0 if OK, -ve on error
+	 * Return: 0 if OK, or negative error code on failure
 	 */
 	int (*request)(struct udevice *dev, int func, int flags);
 
 	/**
 	* get_periph_id() - get the peripheral ID for a device
+	* @dev:		Pinctrl device to use for decoding
+	* @periph:	Device to check
 	*
 	* This generally looks at the peripheral's device tree node to work
 	* out the peripheral ID. The return value is normally interpreted as
 	* enum periph_id. so long as this is defined by the platform (which it
 	* should be).
 	*
-	* @dev:		Pinctrl device to use for decoding
-	* @periph:	Device to check
-	* @return peripheral ID of @periph, or -ENOENT on error
+	* Return: peripheral ID of @periph, or -ENOENT on error
 	*/
 	int (*get_periph_id)(struct udevice *dev, struct udevice *periph);
 
 	/**
 	 * get_gpio_mux() - get the mux value for a particular GPIO
+	 * @dev:	Pinctrl device to use
+	 * @banknum:	GPIO bank number
+	 * @index:	GPIO index within the bank
 	 *
 	 * This allows the raw mux value for a GPIO to be obtained. It is
 	 * useful for displaying the function being used by that GPIO, such
@@ -139,46 +255,50 @@ struct pinctrl_ops {
 	 * subsystem and should not be used by generic code. Typically it is
 	 * used by a GPIO driver with knowledge of the SoC pinctrl setup.
 	 *
-	* @dev:		Pinctrl device to use
-	* @banknum:	GPIO bank number
-	* @index:	GPIO index within the bank
-	* @return mux value (SoC-specific, e.g. 0 for input, 1 for output)
+	 * Return: mux value (SoC-specific, e.g. 0 for input, 1 for output)
 	 */
 	int (*get_gpio_mux)(struct udevice *dev, int banknum, int index);
 
 	/**
 	 * get_pin_muxing() - show pin muxing
-	 *
-	 * This allows to display the muxing of a given pin. It's useful for
-	 * debug purpose to know if a pin is configured as GPIO or as an
-	 * alternate function and which one.
-	 * Typically it is used by a PINCTRL driver with knowledge of the SoC
-	 * pinctrl setup.
-	 *
 	 * @dev:	Pinctrl device to use
 	 * @selector:	Pin selector
 	 * @buf		Pin's muxing description
 	 * @size	Pin's muxing description length
-	 * return 0 if OK, -ve on error
+	 *
+	 * This allows to display the muxing of a given pin. It's useful for
+	 * debug purposes to know if a pin is configured as GPIO or as an
+	 * alternate function and which one. Typically it is used by a PINCTRL
+	 * driver with knowledge of the SoC pinctrl setup.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
 	 */
 	 int (*get_pin_muxing)(struct udevice *dev, unsigned int selector,
 			       char *buf, int size);
 
 	/**
-	 * gpio_request_enable: requests and enables GPIO on a certain pin.
-	 *
+	 * gpio_request_enable() - request and enable GPIO on a certain pin.
 	 * @dev:	Pinctrl device to use
 	 * @selector:	Pin selector
-	 * return 0 if OK, -ve on error
+	 *
+	 * Implement this only if you can mux every pin individually as GPIO.
+	 * The affected GPIO range is passed along with an offset(pin number)
+	 * into that specific GPIO range - function selectors and pin groups are
+	 * orthogonal to this, the core will however make sure the pins do not
+	 * collide.
+	 *
+	 * Return: 0 if OK, or negative error code on failure
 	 */
 	int (*gpio_request_enable)(struct udevice *dev, unsigned int selector);
 
 	/**
-	 * gpio_disable_free: free up GPIO muxing on a certain pin.
-	 *
+	 * gpio_disable_free() - free up GPIO muxing on a certain pin.
 	 * @dev:	Pinctrl device to use
 	 * @selector:	Pin selector
-	 * return 0 if OK, -ve on error
+	 *
+	 * This function is the reverse of gpio_request_enable().
+	 *
+	 * Return: 0 if OK, or negative error code on failure
 	 */
 	int (*gpio_disable_free)(struct udevice *dev, unsigned int selector);
 };
@@ -230,8 +350,8 @@ struct pinctrl_ops {
  *	push-pull mode, the argument is ignored.
  * @PIN_CONFIG_DRIVE_STRENGTH: the pin will sink or source at most the current
  *	passed as argument. The argument is in mA.
- * @PIN_CONFIG_DRIVE_STRENGTH_UA: the pin will sink or source at most the current
- *	passed as argument. The argument is in uA.
+ * @PIN_CONFIG_DRIVE_STRENGTH_UA: the pin will sink or source at most the
+ *	current passed as argument. The argument is in uA.
  * @PIN_CONFIG_INPUT_DEBOUNCE: this will configure the pin to debounce mode,
  *	which means it will wait for signals to settle when reading inputs. The
  *	argument gives the debounce time in usecs. Setting the
@@ -307,12 +427,13 @@ enum pin_config_param {
 #if CONFIG_IS_ENABLED(PINCTRL_GENERIC)
 /**
  * pinctrl_generic_set_state() - generic set_state operation
+ * @pctldev:	Pinctrl device to use
+ * @config:	Config device (pseudo device), pointing a config node in DTS
+ *
  * Parse the DT node of @config and its children and handle generic properties
  * such as "pins", "groups", "functions", and pin configuration parameters.
  *
- * @pctldev: pinctrl device
- * @config: config device (pseudo device), pointing a config node in DTS
- * @return: 0 on success, or negative error code on failure
+ * Return: 0 on success, or negative error code on failure
  */
 int pinctrl_generic_set_state(struct udevice *pctldev, struct udevice *config);
 #else
@@ -326,10 +447,10 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev,
 #if CONFIG_IS_ENABLED(PINCTRL)
 /**
  * pinctrl_select_state() - set a device to a given state
+ * @dev:	Peripheral device
+ * @statename:	State name, like "default"
  *
- * @dev: peripheral device
- * @statename: state name, like "default"
- * @return: 0 on success, or negative error code on failure
+ * Return: 0 on success, or negative error code on failure
  */
 int pinctrl_select_state(struct udevice *dev, const char *statename);
 #else
@@ -342,40 +463,43 @@ static inline int pinctrl_select_state(struct udevice *dev,
 
 /**
  * pinctrl_request() - Request a particular pinctrl function
- *
- * @dev:	Device to check (UCLASS_PINCTRL)
+ * @dev:	Pinctrl device to use
  * @func:	Function number (driver-specific)
  * @flags:	Flags (driver-specific)
- * @return 0 if OK, -ve on error
+ *
+ * Return: 0 if OK, or negative error code on failure
  */
 int pinctrl_request(struct udevice *dev, int func, int flags);
 
 /**
  * pinctrl_request_noflags() - Request a particular pinctrl function
+ * @dev:	Pinctrl device to use
+ * @func:	Function number (driver-specific)
  *
  * This is similar to pinctrl_request() but uses 0 for @flags.
  *
- * @dev:	Device to check (UCLASS_PINCTRL)
- * @func:	Function number (driver-specific)
- * @return 0 if OK, -ve on error
+ * Return: 0 if OK, or negative error code on failure
  */
 int pinctrl_request_noflags(struct udevice *dev, int func);
 
 /**
  * pinctrl_get_periph_id() - get the peripheral ID for a device
+ * @dev:	Pinctrl device to use for decoding
+ * @periph:	Device to check
  *
  * This generally looks at the peripheral's device tree node to work out the
  * peripheral ID. The return value is normally interpreted as enum periph_id.
  * so long as this is defined by the platform (which it should be).
  *
- * @dev:	Pinctrl device to use for decoding
- * @periph:	Device to check
- * @return peripheral ID of @periph, or -ENOENT on error
+ * Return: peripheral ID of @periph, or -ENOENT on error
  */
 int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
 
 /**
  * pinctrl_get_gpio_mux() - get the mux value for a particular GPIO
+ * @dev:	Pinctrl device to use
+ * @banknum:	GPIO bank number
+ * @index:	GPIO index within the bank
  *
  * This allows the raw mux value for a GPIO to be obtained. It is
  * useful for displaying the function being used by that GPIO, such
@@ -383,66 +507,63 @@ int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
  * subsystem and should not be used by generic code. Typically it is
  * used by a GPIO driver with knowledge of the SoC pinctrl setup.
  *
- * @dev:	Pinctrl device to use
- * @banknum:	GPIO bank number
- * @index:	GPIO index within the bank
- * @return mux value (SoC-specific, e.g. 0 for input, 1 for output)
+ * Return: mux value (SoC-specific, e.g. 0 for input, 1 for output)
 */
 int pinctrl_get_gpio_mux(struct udevice *dev, int banknum, int index);
 
 /**
  * pinctrl_get_pin_muxing() - Returns the muxing description
+ * @dev:	Pinctrl device to use
+ * @selector:	Pin index within pin-controller
+ * @buf:	Pin's muxing description
+ * @size:	Pin's muxing description length
  *
  * This allows to display the muxing description of the given pin for
  * debug purpose
  *
- * @dev:	Pinctrl device to use
- * @selector	Pin index within pin-controller
- * @buf		Pin's muxing description
- * @size	Pin's muxing description length
- * @return 0 if OK, -ve on error
+ * Return: 0 if OK, or negative error code on failure
  */
 int pinctrl_get_pin_muxing(struct udevice *dev, int selector, char *buf,
 			   int size);
 
 /**
  * pinctrl_get_pins_count() - display pin-controller pins number
+ * @dev:	Pinctrl device to use
  *
  * This allows to know the number of pins owned by a given pin-controller
  *
- * @dev:	Pinctrl device to use
- * @return pins number if OK, -ve on error
+ * Return: number of pins if OK, or negative error code on failure
  */
 int pinctrl_get_pins_count(struct udevice *dev);
 
 /**
  * pinctrl_get_pin_name() - Returns the pin's name
+ * @dev:	Pinctrl device to use
+ * @selector:	Pin index within pin-controller
+ * @buf:	Pin's name
  *
  * This allows to display the pin's name for debug purpose
  *
- * @dev:	Pinctrl device to use
- * @selector	Pin index within pin-controller
- * @buf		Pin's name
- * @return 0 if OK, -ve on error
+ * Return: 0 if OK, or negative error code on failure
  */
 int pinctrl_get_pin_name(struct udevice *dev, int selector, char *buf,
 			 int size);
 
 /**
  * pinctrl_gpio_request() - request a single pin to be used as GPIO
+ * @dev:	GPIO peripheral device
+ * @offset:	GPIO pin offset from the GPIO controller
  *
- * @dev: GPIO peripheral device
- * @offset: the GPIO pin offset from the GPIO controller
- * @return: 0 on success, or negative error code on failure
+ * Return: 0 on success, or negative error code on failure
  */
 int pinctrl_gpio_request(struct udevice *dev, unsigned offset);
 
 /**
  * pinctrl_gpio_free() - free a single pin used as GPIO
+ * @dev:	GPIO peripheral device
+ * @offset:	GPIO pin offset from the GPIO controller
  *
- * @dev: GPIO peripheral device
- * @offset: the GPIO pin offset from the GPIO controller
- * @return: 0 on success, or negative error code on failure
+ * Return: 0 on success, or negative error code on failure
  */
 int pinctrl_gpio_free(struct udevice *dev, unsigned offset);
 
-- 
2.28.0

[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h

[Heinrich Schuchardt]

On 15.08.20 17:52, Sean Anderson wrote:
> This normalizes the documentatation to conform to kernel-doc style [1]. It

Nits:
%s/documentatation/documentation/

> also moves the documentation for pinctrl_ops inline, and adds argument and
> return-value documentation. I have kept the usual function style for these
> comments. I could not find any existing examples of function documentation
> inside structs.
>
> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>


Before and after this patch the documentation format is incorrect. See
output below for the situation after the patch.

Please include include/dm/pinctrl.h into doc/api/index.rst via a new
file doc/api/pnctrl.rst.

A documentation style accepted by 'make htmldocs' is:

struct pinctrl_ops {
??? /**
???? * @get_pins_count:?????????? Get the number of selectable pins
???? *
???? * @get_pins_count.dev:?????? Pinctrl device to use
???? *
???? * This function is necessary to parse the "pins" property
     * in DTS.
???? *
???? * @get_pins_count.Return:??? number of selectable named pins
???? *??????????????????????????? available in this driver
???? */
    int (*get_pins_count)(struct udevice *dev);

See the nested structures in

https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#in-line-member-documentation-comments
and
https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#nested-structs-unions

The generated doc looks like this:

Members
get_pins_count
    Get the number of selectable pins
    get_pins_count.dev: Pinctrl device to use
    This function is necessary to parse the ?pins? property in DTS.
    get_pins_count.Return: number of selectable named pins
        available in this driver

Best regards

Heinrich

./include/dm/pinctrl.h:38: warning: Incorrect use of kernel-doc format:
        * get_pins_count() - Get the number of selectable pins

./include/dm/pinctrl.h:48: warning: Incorrect use of kernel-doc format:
        * get_pin_name() - Get the name of a pin
./include/dm/pinctrl.h:61: warning: Incorrect use of kernel-doc format:
        * get_groups_count() - Get the number of selectable groups
./include/dm/pinctrl.h:71: warning: Incorrect use of kernel-doc format:
        * get_group_name() - Get the name of a group
./include/dm/pinctrl.h:84: warning: Incorrect use of kernel-doc format:
        * get_functions_count() - Get the number of selectable functions
./include/dm/pinctrl.h:94: warning: Incorrect use of kernel-doc format:
        * get_function_name() - Get the name of a function
./include/dm/pinctrl.h:108: warning: Incorrect use of kernel-doc format:
         * pinmux_set() - Mux a pin to a function
./include/dm/pinctrl.h:123: warning: Incorrect use of kernel-doc format:
         * pinmux_set() - Mux a group of pins to a function
./include/dm/pinctrl.h:138: warning: Incorrect use of kernel-doc format:
         * pinmux_property_set() - Enable a pinmux group
./include/dm/pinctrl.h:168: warning: Incorrect use of kernel-doc format:
         * pinconf_set() - configure an individual pin with a parameter
./include/dm/pinctrl.h:183: warning: Incorrect use of kernel-doc format:
         * pinconf_group_set() - configure all pins in a group with a
parameter
./include/dm/pinctrl.h:198: warning: Incorrect use of kernel-doc format:
         * set_state() - Configure a pinctrl device
./include/dm/pinctrl.h:211: warning: Incorrect use of kernel-doc format:
         * set_state_simple() - Configure a pinctrl device
./include/dm/pinctrl.h:223: warning: Incorrect use of kernel-doc format:
         * request() - Request a particular pinctrl function
./include/dm/pinctrl.h:234: warning: Incorrect use of kernel-doc format:
        * get_periph_id() - get the peripheral ID for a device
./include/dm/pinctrl.h:248: warning: Incorrect use of kernel-doc format:
         * get_gpio_mux() - get the mux value for a particular GPIO
./include/dm/pinctrl.h:264: warning: Incorrect use of kernel-doc format:
         * get_pin_muxing() - show pin muxing
./include/dm/pinctrl.h:281: warning: Incorrect use of kernel-doc format:
         * gpio_request_enable() - request and enable GPIO on a certain pin.
./include/dm/pinctrl.h:296: warning: Incorrect use of kernel-doc format:
         * gpio_disable_free() - free up GPIO muxing on a certain pin.
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_pins_count' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_pin_name' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_groups_count' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_group_name' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_functions_count' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_function_name' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'pinmux_set' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'pinmux_group_set' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'pinmux_property_set' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'pinconf_set' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'pinconf_group_set' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'set_state' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'set_state_simple' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'request' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_periph_id' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_gpio_mux' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'get_pin_muxing' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'gpio_request_enable' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:305: warning: Function parameter or member
'gpio_disable_free' not described in 'pinctrl_ops'
./include/dm/pinctrl.h:401: warning: cannot understand function
prototype: 'enum pin_config_param '
./include/dm/pinctrl.h:551: warning: Function parameter or member 'size'
not described in 'pinctrl_get_pin_name'

> ---
>
> (no changes since v4)
>
> Changes in v4:
> - New
>
>  include/dm/pinctrl.h | 363 ++++++++++++++++++++++++++++---------------
>  1 file changed, 242 insertions(+), 121 deletions(-)
>
> diff --git a/include/dm/pinctrl.h b/include/dm/pinctrl.h
> index d50af1ce38..5b994f22cc 100644
> --- a/include/dm/pinctrl.h
> +++ b/include/dm/pinctrl.h
> @@ -11,11 +11,10 @@
>
>  /**
>   * struct pinconf_param - pin config parameters
> - *
> - * @property: property name in DT nodes
> - * @param: ID for this config parameter
> - * @default_value: default value for this config parameter used in case
> - *	no value is specified in DT nodes
> + * @property:		Property name in DT nodes
> + * @param:		ID for this config parameter
> + * @default_value:	default value for this config parameter used in case
> + *			no value is specified in DT nodes
>   */
>  struct pinconf_param {
>  	const char * const property;
> @@ -27,111 +26,228 @@ struct pinconf_param {
>   * struct pinctrl_ops - pin control operations, to be implemented by
>   * pin controller drivers.
>   *
> - * The @set_state is the only mandatory operation.  You can implement your
> - * pinctrl driver with its own @set_state.  In this case, the other callbacks
> - * are not required.  Otherwise, generic pinctrl framework is also available;
> - * use pinctrl_generic_set_state for @set_state, and implement other operations
> + * set_state() is the only mandatory operation. You can implement your pinctrl
> + * driver with its own @set_state. In this case, the other callbacks are not
> + * required. Otherwise, generic pinctrl framework is also available; use
> + * pinctrl_generic_set_state for @set_state, and implement other operations
>   * depending on your necessity.
> - *
> - * @get_pins_count: return number of selectable named pins available
> - *	in this driver. (necessary to parse "pins" property in DTS)
> - * @get_pin_name: return the pin name of the pin selector,
> - *	called by the core to figure out which pin it shall do
> - *	operations to. (necessary to parse "pins" property in DTS)
> - * @get_groups_count: return number of selectable named groups available
> - *	in this driver. (necessary to parse "groups" property in DTS)
> - * @get_group_name: return the group name of the group selector,
> - *	called by the core to figure out which pin group it shall do
> - *	operations to. (necessary to parse "groups" property in DTS)
> - * @get_functions_count: return number of selectable named functions available
> - *	in this driver. (necessary for pin-muxing)
> - * @get_function_name: return the function name of the muxing selector,
> - *	called by the core to figure out which mux setting it shall map a
> - *	certain device to. (necessary for pin-muxing)
> - * @pinmux_set: enable a certain muxing function with a certain pin.
> - *	The @func_selector selects a certain function whereas @pin_selector
> - *	selects a certain pin to be used. On simple controllers one of them
> - *	may be ignored. (necessary for pin-muxing against a single pin)
> - * @pinmux_group_set: enable a certain muxing function with a certain pin
> - *	group. The @func_selector selects a certain function whereas
> - *	@group_selector selects a certain set of pins to be used. On simple
> - *	controllers one of them may be ignored.
> - *	(necessary for pin-muxing against a pin group)
> - * @pinmux_property_set: enable a pinmux group. @pinmux_group should specify the
> - *      pin identifier and mux settings. The exact format of a pinmux group is
> - *      left up to the driver. The pin selector for the mux-ed pin should be
> - *      returned on success. (necessary to parse the "pinmux" property in DTS)
> - * @pinconf_num_params: number of driver-specific parameters to be parsed
> - *	from device trees  (necessary for pin-configuration)
> - * @pinconf_params: list of driver_specific parameters to be parsed from
> - *	device trees  (necessary for pin-configuration)
> - * @pinconf_set: configure an individual pin with a given parameter.
> - *	(necessary for pin-configuration against a single pin)
> - * @pinconf_group_set: configure all pins in a group with a given parameter.
> - *	(necessary for pin-configuration against a pin group)
> - * @set_state: do pinctrl operations specified by @config, a pseudo device
> - *	pointing a config node. (necessary for pinctrl_full)
> - * @set_state_simple: do needed pinctrl operations for a peripherl @periph.
> - *	(necessary for pinctrl_simple)
> - * @get_pin_muxing: display the muxing of a given pin.
> - * @gpio_request_enable: requests and enables GPIO on a certain pin.
> - *	Implement this only if you can mux every pin individually as GPIO. The
> - *	affected GPIO range is passed along with an offset(pin number) into that
> - *	specific GPIO range - function selectors and pin groups are orthogonal
> - *	to this, the core will however make sure the pins do not collide.
> - * @gpio_disable_free: free up GPIO muxing on a certain pin, the reverse of
> - *	@gpio_request_enable
>   */
>  struct pinctrl_ops {
> +	/**
> +	 * get_pins_count() - Get the number of selectable pins
> +	 * @dev:	Pinctrl device to use
> +	 *
> +	 * This function is necessary to parse the "pins" property in DTS.
> +	 *
> +	 * Return: number of selectable named pins available in this driver
> +	 */
>  	int (*get_pins_count)(struct udevice *dev);
> +
> +	/**
> +	 * get_pin_name() - Get the name of a pin
> +	 * @dev:	Pinctrl device of the pin
> +	 * @selector:	The pin selector
> +	 *
> +	 * This function is called by the core to figure out which pin it shall
> +	 * do operations to. This function is necessary to parse the "pins"
> +	 * property in DTS.
> +	 *
> +	 * Return: const pointer to the name of the pin
> +	 */
>  	const char *(*get_pin_name)(struct udevice *dev, unsigned selector);
> +
> +	/**
> +	 * get_groups_count() - Get the number of selectable groups
> +	 * @dev:	Pinctrl device to use
> +	 *
> +	 * This function is necessary to parse the "groups" property in DTS.
> +	 *
> +	 * Return: number of selectable named groups available in the driver
> +	 */
>  	int (*get_groups_count)(struct udevice *dev);
> +
> +	/**
> +	 * get_group_name() - Get the name of a group
> +	 * @dev:	Pinctrl device of the group
> +	 * @selector:	The group selector
> +	 *
> +	 * This function is called by the core to figure out which group it
> +	 * shall do operations to. This function is necessary to parse the
> +	 * "groups" property in DTS.
> +	 *
> +	 * Return: const pointer to the name of the group
> +	 */
>  	const char *(*get_group_name)(struct udevice *dev, unsigned selector);
> +
> +	/**
> +	 * get_functions_count() - Get the number of selectable functions
> +	 * @dev:	Pinctrl device to use
> +	 *
> +	 * This function is necessary for pin-muxing.
> +	 *
> +	 * Return: number of selectable named functions available in this driver
> +	 */
>  	int (*get_functions_count)(struct udevice *dev);
> +
> +	/**
> +	 * get_function_name() - Get the name of a function
> +	 * @dev:	Pinmux device of the function
> +	 * @selector:	The function selector
> +	 *
> +	 * This function is called by the core to figure out which mux setting
> +	 * it shall map a certain device to. This function is necessary for
> +	 * pin-muxing.
> +	 *
> +	 * Return: const pointer to the function name of the muxing selector
> +	 */
>  	const char *(*get_function_name)(struct udevice *dev,
>  					 unsigned selector);
> +
> +	/**
> +	 * pinmux_set() - Mux a pin to a function
> +	 * @dev:		Pinctrl device to use
> +	 * @pin_selector:	The pin selector
> +	 * @func_selector:	The func selector
> +	 *
> +	 * On simple controllers one of @pin_selector or @func_selector may be
> +	 * ignored. This function is necessary for pin-muxing against a single
> +	 * pin.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*pinmux_set)(struct udevice *dev, unsigned pin_selector,
>  			  unsigned func_selector);
> +
> +	/**
> +	 * pinmux_set() - Mux a group of pins to a function
> +	 * @dev:		Pinctrl device to use
> +	 * @group_selector:	The group selector
> +	 * @func_selector:	The func selector
> +	 *
> +	 * On simple controllers one of @group_selector or @func_selector may be
> +	 * ignored. This function is necessary for pin-muxing against a group of
> +	 * pins.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*pinmux_group_set)(struct udevice *dev, unsigned group_selector,
>  				unsigned func_selector);
> +
> +	/**
> +	 * pinmux_property_set() - Enable a pinmux group
> +	 * @dev:		Pinctrl device to use
> +	 * @pinmux_group:	A u32 representing the pin identifier and mux
> +	 *			settings. The exact format of a pinmux group is
> +	 *			left up to the driver.
> +	 *
> +	 * Mux a single pin to a single function based on a driver-specific
> +	 * pinmux group. This function is necessary for parsing the "pinmux"
> +	 * property in DTS, and for pin-muxing against a pinmux group.
> +	 *
> +	 * Return: pin selector for the muxed pin if OK, or negative error code
> +	 * on failure
> +	 */
>  	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);
> +
> +	/**
> +	 * @pinconf_num_params:	Number of driver-specific parameters to be
> +	 *			parsed from device trees. This member is
> +	 *			necessary for pin configuration.
> +	 */
>  	unsigned int pinconf_num_params;
> +
> +	/**
> +	 * @pinconf_params:	List of driver-specific parameters to be parsed
> +	 *			from the device tree. This member is necessary
> +	 *			for pin configuration.
> +	 */
>  	const struct pinconf_param *pinconf_params;
> +
> +	/**
> +	 * pinconf_set() - configure an individual pin with a parameter
> +	 * @dev:		Pinctrl device to use
> +	 * @pin_selector:	The pin selector
> +	 * @param:		A &enum pin_config_param from @pinconf_params
> +	 * @argument:		The argument to this param from the device tree
> +	 *
> +	 * This function is necessary for pin configuration against a single
> +	 * pin.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*pinconf_set)(struct udevice *dev, unsigned pin_selector,
>  			   unsigned param, unsigned argument);
> +
> +	/**
> +	 * pinconf_group_set() - configure all pins in a group with a parameter
> +	 * @dev:		Pinctrl device to use
> +	 * @pin_selector:	The group selector
> +	 * @param:		A &enum pin_config_param from @pinconf_params
> +	 * @argument:		The argument to this param from the device tree
> +	 *
> +	 * This function is necessary for pin configuration against a group of
> +	 * pins.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*pinconf_group_set)(struct udevice *dev, unsigned group_selector,
>  				 unsigned param, unsigned argument);
> +
> +	/**
> +	 * set_state() - Configure a pinctrl device
> +	 * @dev:	Pinctrl device to use
> +	 * @config:	Pseudo device pointing a config node
> +	 *
> +	 * This function is required to be implemented by all pinctrl drivers.
> +	 * Drivers may set this member to pinctrl_generic_set_state(), which
> +	 * will call other functions in &struct pinctrl_ops to parse @config.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*set_state)(struct udevice *dev, struct udevice *config);
>
> -	/* for pinctrl-simple */
> +	/**
> +	 * set_state_simple() - Configure a pinctrl device
> +	 * @dev:	Pinctrl device to use
> +	 * @config:	Pseudo-device pointing a config node
> +	 *
> +	 * This function is usually a simpler version of set_state(). Only the
> +	 * first pinctrl device on the system is supported by this function.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
> +	 */
>  	int (*set_state_simple)(struct udevice *dev, struct udevice *periph);
> +
>  	/**
>  	 * request() - Request a particular pinctrl function
> +	 * @dev:	Device to adjust (UCLASS_PINCTRL)
> +	 * @func:	Function number (driver-specific)
>  	 *
>  	 * This activates the selected function.
>  	 *
> -	 * @dev:	Device to adjust (UCLASS_PINCTRL)
> -	 * @func:	Function number (driver-specific)
> -	 * @return 0 if OK, -ve on error
> +	 * Return: 0 if OK, or negative error code on failure
>  	 */
>  	int (*request)(struct udevice *dev, int func, int flags);
>
>  	/**
>  	* get_periph_id() - get the peripheral ID for a device
> +	* @dev:		Pinctrl device to use for decoding
> +	* @periph:	Device to check
>  	*
>  	* This generally looks at the peripheral's device tree node to work
>  	* out the peripheral ID. The return value is normally interpreted as
>  	* enum periph_id. so long as this is defined by the platform (which it
>  	* should be).
>  	*
> -	* @dev:		Pinctrl device to use for decoding
> -	* @periph:	Device to check
> -	* @return peripheral ID of @periph, or -ENOENT on error
> +	* Return: peripheral ID of @periph, or -ENOENT on error
>  	*/
>  	int (*get_periph_id)(struct udevice *dev, struct udevice *periph);
>
>  	/**
>  	 * get_gpio_mux() - get the mux value for a particular GPIO
> +	 * @dev:	Pinctrl device to use
> +	 * @banknum:	GPIO bank number
> +	 * @index:	GPIO index within the bank
>  	 *
>  	 * This allows the raw mux value for a GPIO to be obtained. It is
>  	 * useful for displaying the function being used by that GPIO, such
> @@ -139,46 +255,50 @@ struct pinctrl_ops {
>  	 * subsystem and should not be used by generic code. Typically it is
>  	 * used by a GPIO driver with knowledge of the SoC pinctrl setup.
>  	 *
> -	* @dev:		Pinctrl device to use
> -	* @banknum:	GPIO bank number
> -	* @index:	GPIO index within the bank
> -	* @return mux value (SoC-specific, e.g. 0 for input, 1 for output)
> +	 * Return: mux value (SoC-specific, e.g. 0 for input, 1 for output)
>  	 */
>  	int (*get_gpio_mux)(struct udevice *dev, int banknum, int index);
>
>  	/**
>  	 * get_pin_muxing() - show pin muxing
> -	 *
> -	 * This allows to display the muxing of a given pin. It's useful for
> -	 * debug purpose to know if a pin is configured as GPIO or as an
> -	 * alternate function and which one.
> -	 * Typically it is used by a PINCTRL driver with knowledge of the SoC
> -	 * pinctrl setup.
> -	 *
>  	 * @dev:	Pinctrl device to use
>  	 * @selector:	Pin selector
>  	 * @buf		Pin's muxing description
>  	 * @size	Pin's muxing description length
> -	 * return 0 if OK, -ve on error
> +	 *
> +	 * This allows to display the muxing of a given pin. It's useful for
> +	 * debug purposes to know if a pin is configured as GPIO or as an
> +	 * alternate function and which one. Typically it is used by a PINCTRL
> +	 * driver with knowledge of the SoC pinctrl setup.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
>  	 */
>  	 int (*get_pin_muxing)(struct udevice *dev, unsigned int selector,
>  			       char *buf, int size);
>
>  	/**
> -	 * gpio_request_enable: requests and enables GPIO on a certain pin.
> -	 *
> +	 * gpio_request_enable() - request and enable GPIO on a certain pin.
>  	 * @dev:	Pinctrl device to use
>  	 * @selector:	Pin selector
> -	 * return 0 if OK, -ve on error
> +	 *
> +	 * Implement this only if you can mux every pin individually as GPIO.
> +	 * The affected GPIO range is passed along with an offset(pin number)
> +	 * into that specific GPIO range - function selectors and pin groups are
> +	 * orthogonal to this, the core will however make sure the pins do not
> +	 * collide.
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
>  	 */
>  	int (*gpio_request_enable)(struct udevice *dev, unsigned int selector);
>
>  	/**
> -	 * gpio_disable_free: free up GPIO muxing on a certain pin.
> -	 *
> +	 * gpio_disable_free() - free up GPIO muxing on a certain pin.
>  	 * @dev:	Pinctrl device to use
>  	 * @selector:	Pin selector
> -	 * return 0 if OK, -ve on error
> +	 *
> +	 * This function is the reverse of gpio_request_enable().
> +	 *
> +	 * Return: 0 if OK, or negative error code on failure
>  	 */
>  	int (*gpio_disable_free)(struct udevice *dev, unsigned int selector);
>  };
> @@ -230,8 +350,8 @@ struct pinctrl_ops {
>   *	push-pull mode, the argument is ignored.
>   * @PIN_CONFIG_DRIVE_STRENGTH: the pin will sink or source at most the current
>   *	passed as argument. The argument is in mA.
> - * @PIN_CONFIG_DRIVE_STRENGTH_UA: the pin will sink or source at most the current
> - *	passed as argument. The argument is in uA.
> + * @PIN_CONFIG_DRIVE_STRENGTH_UA: the pin will sink or source at most the
> + *	current passed as argument. The argument is in uA.
>   * @PIN_CONFIG_INPUT_DEBOUNCE: this will configure the pin to debounce mode,
>   *	which means it will wait for signals to settle when reading inputs. The
>   *	argument gives the debounce time in usecs. Setting the
> @@ -307,12 +427,13 @@ enum pin_config_param {
>  #if CONFIG_IS_ENABLED(PINCTRL_GENERIC)
>  /**
>   * pinctrl_generic_set_state() - generic set_state operation
> + * @pctldev:	Pinctrl device to use
> + * @config:	Config device (pseudo device), pointing a config node in DTS
> + *
>   * Parse the DT node of @config and its children and handle generic properties
>   * such as "pins", "groups", "functions", and pin configuration parameters.
>   *
> - * @pctldev: pinctrl device
> - * @config: config device (pseudo device), pointing a config node in DTS
> - * @return: 0 on success, or negative error code on failure
> + * Return: 0 on success, or negative error code on failure
>   */
>  int pinctrl_generic_set_state(struct udevice *pctldev, struct udevice *config);
>  #else
> @@ -326,10 +447,10 @@ static inline int pinctrl_generic_set_state(struct udevice *pctldev,
>  #if CONFIG_IS_ENABLED(PINCTRL)
>  /**
>   * pinctrl_select_state() - set a device to a given state
> + * @dev:	Peripheral device
> + * @statename:	State name, like "default"
>   *
> - * @dev: peripheral device
> - * @statename: state name, like "default"
> - * @return: 0 on success, or negative error code on failure
> + * Return: 0 on success, or negative error code on failure
>   */
>  int pinctrl_select_state(struct udevice *dev, const char *statename);
>  #else
> @@ -342,40 +463,43 @@ static inline int pinctrl_select_state(struct udevice *dev,
>
>  /**
>   * pinctrl_request() - Request a particular pinctrl function
> - *
> - * @dev:	Device to check (UCLASS_PINCTRL)
> + * @dev:	Pinctrl device to use
>   * @func:	Function number (driver-specific)
>   * @flags:	Flags (driver-specific)
> - * @return 0 if OK, -ve on error
> + *
> + * Return: 0 if OK, or negative error code on failure
>   */
>  int pinctrl_request(struct udevice *dev, int func, int flags);
>
>  /**
>   * pinctrl_request_noflags() - Request a particular pinctrl function
> + * @dev:	Pinctrl device to use
> + * @func:	Function number (driver-specific)
>   *
>   * This is similar to pinctrl_request() but uses 0 for @flags.
>   *
> - * @dev:	Device to check (UCLASS_PINCTRL)
> - * @func:	Function number (driver-specific)
> - * @return 0 if OK, -ve on error
> + * Return: 0 if OK, or negative error code on failure
>   */
>  int pinctrl_request_noflags(struct udevice *dev, int func);
>
>  /**
>   * pinctrl_get_periph_id() - get the peripheral ID for a device
> + * @dev:	Pinctrl device to use for decoding
> + * @periph:	Device to check
>   *
>   * This generally looks at the peripheral's device tree node to work out the
>   * peripheral ID. The return value is normally interpreted as enum periph_id.
>   * so long as this is defined by the platform (which it should be).
>   *
> - * @dev:	Pinctrl device to use for decoding
> - * @periph:	Device to check
> - * @return peripheral ID of @periph, or -ENOENT on error
> + * Return: peripheral ID of @periph, or -ENOENT on error
>   */
>  int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
>
>  /**
>   * pinctrl_get_gpio_mux() - get the mux value for a particular GPIO
> + * @dev:	Pinctrl device to use
> + * @banknum:	GPIO bank number
> + * @index:	GPIO index within the bank
>   *
>   * This allows the raw mux value for a GPIO to be obtained. It is
>   * useful for displaying the function being used by that GPIO, such
> @@ -383,66 +507,63 @@ int pinctrl_get_periph_id(struct udevice *dev, struct udevice *periph);
>   * subsystem and should not be used by generic code. Typically it is
>   * used by a GPIO driver with knowledge of the SoC pinctrl setup.
>   *
> - * @dev:	Pinctrl device to use
> - * @banknum:	GPIO bank number
> - * @index:	GPIO index within the bank
> - * @return mux value (SoC-specific, e.g. 0 for input, 1 for output)
> + * Return: mux value (SoC-specific, e.g. 0 for input, 1 for output)
>  */
>  int pinctrl_get_gpio_mux(struct udevice *dev, int banknum, int index);
>
>  /**
>   * pinctrl_get_pin_muxing() - Returns the muxing description
> + * @dev:	Pinctrl device to use
> + * @selector:	Pin index within pin-controller
> + * @buf:	Pin's muxing description
> + * @size:	Pin's muxing description length
>   *
>   * This allows to display the muxing description of the given pin for
>   * debug purpose
>   *
> - * @dev:	Pinctrl device to use
> - * @selector	Pin index within pin-controller
> - * @buf		Pin's muxing description
> - * @size	Pin's muxing description length
> - * @return 0 if OK, -ve on error
> + * Return: 0 if OK, or negative error code on failure
>   */
>  int pinctrl_get_pin_muxing(struct udevice *dev, int selector, char *buf,
>  			   int size);
>
>  /**
>   * pinctrl_get_pins_count() - display pin-controller pins number
> + * @dev:	Pinctrl device to use
>   *
>   * This allows to know the number of pins owned by a given pin-controller
>   *
> - * @dev:	Pinctrl device to use
> - * @return pins number if OK, -ve on error
> + * Return: number of pins if OK, or negative error code on failure
>   */
>  int pinctrl_get_pins_count(struct udevice *dev);
>
>  /**
>   * pinctrl_get_pin_name() - Returns the pin's name
> + * @dev:	Pinctrl device to use
> + * @selector:	Pin index within pin-controller
> + * @buf:	Pin's name
>   *
>   * This allows to display the pin's name for debug purpose
>   *
> - * @dev:	Pinctrl device to use
> - * @selector	Pin index within pin-controller
> - * @buf		Pin's name
> - * @return 0 if OK, -ve on error
> + * Return: 0 if OK, or negative error code on failure
>   */
>  int pinctrl_get_pin_name(struct udevice *dev, int selector, char *buf,
>  			 int size);
>
>  /**
>   * pinctrl_gpio_request() - request a single pin to be used as GPIO
> + * @dev:	GPIO peripheral device
> + * @offset:	GPIO pin offset from the GPIO controller
>   *
> - * @dev: GPIO peripheral device
> - * @offset: the GPIO pin offset from the GPIO controller
> - * @return: 0 on success, or negative error code on failure
> + * Return: 0 on success, or negative error code on failure
>   */
>  int pinctrl_gpio_request(struct udevice *dev, unsigned offset);
>
>  /**
>   * pinctrl_gpio_free() - free a single pin used as GPIO
> + * @dev:	GPIO peripheral device
> + * @offset:	GPIO pin offset from the GPIO controller
>   *
> - * @dev: GPIO peripheral device
> - * @offset: the GPIO pin offset from the GPIO controller
> - * @return: 0 on success, or negative error code on failure
> + * Return: 0 on success, or negative error code on failure
>   */
>  int pinctrl_gpio_free(struct udevice *dev, unsigned offset);
>
>

[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h

[Sean Anderson]

On 9/2/20 8:26 AM, Heinrich Schuchardt wrote:
> On 15.08.20 17:52, Sean Anderson wrote:
>> This normalizes the documentatation to conform to kernel-doc style [1]. It
> 
> Nits:
> %s/documentatation/documentation/
> 
>> also moves the documentation for pinctrl_ops inline, and adds argument and
>> return-value documentation. I have kept the usual function style for these
>> comments. I could not find any existing examples of function documentation
>> inside structs.
>>
>> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> 
> Before and after this patch the documentation format is incorrect. See
> output below for the situation after the patch.
> 
> Please include include/dm/pinctrl.h into doc/api/index.rst via a new
> file doc/api/pnctrl.rst.
> 
> A documentation style accepted by 'make htmldocs' is:
> 
> struct pinctrl_ops {
>     /**
>      * @get_pins_count:           Get the number of selectable pins
>      *
>      * @get_pins_count.dev:       Pinctrl device to use
>      *
>      * This function is necessary to parse the "pins" property
>      * in DTS.
>      *
>      * @get_pins_count.Return:    number of selectable named pins
>      *                            available in this driver
>      */
>     int (*get_pins_count)(struct udevice *dev);
> 
> See the nested structures in
> 
> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#in-line-member-documentation-comments
> and
> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#nested-structs-unions

Thanks for suggesting those changes. This works better. As far as I can
tell, fully-qualified members are only necessary when documenting nested
structs at the top-level. When using in-line documentation, members are
automatically nested. With that in mind, I am using the following format

	/**
	 * @pinmux_property_set: Enable a pinmux group
	 *
	 * @dev: Pinctrl device to use
	 *
	 * @pinmux_group: A u32 representing the pin identifier and mux
	 *                settings. The exact format of a pinmux group is left
	 *                up to the driver.
	 *
	 * Mux a single pin to a single function based on a driver-specific
	 * pinmux group. This function is necessary for parsing the "pinmux"
	 * property in DTS, and for pin-muxing against a pinmux group.
	 *
	 * @Return:
	 *	Pin selector for the muxed pin if OK, or negative error code on
	 *	failure
	 */
	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);

However, I am having some problems with multi-line descriptions. In the
example above, the desciption for @pinmux_group has the first line
bolded and the rest of the description is typeset as normal, but is
indented. The generated html is

<dl class="docutils">
<dt><strong>pinmux_group</strong>: A u32 representing the pin identifier
and mux</dt>
<dd>settings. The exact format of a pinmux group is left up to the
driver.</dd>
</dl>

However, it should be something like

<p><strong>pinmux_group</strong>: A u32 representing the pin identifier
and mux settings. The exact format of a pinmux group is left up to the
driver.</p>

I have also tried an alternate style, as shown for @Return. However,
this too generated the wrong html:

<dl class="last docutils">
<dt><strong>Return</strong>:</dt>
<dd>Pin selector for the muxed pin if OK, or negative error code on
failure</dd>
</dl>

where it should generate something like

<p class="last"><strong>Return</strong>: Pin selector for the muxed pin
if OK, or negative error code on failure</p>

Do you have any suggestions for generating the latter forms? My current
work is at [1].

--Sean

[1] https://github.com/Forty-Bot/u-boot/commit/e835cf9552dc96438699806731a208b40daead7b

[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h

[Heinrich Schuchardt]

On 02.09.20 16:56, Sean Anderson wrote:
>
> On 9/2/20 8:26 AM, Heinrich Schuchardt wrote:
>> On 15.08.20 17:52, Sean Anderson wrote:
>>> This normalizes the documentatation to conform to kernel-doc style [1]. It
>>
>> Nits:
>> %s/documentatation/documentation/
>>
>>> also moves the documentation for pinctrl_ops inline, and adds argument and
>>> return-value documentation. I have kept the usual function style for these
>>> comments. I could not find any existing examples of function documentation
>>> inside structs.
>>>
>>> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>>>
>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>>
>> Before and after this patch the documentation format is incorrect. See
>> output below for the situation after the patch.
>>
>> Please include include/dm/pinctrl.h into doc/api/index.rst via a new
>> file doc/api/pnctrl.rst.
>>
>> A documentation style accepted by 'make htmldocs' is:
>>
>> struct pinctrl_ops {
>>     /**
>>      * @get_pins_count:           Get the number of selectable pins
>>      *
>>      * @get_pins_count.dev:       Pinctrl device to use
>>      *
>>      * This function is necessary to parse the "pins" property
>>      * in DTS.
>>      *
>>      * @get_pins_count.Return:    number of selectable named pins
>>      *                            available in this driver
>>      */
>>     int (*get_pins_count)(struct udevice *dev);
>>
>> See the nested structures in
>>
>> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#in-line-member-documentation-comments
>> and
>> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#nested-structs-unions
>
> Thanks for suggesting those changes. This works better. As far as I can
> tell, fully-qualified members are only necessary when documenting nested
> structs at the top-level. When using in-line documentation, members are
> automatically nested. With that in mind, I am using the following format
>
> 	/**
> 	 * @pinmux_property_set: Enable a pinmux group
> 	 *
> 	 * @dev: Pinctrl device to use
> 	 *
> 	 * @pinmux_group: A u32 representing the pin identifier and mux
> 	 *                settings. The exact format of a pinmux group is left
> 	 *                up to the driver.
> 	 *
> 	 * Mux a single pin to a single function based on a driver-specific
> 	 * pinmux group. This function is necessary for parsing the "pinmux"
> 	 * property in DTS, and for pin-muxing against a pinmux group.
> 	 *
> 	 * @Return:
> 	 *	Pin selector for the muxed pin if OK, or negative error code on
> 	 *	failure
> 	 */
> 	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);
>
> However, I am having some problems with multi-line descriptions. In the
> example above, the desciption for @pinmux_group has the first line
> bolded and the rest of the description is typeset as normal, but is
> indented. The generated html is

Use fully qualified names and you are fine.

Best regards

Heinrich

>
> <dl class="docutils">
> <dt><strong>pinmux_group</strong>: A u32 representing the pin identifier
> and mux</dt>
> <dd>settings. The exact format of a pinmux group is left up to the
> driver.</dd>
> </dl>
>
> However, it should be something like
>
> <p><strong>pinmux_group</strong>: A u32 representing the pin identifier
> and mux settings. The exact format of a pinmux group is left up to the
> driver.</p>
>
> I have also tried an alternate style, as shown for @Return. However,
> this too generated the wrong html:
>
> <dl class="last docutils">
> <dt><strong>Return</strong>:</dt>
> <dd>Pin selector for the muxed pin if OK, or negative error code on
> failure</dd>
> </dl>
>
> where it should generate something like
>
> <p class="last"><strong>Return</strong>: Pin selector for the muxed pin
> if OK, or negative error code on failure</p>
>
> Do you have any suggestions for generating the latter forms? My current
> work is at [1].
>
> --Sean
>
> [1] https://github.com/Forty-Bot/u-boot/commit/e835cf9552dc96438699806731a208b40daead7b
>

[PATCH v5 02/11] pinctrl: Reformat documentation in dm/pinctrl.h

[Sean Anderson]

On 9/2/20 11:17 AM, Heinrich Schuchardt wrote:
> On 02.09.20 16:56, Sean Anderson wrote:
>>
>> On 9/2/20 8:26 AM, Heinrich Schuchardt wrote:
>>> On 15.08.20 17:52, Sean Anderson wrote:
>>>> This normalizes the documentatation to conform to kernel-doc style [1]. It
>>>
>>> Nits:
>>> %s/documentatation/documentation/
>>>
>>>> also moves the documentation for pinctrl_ops inline, and adds argument and
>>>> return-value documentation. I have kept the usual function style for these
>>>> comments. I could not find any existing examples of function documentation
>>>> inside structs.
>>>>
>>>> [1] https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>
>>>
>>> Before and after this patch the documentation format is incorrect. See
>>> output below for the situation after the patch.
>>>
>>> Please include include/dm/pinctrl.h into doc/api/index.rst via a new
>>> file doc/api/pnctrl.rst.
>>>
>>> A documentation style accepted by 'make htmldocs' is:
>>>
>>> struct pinctrl_ops {
>>>     /**
>>>      * @get_pins_count:           Get the number of selectable pins
>>>      *
>>>      * @get_pins_count.dev:       Pinctrl device to use
>>>      *
>>>      * This function is necessary to parse the "pins" property
>>>      * in DTS.
>>>      *
>>>      * @get_pins_count.Return:    number of selectable named pins
>>>      *                            available in this driver
>>>      */
>>>     int (*get_pins_count)(struct udevice *dev);
>>>
>>> See the nested structures in
>>>
>>> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#in-line-member-documentation-comments
>>> and
>>> https://www.kernel.org/doc/html/v5.7/doc-guide/kernel-doc.html#nested-structs-unions
>>
>> Thanks for suggesting those changes. This works better. As far as I can
>> tell, fully-qualified members are only necessary when documenting nested
>> structs at the top-level. When using in-line documentation, members are
>> automatically nested. With that in mind, I am using the following format
>>
>> 	/**
>> 	 * @pinmux_property_set: Enable a pinmux group
>> 	 *
>> 	 * @dev: Pinctrl device to use
>> 	 *
>> 	 * @pinmux_group: A u32 representing the pin identifier and mux
>> 	 *                settings. The exact format of a pinmux group is left
>> 	 *                up to the driver.
>> 	 *
>> 	 * Mux a single pin to a single function based on a driver-specific
>> 	 * pinmux group. This function is necessary for parsing the "pinmux"
>> 	 * property in DTS, and for pin-muxing against a pinmux group.
>> 	 *
>> 	 * @Return:
>> 	 *	Pin selector for the muxed pin if OK, or negative error code on
>> 	 *	failure
>> 	 */
>> 	int (*pinmux_property_set)(struct udevice *dev, u32 pinmux_group);
>>
>> However, I am having some problems with multi-line descriptions. In the
>> example above, the desciption for @pinmux_group has the first line
>> bolded and the rest of the description is typeset as normal, but is
>> indented. The generated html is
> 
> Use fully qualified names and you are fine.

I also experienced this issue when using fully-qualified names.

> 
> Best regards
> 
> Heinrich
> 
>>
>> <dl class="docutils">
>> <dt><strong>pinmux_group</strong>: A u32 representing the pin identifier
>> and mux</dt>
>> <dd>settings. The exact format of a pinmux group is left up to the
>> driver.</dd>
>> </dl>
>>
>> However, it should be something like
>>
>> <p><strong>pinmux_group</strong>: A u32 representing the pin identifier
>> and mux settings. The exact format of a pinmux group is left up to the
>> driver.</p>
>>
>> I have also tried an alternate style, as shown for @Return. However,
>> this too generated the wrong html:
>>
>> <dl class="last docutils">
>> <dt><strong>Return</strong>:</dt>
>> <dd>Pin selector for the muxed pin if OK, or negative error code on
>> failure</dd>
>> </dl>
>>
>> where it should generate something like
>>
>> <p class="last"><strong>Return</strong>: Pin selector for the muxed pin
>> if OK, or negative error code on failure</p>
>>
>> Do you have any suggestions for generating the latter forms? My current
>> work is at [1].
>>
>> --Sean
>>
>> [1] https://github.com/Forty-Bot/u-boot/commit/e835cf9552dc96438699806731a208b40daead7b
>>
> 

[PATCH v5 03/11] test: pinmux: Add test for pin muxing

[Sean Anderson]

This extends the pinctrl-sandbox driver to support pin muxing, and adds a
test for that behaviour. The test is done in C and not python (like the
existing tests for the pinctrl uclass) because it needs to call
pinctrl_select_state.  Another option could be to add a command that
invokes pinctrl_select_state and then test everything in
test/py/tests/test_pinmux.py.

The pinctrl-sandbox driver now mimics the way that many pinmux devices
work.  There are two groups of pins which are muxed together, as well as
four pins which are muxed individually. I have tried to test all normal
paths. However, very few error cases are explicitly checked for.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- Add sandbox dt-binding headers to MAINTAINERS
- Add test/dm/pinmux.c to patch
- Move sandbox_* variables into a priv structure. This resets them to the
  default state every time we re-probe.

Changes in v3:
- Add dt-bindings/pinctrl/sandbox-pinmux.h to patch

Changes in v2:
- New

 MAINTAINERS                                  |   1 +
 arch/sandbox/dts/test.dts                    |  45 ++++-
 drivers/pinctrl/pinctrl-sandbox.c            | 186 ++++++++++++++-----
 include/dt-bindings/pinctrl/sandbox-pinmux.h |  19 ++
 test/dm/Makefile                             |   3 +
 test/dm/pinmux.c                             |  57 ++++++
 test/py/tests/test_pinmux.py                 |  36 ++--
 7 files changed, 274 insertions(+), 73 deletions(-)
 create mode 100644 include/dt-bindings/pinctrl/sandbox-pinmux.h
 create mode 100644 test/dm/pinmux.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 17ac45587b..24de698433 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -953,6 +953,7 @@ SANDBOX
 M:	Simon Glass <sjg@chromium.org>
 S:	Maintained
 F:	arch/sandbox/
+F:	include/dt-bindings/*/sandbox*.h
 
 SH
 M:	Marek Vasut <marek.vasut+renesas@gmail.com>
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 491893a17d..0255c97cf5 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -2,6 +2,7 @@
 
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/gpio/sandbox-gpio.h>
+#include <dt-bindings/pinctrl/sandbox-pinmux.h>
 
 / {
 	model = "sandbox";
@@ -1024,30 +1025,60 @@
 	pinctrl {
 		compatible = "sandbox,pinctrl";
 
-		pinctrl-names = "default";
-		pinctrl-0 = <&gpios>;
+		pinctrl-names = "default", "alternate";
+		pinctrl-0 = <&pinctrl_gpios>, <&pinctrl_i2s>;
+		pinctrl-1 = <&pinctrl_spi>, <&pinctrl_i2c>;
 
-		gpios: gpios {
+		pinctrl_gpios: gpios {
 			gpio0 {
-				pins = "GPIO0";
+				pins = "P5";
+				function = "GPIO";
 				bias-pull-up;
 				input-disable;
 			};
 			gpio1 {
-				pins = "GPIO1";
+				pins = "P6";
+				function = "GPIO";
 				output-high;
 				drive-open-drain;
 			};
 			gpio2 {
-				pins = "GPIO2";
+				pinmux = <SANDBOX_PINMUX(7, SANDBOX_PINMUX_GPIO)>;
 				bias-pull-down;
 				input-enable;
 			};
 			gpio3 {
-				pins = "GPIO3";
+				pinmux = <SANDBOX_PINMUX(8, SANDBOX_PINMUX_GPIO)>;
 				bias-disable;
 			};
 		};
+
+		pinctrl_i2c: i2c {
+			groups {
+				groups = "I2C_UART";
+				function = "I2C";
+			};
+
+			pins {
+				pins = "P0", "P1";
+				drive-open-drain;
+			};
+		};
+
+		pinctrl_i2s: i2s {
+			groups = "SPI_I2S";
+			function = "I2S";
+		};
+
+		pinctrl_spi: spi {
+			groups = "SPI_I2S";
+			function = "SPI";
+
+			cs {
+				pinmux = <SANDBOX_PINMUX(5, SANDBOX_PINMUX_CS)>,
+					 <SANDBOX_PINMUX(6, SANDBOX_PINMUX_CS)>;
+			};
+		};
 	};
 
 	hwspinlock at 0 {
diff --git a/drivers/pinctrl/pinctrl-sandbox.c b/drivers/pinctrl/pinctrl-sandbox.c
index ac0119d198..d27f74248d 100644
--- a/drivers/pinctrl/pinctrl-sandbox.c
+++ b/drivers/pinctrl/pinctrl-sandbox.c
@@ -1,57 +1,70 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright (C) 2015  Masahiro Yamada <yamada.masahiro@socionext.com>
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ * Copyright (C) 2015 Masahiro Yamada <yamada.masahiro@socionext.com>
  */
 
-/* #define DEBUG */
-
 #include <common.h>
 #include <dm.h>
-#include <log.h>
 #include <dm/pinctrl.h>
+#include <dt-bindings/pinctrl/sandbox-pinmux.h>
+#include <log.h>
 #include <linux/bitops.h>
 
+/*
+ * This driver emulates a pin controller with the following rules:
+ * - The pinctrl config for each pin must be set individually
+ * - The first three pins (P0-P2) must be muxed as a group
+ * - The next two pins (P3-P4) must be muxed as a group
+ * - The last four pins (P5-P8) must be muxed individually
+ */
+
 static const char * const sandbox_pins[] = {
-	"SCL",
-	"SDA",
-	"TX",
-	"RX",
-	"W1",
-	"GPIO0",
-	"GPIO1",
-	"GPIO2",
-	"GPIO3",
+#define PIN(x) \
+	[x] = "P" #x
+	PIN(0),
+	PIN(1),
+	PIN(2),
+	PIN(3),
+	PIN(4),
+	PIN(5),
+	PIN(6),
+	PIN(7),
+	PIN(8),
+#undef PIN
 };
 
-static const char * const sandbox_pins_muxing[] = {
-	"I2C SCL",
-	"I2C SDA",
-	"Uart TX",
-	"Uart RX",
-	"1-wire gpio",
-	"gpio",
-	"gpio",
-	"gpio",
-	"gpio",
+static const char * const sandbox_pins_muxing[][2] = {
+	{ "UART TX", "I2C SCL" },
+	{ "UART RX", "I2C SDA" },
+	{ "SPI SCLK", "I2S SCK" },
+	{ "SPI MOSI", "I2S SD" },
+	{ "SPI MISO", "I2S WS" },
+	{ "GPIO0", "SPI CS0" },
+	{ "GPIO1", "SPI CS1" },
+	{ "GPIO2", "PWM0" },
+	{ "GPIO3", "PWM1" },
 };
 
+#define SANDBOX_GROUP_I2C_UART 0
+#define SANDBOX_GROUP_SPI_I2S 1
+
 static const char * const sandbox_groups[] = {
-	"i2c",
-	"serial_a",
-	"serial_b",
-	"spi",
-	"w1",
+	[SANDBOX_GROUP_I2C_UART] = "I2C_UART",
+	[SANDBOX_GROUP_SPI_I2S] = "SPI_I2S",
 };
 
 static const char * const sandbox_functions[] = {
-	"i2c",
-	"serial",
-	"spi",
-	"w1",
-	"gpio",
-	"gpio",
-	"gpio",
-	"gpio",
+#define FUNC(id) \
+	[SANDBOX_PINMUX_##id] = #id
+	FUNC(UART),
+	FUNC(I2C),
+	FUNC(SPI),
+	FUNC(I2S),
+	FUNC(GPIO),
+	FUNC(CS),
+	FUNC(PWM),
+#undef FUNC
 };
 
 static const struct pinconf_param sandbox_conf_params[] = {
@@ -68,9 +81,12 @@ static const struct pinconf_param sandbox_conf_params[] = {
 	{ "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 },
 };
 
-/* bitfield used to save param and value of each pin/selector */
-static unsigned int sandbox_pins_param[ARRAY_SIZE(sandbox_pins)];
-static unsigned int sandbox_pins_value[ARRAY_SIZE(sandbox_pins)];
+/* Bitfield used to save param and value of each pin/selector */
+struct sandbox_pinctrl_priv {
+	unsigned int mux;
+	unsigned int pins_param[ARRAY_SIZE(sandbox_pins)];
+	unsigned int pins_value[ARRAY_SIZE(sandbox_pins)];
+};
 
 static int sandbox_get_pins_count(struct udevice *dev)
 {
@@ -87,16 +103,18 @@ static int sandbox_get_pin_muxing(struct udevice *dev,
 				  char *buf, int size)
 {
 	const struct pinconf_param *p;
+	struct sandbox_pinctrl_priv *priv = dev_get_priv(dev);
 	int i;
 
-	snprintf(buf, size, "%s", sandbox_pins_muxing[selector]);
+	snprintf(buf, size, "%s",
+		 sandbox_pins_muxing[selector][!!(priv->mux & BIT(selector))]);
 
-	if (sandbox_pins_param[selector]) {
+	if (priv->pins_param[selector]) {
 		for (i = 0, p = sandbox_conf_params;
 		     i < ARRAY_SIZE(sandbox_conf_params);
 		     i++, p++) {
-			if ((sandbox_pins_param[selector] & BIT(p->param)) &&
-			    (!!(sandbox_pins_value[selector] & BIT(p->param)) ==
+			if ((priv->pins_param[selector] & BIT(p->param)) &&
+			    (!!(priv->pins_value[selector] & BIT(p->param)) ==
 			     p->default_value)) {
 				strncat(buf, " ", size);
 				strncat(buf, p->property, size);
@@ -133,12 +151,32 @@ static const char *sandbox_get_function_name(struct udevice *dev,
 static int sandbox_pinmux_set(struct udevice *dev, unsigned pin_selector,
 			      unsigned func_selector)
 {
+	int mux;
+	struct sandbox_pinctrl_priv *priv = dev_get_priv(dev);
+
 	debug("sandbox pinmux: pin = %d (%s), function = %d (%s)\n",
 	      pin_selector, sandbox_get_pin_name(dev, pin_selector),
 	      func_selector, sandbox_get_function_name(dev, func_selector));
 
-	sandbox_pins_param[pin_selector] = 0;
-	sandbox_pins_value[pin_selector] = 0;
+	if (pin_selector < 5)
+		return -EINVAL;
+
+	switch (func_selector) {
+	case SANDBOX_PINMUX_GPIO:
+		mux = 0;
+		break;
+	case SANDBOX_PINMUX_CS:
+	case SANDBOX_PINMUX_PWM:
+		mux = BIT(pin_selector);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	priv->mux &= ~BIT(pin_selector);
+	priv->mux |= mux;
+	priv->pins_param[pin_selector] = 0;
+	priv->pins_value[pin_selector] = 0;
 
 	return 0;
 }
@@ -147,25 +185,75 @@ static int sandbox_pinmux_group_set(struct udevice *dev,
 				    unsigned group_selector,
 				    unsigned func_selector)
 {
+	bool mux;
+	int i, group_start, group_end;
+	struct sandbox_pinctrl_priv *priv = dev_get_priv(dev);
+	unsigned int mask;
+
 	debug("sandbox pinmux: group = %d (%s), function = %d (%s)\n",
 	      group_selector, sandbox_get_group_name(dev, group_selector),
 	      func_selector, sandbox_get_function_name(dev, func_selector));
 
+	if (group_selector == SANDBOX_GROUP_I2C_UART) {
+		group_start = 0;
+		group_end = 1;
+
+		if (func_selector == SANDBOX_PINMUX_UART)
+			mux = false;
+		else if (func_selector == SANDBOX_PINMUX_I2C)
+			mux = true;
+		else
+			return -EINVAL;
+	} else if (group_selector == SANDBOX_GROUP_SPI_I2S) {
+		group_start = 2;
+		group_end = 4;
+
+		if (func_selector == SANDBOX_PINMUX_SPI)
+			mux = false;
+		else if (func_selector == SANDBOX_PINMUX_I2S)
+			mux = true;
+		else
+			return -EINVAL;
+	} else {
+		return -EINVAL;
+	}
+
+	mask = GENMASK(group_end, group_start);
+	priv->mux &= ~mask;
+	priv->mux |= mux ? mask : 0;
+
+	for (i = group_start; i < group_end; i++) {
+		priv->pins_param[i] = 0;
+		priv->pins_value[i] = 0;
+	}
+
 	return 0;
 }
 
+static int sandbox_pinmux_property_set(struct udevice *dev, u32 pinmux_group)
+{
+	int ret;
+	unsigned pin_selector = pinmux_group & 0xFFFF;
+	unsigned func_selector = pinmux_group >> 16;
+
+	ret = sandbox_pinmux_set(dev, pin_selector, func_selector);
+	return ret ? ret : pin_selector;
+}
+
 static int sandbox_pinconf_set(struct udevice *dev, unsigned pin_selector,
 			       unsigned param, unsigned argument)
 {
+	struct sandbox_pinctrl_priv *priv = dev_get_priv(dev);
+
 	debug("sandbox pinconf: pin = %d (%s), param = %d, arg = %d\n",
 	      pin_selector, sandbox_get_pin_name(dev, pin_selector),
 	      param, argument);
 
-	sandbox_pins_param[pin_selector] |= BIT(param);
+	priv->pins_param[pin_selector] |= BIT(param);
 	if (argument)
-		sandbox_pins_value[pin_selector] |= BIT(param);
+		priv->pins_value[pin_selector] |= BIT(param);
 	else
-		sandbox_pins_value[pin_selector] &= ~BIT(param);
+		priv->pins_value[pin_selector] &= ~BIT(param);
 
 	return 0;
 }
@@ -191,6 +279,7 @@ const struct pinctrl_ops sandbox_pinctrl_ops = {
 	.get_function_name = sandbox_get_function_name,
 	.pinmux_set = sandbox_pinmux_set,
 	.pinmux_group_set = sandbox_pinmux_group_set,
+	.pinmux_property_set = sandbox_pinmux_property_set,
 	.pinconf_num_params = ARRAY_SIZE(sandbox_conf_params),
 	.pinconf_params = sandbox_conf_params,
 	.pinconf_set = sandbox_pinconf_set,
@@ -207,5 +296,6 @@ U_BOOT_DRIVER(sandbox_pinctrl) = {
 	.name = "sandbox_pinctrl",
 	.id = UCLASS_PINCTRL,
 	.of_match = sandbox_pinctrl_match,
+	.priv_auto_alloc_size = sizeof(struct sandbox_pinctrl_priv),
 	.ops = &sandbox_pinctrl_ops,
 };
diff --git a/include/dt-bindings/pinctrl/sandbox-pinmux.h b/include/dt-bindings/pinctrl/sandbox-pinmux.h
new file mode 100644
index 0000000000..891af072e5
--- /dev/null
+++ b/include/dt-bindings/pinctrl/sandbox-pinmux.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ */
+
+#ifndef SANDBOX_PINMUX_H
+#define SANDBOX_PINMUX_H
+
+#define SANDBOX_PINMUX_UART 0
+#define SANDBOX_PINMUX_I2C  1
+#define SANDBOX_PINMUX_SPI  2
+#define SANDBOX_PINMUX_I2S  3
+#define SANDBOX_PINMUX_GPIO 4
+#define SANDBOX_PINMUX_CS   5
+#define SANDBOX_PINMUX_PWM  6
+
+#define SANDBOX_PINMUX(pin, func) ((func) << 16 | (pin))
+
+#endif /* SANDBOX_PINMUX_H */
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 864c8d0b4c..a5b8eac678 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -80,4 +80,7 @@ obj-$(CONFIG_DM_RNG) += rng.o
 obj-$(CONFIG_CLK_K210_SET_RATE) += k210_pll.o
 obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
 obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
+ifneq ($(CONFIG_PINMUX),)
+obj-$(CONFIG_PINCONF) += pinmux.o
+endif
 endif
diff --git a/test/dm/pinmux.c b/test/dm/pinmux.c
new file mode 100644
index 0000000000..e7607c9b25
--- /dev/null
+++ b/test/dm/pinmux.c
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <dm/pinctrl.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+static int dm_test_pinmux(struct unit_test_state *uts)
+{
+	char buf[64];
+	struct udevice *dev;
+
+#define test_muxing(selector, expected) do { \
+	ut_assertok(pinctrl_get_pin_muxing(dev, selector, buf, sizeof(buf))); \
+	ut_asserteq_str(expected, (char *)&buf); \
+} while (0)
+
+	ut_assertok(uclass_get_device_by_name(UCLASS_PINCTRL, "pinctrl", &dev));
+	test_muxing(0, "UART TX.");
+	test_muxing(1, "UART RX.");
+	test_muxing(2, "I2S SCK.");
+	test_muxing(3, "I2S SD.");
+	test_muxing(4, "I2S WS.");
+	test_muxing(5, "GPIO0 bias-pull-up input-disable.");
+	test_muxing(6, "GPIO1 drive-open-drain.");
+	test_muxing(7, "GPIO2 bias-pull-down input-enable.");
+	test_muxing(8, "GPIO3 bias-disable.");
+
+	ut_assertok(pinctrl_select_state(dev, "alternate"));
+	test_muxing(0, "I2C SCL drive-open-drain.");
+	test_muxing(1, "I2C SDA drive-open-drain.");
+	test_muxing(2, "SPI SCLK.");
+	test_muxing(3, "SPI MOSI.");
+	test_muxing(4, "SPI MISO.");
+	test_muxing(5, "SPI CS0.");
+	test_muxing(6, "SPI CS1.");
+	test_muxing(7, "GPIO2 bias-pull-down input-enable.");
+	test_muxing(8, "GPIO3 bias-disable.");
+
+	ut_assertok(pinctrl_select_state(dev, "0"));
+	test_muxing(0, "I2C SCL drive-open-drain.");
+	test_muxing(1, "I2C SDA drive-open-drain.");
+	test_muxing(2, "I2S SCK.");
+	test_muxing(3, "I2S SD.");
+	test_muxing(4, "I2S WS.");
+	test_muxing(5, "GPIO0 bias-pull-up input-disable.");
+	test_muxing(6, "GPIO1 drive-open-drain.");
+	test_muxing(7, "GPIO2 bias-pull-down input-enable.");
+	test_muxing(8, "GPIO3 bias-disable.");
+
+	return 0;
+}
+DM_TEST(dm_test_pinmux, DM_TESTF_SCAN_FDT);
diff --git a/test/py/tests/test_pinmux.py b/test/py/tests/test_pinmux.py
index 4e6df992a4..0cbbae000c 100644
--- a/test/py/tests/test_pinmux.py
+++ b/test/py/tests/test_pinmux.py
@@ -28,15 +28,15 @@ def test_pinmux_status_all(u_boot_console):
     assert ('a6        : gpio output .' in output)
 
     assert ('pinctrl:' in output)
-    assert ('SCL       : I2C SCL.' in output)
-    assert ('SDA       : I2C SDA.' in output)
-    assert ('TX        : Uart TX.' in output)
-    assert ('RX        : Uart RX.' in output)
-    assert ('W1        : 1-wire gpio.' in output)
-    assert ('GPIO0     : gpio bias-pull-up input-disable.' in output)
-    assert ('GPIO1     : gpio drive-open-drain.' in output)
-    assert ('GPIO2     : gpio bias-pull-down input-enable.' in output)
-    assert ('GPIO3     : gpio bias-disable.' in output)
+    assert ('P0        : UART TX.' in output)
+    assert ('P1        : UART RX.' in output)
+    assert ('P2        : I2S SCK.' in output)
+    assert ('P3        : I2S SD.' in output)
+    assert ('P4        : I2S WS.' in output)
+    assert ('P5        : GPIO0 bias-pull-up input-disable.' in output)
+    assert ('P6        : GPIO1 drive-open-drain.' in output)
+    assert ('P7        : GPIO2 bias-pull-down input-enable.' in output)
+    assert ('P8        : GPIO3 bias-disable.' in output)
 
 @pytest.mark.buildconfigspec('cmd_pinmux')
 @pytest.mark.boardspec('sandbox')
@@ -73,12 +73,12 @@ def test_pinmux_status(u_boot_console):
     assert (not 'pinctrl-gpio:' in output)
     assert (not 'pinctrl:' in output)
 
-    assert ('SCL       : I2C SCL.' in output)
-    assert ('SDA       : I2C SDA.' in output)
-    assert ('TX        : Uart TX.' in output)
-    assert ('RX        : Uart RX.' in output)
-    assert ('W1        : 1-wire gpio.' in output)
-    assert ('GPIO0     : gpio bias-pull-up input-disable.' in output)
-    assert ('GPIO1     : gpio drive-open-drain.' in output)
-    assert ('GPIO2     : gpio bias-pull-down input-enable.' in output)
-    assert ('GPIO3     : gpio bias-disable.' in output)
+    assert ('P0        : UART TX.' in output)
+    assert ('P1        : UART RX.' in output)
+    assert ('P2        : I2S SCK.' in output)
+    assert ('P3        : I2S SD.' in output)
+    assert ('P4        : I2S WS.' in output)
+    assert ('P5        : GPIO0 bias-pull-up input-disable.' in output)
+    assert ('P6        : GPIO1 drive-open-drain.' in output)
+    assert ('P7        : GPIO2 bias-pull-down input-enable.' in output)
+    assert ('P8        : GPIO3 bias-disable.' in output)
-- 
2.28.0

[PATCH v5 04/11] pinctrl: Add support for Kendryte K210 FPIOA

[Sean Anderson]

The Fully-Programmable Input/Output Array (FPIOA) device controls pin
multiplexing on the K210. The FPIOA can remap any supported function to any
multifunctional IO pin. It can also perform basic GPIO functions, such as
reading the current value of a pin. However, GPIO functionality remains
largely unimplemented (in favor of the dedicated GPIO peripherals).

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
This patch was previously submitted as part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

Changes from that version include:
- Reformat to reduce errors from checkpatch

(no changes since v2)

Changes in v2:
- Don't clear existing pinctrl settings on probe
- Rewrite to use pinmux property
- Support muxing the output enable signal for each function
- Support output and input inversion
- Update binding documentation

 MAINTAINERS                                   |   2 +
 .../pinctrl/kendryte,k210-fpioa.txt           | 102 +++
 drivers/pinctrl/Kconfig                       |   1 +
 drivers/pinctrl/Makefile                      |   1 +
 drivers/pinctrl/kendryte/Kconfig              |   7 +
 drivers/pinctrl/kendryte/Makefile             |   1 +
 drivers/pinctrl/kendryte/pinctrl.c            | 678 ++++++++++++++++++
 drivers/pinctrl/kendryte/pinctrl.h            |  70 ++
 include/dt-bindings/pinctrl/k210-pinctrl.h    | 277 +++++++
 9 files changed, 1139 insertions(+)
 create mode 100644 doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
 create mode 100644 drivers/pinctrl/kendryte/Kconfig
 create mode 100644 drivers/pinctrl/kendryte/Makefile
 create mode 100644 drivers/pinctrl/kendryte/pinctrl.c
 create mode 100644 drivers/pinctrl/kendryte/pinctrl.h
 create mode 100644 include/dt-bindings/pinctrl/k210-pinctrl.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 24de698433..3abe52570f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -929,7 +929,9 @@ RISC-V KENDRYTE
 M:	Sean Anderson <seanga2@gmail.com>
 S:	Maintained
 F:	doc/device-tree-bindings/mfd/kendryte,k210-sysctl.txt
+F:	doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
 F:	drivers/clk/kendryte/
+F:	drivers/pinctrl/kendryte/
 F:	include/kendryte/
 
 RNG
diff --git a/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt b/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
new file mode 100644
index 0000000000..06a9cc060f
--- /dev/null
+++ b/doc/device-tree-bindings/pinctrl/kendryte,k210-fpioa.txt
@@ -0,0 +1,102 @@
+Kendryte K210 FPIOA
+
+This binding describes the Fully-Programmable Input/Output Array (FPIOA) found
+in Kendryte K210 SoCs. Any of the 256 functions can be mapped to any of the 48
+pins.
+
+Required properties:
+- compatible: should be "kendryte,k210-fpioa"
+- reg: address and length of the FPIOA registers
+- kendryte,sysctl: phandle to the "sysctl" register map node
+- kendryte,power-offset: offset in the register map of the power bank control
+  register (in bytes)
+
+Configuration nodes
+
+Pin configuration nodes are documented in pinctrl-bindings.txt
+
+Required properties for pin-configuration nodes or sub-nodes are:
+- groups: list of power groups to which the configuration applies. Valid groups
+  are:
+	A0, A1, A2, B0, B1, B2, C0, C1
+  (either this or "pinmux" must be specified)
+- pinmux: integer array representing pin multiplexing configuration. In addition
+  to the 256 standard functions, each pin can also output the direction
+  indicator (DO) of any function. This signal is high whenever the function
+  would normally drive the output. Helper macros to ease assembling the "pinmux"
+  arguments from the pin and function are provided by the FPIOA header file at:
+  <dt-bindings/pinctrl/k210-pinctrl.h>
+  Integer values in the "pinmux" argument list are assembled as:
+  ((PIN << 16) | (DO << 8) | (FUNC))
+  Valid values for PIN are numbers 0 through 47.
+  Valid values for DO are 0 or 1.
+  Valid values for FUNC are numbers 0 through 255. For a complete list of
+  acceptable functions, consult the FPIOA header file.
+  (either this or "groups" must be specified)
+
+Optional properties for "pinmux" nodes are:
+	bias-disable, bias-pull-down, bias-pull-up, drive-strength,
+	drive-strength-ua, input-enable, input-disable, input-schmitt-enable,
+	input-schmitt-disable, output-low, output-high, output-enable,
+	output-disable, slew-rate, output-polarity-invert, input-polarity-invert
+
+Optional properties for "groups" nodes are:
+	power-source
+
+Notes on specific properties include:
+- bias-pull-up, -down, and -pin-default: The pull strength cannot be configured.
+- drive-strength: There are 8 drive strength settings between 11 and 50 mA.
+- input- and output-polarity-invert: Invert the polarity of either the input or
+  the output, respectively.
+- power-source: Controls the output voltage of a bank of pins. Either
+  K210_PC_POWER_1V8 or K210_PC_POWER_3V3 may be specified.
+- slew-rate: Specifying this property reduces the slew rate.
+
+Example:
+fpioa: pinmux@502B0000 {
+	compatible = "kendryte,k210-fpioa";
+	reg = <0x502B0000 0x100>;
+	kendryte,sysctl = <&sysctl>;
+	kendryte,power-offset = <K210_SYSCTL_POWER_SEL>;
+
+	/* JTAG running at 3.3V and driven at 11 mA */
+	fpioa_jtag: jtag {
+		voltage {
+			group = "A0";
+			power-source = <K210_PC_POWER_3V3>;
+		};
+
+		jtag {
+			pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCK)>,
+				 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+				 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+				 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+			drive-strength = <11>;
+		}
+	};
+
+	/* I2C configured for use with a TCA9800 level shifter */
+	fpioa_i2c: i2c {
+		i2c {
+			pinmux = <K210_FPIOA(6, K210_PCF_I2C0_SCLK)>,
+				 <K210_FPIOA(7, K210_PCF_I2C0_SDA)>;
+		};
+
+		direction {
+			pinmux = <K210_FPIOA_DO(8, K210_PCF_I2C0_SDA)>;
+			output-polarity-invert;
+		};
+	};
+
+	/* UART with an active-high TX status LED */
+	fpioa_uart1: uart1 {
+		uart {
+			pinmux = <K210_FPIOA(9, K210_PCF_UART1_TX)>,
+				 <K210_FPIOA(10, K210_PCF_UART1_RX)>;
+		};
+
+		status {
+			pinmux = <K210_FPIOA_DO(11, K210_PCF_UART1_TX)>;
+		};
+	};
+};
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
index cdbccfd285..7e208f573b 100644
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -289,6 +289,7 @@ endif
 source "drivers/pinctrl/broadcom/Kconfig"
 source "drivers/pinctrl/exynos/Kconfig"
 source "drivers/pinctrl/intel/Kconfig"
+source "drivers/pinctrl/kendryte/Kconfig"
 source "drivers/pinctrl/mediatek/Kconfig"
 source "drivers/pinctrl/meson/Kconfig"
 source "drivers/pinctrl/mscc/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
index 92cff1b100..4c2b4236d1 100644
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_PINCTRL_SANDBOX)	+= pinctrl-sandbox.o
 obj-$(CONFIG_PINCTRL_UNIPHIER)	+= uniphier/
 obj-$(CONFIG_PINCTRL_PIC32)	+= pinctrl_pic32.o
 obj-$(CONFIG_PINCTRL_EXYNOS)	+= exynos/
+obj-$(CONFIG_PINCTRL_K210)	+= kendryte/
 obj-$(CONFIG_PINCTRL_MESON)	+= meson/
 obj-$(CONFIG_PINCTRL_MTK)	+= mediatek/
 obj-$(CONFIG_PINCTRL_MSCC)	+= mscc/
diff --git a/drivers/pinctrl/kendryte/Kconfig b/drivers/pinctrl/kendryte/Kconfig
new file mode 100644
index 0000000000..6fc10e9516
--- /dev/null
+++ b/drivers/pinctrl/kendryte/Kconfig
@@ -0,0 +1,7 @@
+config PINCTRL_K210
+	bool "Kendryte K210 Fully-Programmable Input/Output Array driver"
+	depends on DM && PINCTRL_GENERIC
+	help
+	  Support pin multiplexing on the K210. The "FPIOA" can remap any
+	  supported function to any multifunctional IO pin. It can also perform
+	  basic GPIO functions, such as reading the current value of a pin.
diff --git a/drivers/pinctrl/kendryte/Makefile b/drivers/pinctrl/kendryte/Makefile
new file mode 100644
index 0000000000..1c10d7a929
--- /dev/null
+++ b/drivers/pinctrl/kendryte/Makefile
@@ -0,0 +1 @@
+obj-y += pinctrl.o
diff --git a/drivers/pinctrl/kendryte/pinctrl.c b/drivers/pinctrl/kendryte/pinctrl.c
new file mode 100644
index 0000000000..75c1dd3d02
--- /dev/null
+++ b/drivers/pinctrl/kendryte/pinctrl.c
@@ -0,0 +1,678 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ */
+
+#include <common.h>
+#include <clk.h>
+#include <dm.h>
+#include <dt-bindings/pinctrl/k210-pinctrl.h>
+#include <mapmem.h>
+#include <regmap.h>
+#include <syscon.h>
+#include <asm/io.h>
+#include <linux/err.h>
+#include <linux/bitfield.h>
+#include "pinctrl.h"
+
+struct k210_pc_priv {
+	struct clk clk;
+	struct k210_fpioa __iomem *fpioa; /* FPIOA register */
+	struct regmap *sysctl; /* Sysctl regmap */
+	u32 power_offset; /* Power bank register offset */
+};
+
+#ifdef CONFIG_CMD_PINMUX
+static const char k210_pc_pin_names[][6] = {
+#define PIN(i) \
+	[i] = "IO_" #i
+	PIN(0),
+	PIN(1),
+	PIN(2),
+	PIN(3),
+	PIN(4),
+	PIN(5),
+	PIN(6),
+	PIN(7),
+	PIN(8),
+	PIN(9),
+	PIN(10),
+	PIN(11),
+	PIN(12),
+	PIN(13),
+	PIN(14),
+	PIN(15),
+	PIN(16),
+	PIN(17),
+	PIN(18),
+	PIN(19),
+	PIN(20),
+	PIN(21),
+	PIN(22),
+	PIN(23),
+	PIN(24),
+	PIN(25),
+	PIN(26),
+	PIN(27),
+	PIN(28),
+	PIN(29),
+	PIN(30),
+	PIN(31),
+	PIN(32),
+	PIN(33),
+	PIN(34),
+	PIN(35),
+	PIN(36),
+	PIN(37),
+	PIN(38),
+	PIN(39),
+	PIN(40),
+	PIN(41),
+	PIN(42),
+	PIN(43),
+	PIN(44),
+	PIN(45),
+	PIN(46),
+	PIN(47),
+#undef PIN
+};
+
+static int k210_pc_get_pins_count(struct udevice *dev)
+{
+	return ARRAY_SIZE(k210_pc_pin_names);
+};
+
+static const char *k210_pc_get_pin_name(struct udevice *dev, unsigned selector)
+{
+	return k210_pc_pin_names[selector];
+}
+#endif /* CONFIG_CMD_PINMUX */
+
+/* These are just power domains */
+static const char k210_pc_group_names[][3] = {
+	[0] = "A0",
+	[1] = "A1",
+	[2] = "A2",
+	[3] = "B0",
+	[4] = "B1",
+	[5] = "B2",
+	[6] = "C0",
+	[7] = "C1",
+};
+
+static int k210_pc_get_groups_count(struct udevice *dev)
+{
+	return ARRAY_SIZE(k210_pc_group_names);
+}
+
+static const char *k210_pc_get_group_name(struct udevice *dev,
+					  unsigned selector)
+{
+	return k210_pc_group_names[selector];
+}
+
+enum k210_pc_mode_id {
+	K210_PC_DEFAULT_DISABLED,
+	K210_PC_DEFAULT_IN,
+	K210_PC_DEFAULT_IN_TIE,
+	K210_PC_DEFAULT_OUT,
+	K210_PC_DEFAULT_I2C,
+	K210_PC_DEFAULT_SPI,
+	K210_PC_DEFAULT_GPIO,
+	K210_PC_DEFAULT_INT13,
+};
+
+static const u32 k210_pc_mode_id_to_mode[] = {
+#define DEFAULT(mode) \
+	[K210_PC_DEFAULT_##mode] = K210_PC_MODE_##mode
+	[K210_PC_DEFAULT_DISABLED] = 0,
+	DEFAULT(IN),
+	[K210_PC_DEFAULT_IN_TIE] = K210_PC_MODE_IN,
+	DEFAULT(OUT),
+	DEFAULT(I2C),
+	DEFAULT(SPI),
+	DEFAULT(GPIO),
+	[K210_PC_DEFAULT_INT13] = K210_PC_MODE_IN | K210_PC_PU,
+#undef DEFAULT
+};
+
+/* This saves around 2K vs having a pointer+mode */
+struct k210_pcf_info {
+#ifdef CONFIG_CMD_PINMUX
+	char name[15];
+#endif
+	u8 mode_id;
+};
+
+static const struct k210_pcf_info k210_pcf_infos[] = {
+#ifdef CONFIG_CMD_PINMUX
+#define FUNC(id, mode) \
+	[K210_PCF_##id] = { \
+		.name = #id, \
+		.mode_id = K210_PC_DEFAULT_##mode \
+	}
+#else
+#define FUNC(id, mode) \
+	[K210_PCF_##id] = { \
+		.mode_id = K210_PC_DEFAULT_##mode \
+	}
+#endif
+	FUNC(JTAG_TCLK,      IN),
+	FUNC(JTAG_TDI,       IN),
+	FUNC(JTAG_TMS,       IN),
+	FUNC(JTAG_TDO,       OUT),
+	FUNC(SPI0_D0,        SPI),
+	FUNC(SPI0_D1,        SPI),
+	FUNC(SPI0_D2,        SPI),
+	FUNC(SPI0_D3,        SPI),
+	FUNC(SPI0_D4,        SPI),
+	FUNC(SPI0_D5,        SPI),
+	FUNC(SPI0_D6,        SPI),
+	FUNC(SPI0_D7,        SPI),
+	FUNC(SPI0_SS0,       OUT),
+	FUNC(SPI0_SS1,       OUT),
+	FUNC(SPI0_SS2,       OUT),
+	FUNC(SPI0_SS3,       OUT),
+	FUNC(SPI0_ARB,       IN_TIE),
+	FUNC(SPI0_SCLK,      OUT),
+	FUNC(UARTHS_RX,      IN),
+	FUNC(UARTHS_TX,      OUT),
+	FUNC(RESV6,          IN),
+	FUNC(RESV7,          IN),
+	FUNC(CLK_SPI1,       OUT),
+	FUNC(CLK_I2C1,       OUT),
+	FUNC(GPIOHS0,        GPIO),
+	FUNC(GPIOHS1,        GPIO),
+	FUNC(GPIOHS2,        GPIO),
+	FUNC(GPIOHS3,        GPIO),
+	FUNC(GPIOHS4,        GPIO),
+	FUNC(GPIOHS5,        GPIO),
+	FUNC(GPIOHS6,        GPIO),
+	FUNC(GPIOHS7,        GPIO),
+	FUNC(GPIOHS8,        GPIO),
+	FUNC(GPIOHS9,        GPIO),
+	FUNC(GPIOHS10,       GPIO),
+	FUNC(GPIOHS11,       GPIO),
+	FUNC(GPIOHS12,       GPIO),
+	FUNC(GPIOHS13,       GPIO),
+	FUNC(GPIOHS14,       GPIO),
+	FUNC(GPIOHS15,       GPIO),
+	FUNC(GPIOHS16,       GPIO),
+	FUNC(GPIOHS17,       GPIO),
+	FUNC(GPIOHS18,       GPIO),
+	FUNC(GPIOHS19,       GPIO),
+	FUNC(GPIOHS20,       GPIO),
+	FUNC(GPIOHS21,       GPIO),
+	FUNC(GPIOHS22,       GPIO),
+	FUNC(GPIOHS23,       GPIO),
+	FUNC(GPIOHS24,       GPIO),
+	FUNC(GPIOHS25,       GPIO),
+	FUNC(GPIOHS26,       GPIO),
+	FUNC(GPIOHS27,       GPIO),
+	FUNC(GPIOHS28,       GPIO),
+	FUNC(GPIOHS29,       GPIO),
+	FUNC(GPIOHS30,       GPIO),
+	FUNC(GPIOHS31,       GPIO),
+	FUNC(GPIO0,          GPIO),
+	FUNC(GPIO1,          GPIO),
+	FUNC(GPIO2,          GPIO),
+	FUNC(GPIO3,          GPIO),
+	FUNC(GPIO4,          GPIO),
+	FUNC(GPIO5,          GPIO),
+	FUNC(GPIO6,          GPIO),
+	FUNC(GPIO7,          GPIO),
+	FUNC(UART1_RX,       IN),
+	FUNC(UART1_TX,       OUT),
+	FUNC(UART2_RX,       IN),
+	FUNC(UART2_TX,       OUT),
+	FUNC(UART3_RX,       IN),
+	FUNC(UART3_TX,       OUT),
+	FUNC(SPI1_D0,        SPI),
+	FUNC(SPI1_D1,        SPI),
+	FUNC(SPI1_D2,        SPI),
+	FUNC(SPI1_D3,        SPI),
+	FUNC(SPI1_D4,        SPI),
+	FUNC(SPI1_D5,        SPI),
+	FUNC(SPI1_D6,        SPI),
+	FUNC(SPI1_D7,        SPI),
+	FUNC(SPI1_SS0,       OUT),
+	FUNC(SPI1_SS1,       OUT),
+	FUNC(SPI1_SS2,       OUT),
+	FUNC(SPI1_SS3,       OUT),
+	FUNC(SPI1_ARB,       IN_TIE),
+	FUNC(SPI1_SCLK,      OUT),
+	FUNC(SPI2_D0,        SPI),
+	FUNC(SPI2_SS,        IN),
+	FUNC(SPI2_SCLK,      IN),
+	FUNC(I2S0_MCLK,      OUT),
+	FUNC(I2S0_SCLK,      OUT),
+	FUNC(I2S0_WS,        OUT),
+	FUNC(I2S0_IN_D0,     IN),
+	FUNC(I2S0_IN_D1,     IN),
+	FUNC(I2S0_IN_D2,     IN),
+	FUNC(I2S0_IN_D3,     IN),
+	FUNC(I2S0_OUT_D0,    OUT),
+	FUNC(I2S0_OUT_D1,    OUT),
+	FUNC(I2S0_OUT_D2,    OUT),
+	FUNC(I2S0_OUT_D3,    OUT),
+	FUNC(I2S1_MCLK,      OUT),
+	FUNC(I2S1_SCLK,      OUT),
+	FUNC(I2S1_WS,        OUT),
+	FUNC(I2S1_IN_D0,     IN),
+	FUNC(I2S1_IN_D1,     IN),
+	FUNC(I2S1_IN_D2,     IN),
+	FUNC(I2S1_IN_D3,     IN),
+	FUNC(I2S1_OUT_D0,    OUT),
+	FUNC(I2S1_OUT_D1,    OUT),
+	FUNC(I2S1_OUT_D2,    OUT),
+	FUNC(I2S1_OUT_D3,    OUT),
+	FUNC(I2S2_MCLK,      OUT),
+	FUNC(I2S2_SCLK,      OUT),
+	FUNC(I2S2_WS,        OUT),
+	FUNC(I2S2_IN_D0,     IN),
+	FUNC(I2S2_IN_D1,     IN),
+	FUNC(I2S2_IN_D2,     IN),
+	FUNC(I2S2_IN_D3,     IN),
+	FUNC(I2S2_OUT_D0,    OUT),
+	FUNC(I2S2_OUT_D1,    OUT),
+	FUNC(I2S2_OUT_D2,    OUT),
+	FUNC(I2S2_OUT_D3,    OUT),
+	FUNC(RESV0,          DISABLED),
+	FUNC(RESV1,          DISABLED),
+	FUNC(RESV2,          DISABLED),
+	FUNC(RESV3,          DISABLED),
+	FUNC(RESV4,          DISABLED),
+	FUNC(RESV5,          DISABLED),
+	FUNC(I2C0_SCLK,      I2C),
+	FUNC(I2C0_SDA,       I2C),
+	FUNC(I2C1_SCLK,      I2C),
+	FUNC(I2C1_SDA,       I2C),
+	FUNC(I2C2_SCLK,      I2C),
+	FUNC(I2C2_SDA,       I2C),
+	FUNC(DVP_XCLK,       OUT),
+	FUNC(DVP_RST,        OUT),
+	FUNC(DVP_PWDN,       OUT),
+	FUNC(DVP_VSYNC,      IN),
+	FUNC(DVP_HSYNC,      IN),
+	FUNC(DVP_PCLK,       IN),
+	FUNC(DVP_D0,         IN),
+	FUNC(DVP_D1,         IN),
+	FUNC(DVP_D2,         IN),
+	FUNC(DVP_D3,         IN),
+	FUNC(DVP_D4,         IN),
+	FUNC(DVP_D5,         IN),
+	FUNC(DVP_D6,         IN),
+	FUNC(DVP_D7,         IN),
+	FUNC(SCCB_SCLK,      I2C),
+	FUNC(SCCB_SDA,       I2C),
+	FUNC(UART1_CTS,      IN),
+	FUNC(UART1_DSR,      IN),
+	FUNC(UART1_DCD,      IN),
+	FUNC(UART1_RI,       IN),
+	FUNC(UART1_SIR_IN,   IN),
+	FUNC(UART1_DTR,      OUT),
+	FUNC(UART1_RTS,      OUT),
+	FUNC(UART1_OUT2,     OUT),
+	FUNC(UART1_OUT1,     OUT),
+	FUNC(UART1_SIR_OUT,  OUT),
+	FUNC(UART1_BAUD,     OUT),
+	FUNC(UART1_RE,       OUT),
+	FUNC(UART1_DE,       OUT),
+	FUNC(UART1_RS485_EN, OUT),
+	FUNC(UART2_CTS,      IN),
+	FUNC(UART2_DSR,      IN),
+	FUNC(UART2_DCD,      IN),
+	FUNC(UART2_RI,       IN),
+	FUNC(UART2_SIR_IN,   IN),
+	FUNC(UART2_DTR,      OUT),
+	FUNC(UART2_RTS,      OUT),
+	FUNC(UART2_OUT2,     OUT),
+	FUNC(UART2_OUT1,     OUT),
+	FUNC(UART2_SIR_OUT,  OUT),
+	FUNC(UART2_BAUD,     OUT),
+	FUNC(UART2_RE,       OUT),
+	FUNC(UART2_DE,       OUT),
+	FUNC(UART2_RS485_EN, OUT),
+	FUNC(UART3_CTS,      IN),
+	FUNC(UART3_DSR,      IN),
+	FUNC(UART3_DCD,      IN),
+	FUNC(UART3_RI,       IN),
+	FUNC(UART3_SIR_IN,   IN),
+	FUNC(UART3_DTR,      OUT),
+	FUNC(UART3_RTS,      OUT),
+	FUNC(UART3_OUT2,     OUT),
+	FUNC(UART3_OUT1,     OUT),
+	FUNC(UART3_SIR_OUT,  OUT),
+	FUNC(UART3_BAUD,     OUT),
+	FUNC(UART3_RE,       OUT),
+	FUNC(UART3_DE,       OUT),
+	FUNC(UART3_RS485_EN, OUT),
+	FUNC(TIMER0_TOGGLE1, OUT),
+	FUNC(TIMER0_TOGGLE2, OUT),
+	FUNC(TIMER0_TOGGLE3, OUT),
+	FUNC(TIMER0_TOGGLE4, OUT),
+	FUNC(TIMER1_TOGGLE1, OUT),
+	FUNC(TIMER1_TOGGLE2, OUT),
+	FUNC(TIMER1_TOGGLE3, OUT),
+	FUNC(TIMER1_TOGGLE4, OUT),
+	FUNC(TIMER2_TOGGLE1, OUT),
+	FUNC(TIMER2_TOGGLE2, OUT),
+	FUNC(TIMER2_TOGGLE3, OUT),
+	FUNC(TIMER2_TOGGLE4, OUT),
+	FUNC(CLK_SPI2,       OUT),
+	FUNC(CLK_I2C2,       OUT),
+	FUNC(INTERNAL0,      OUT),
+	FUNC(INTERNAL1,      OUT),
+	FUNC(INTERNAL2,      OUT),
+	FUNC(INTERNAL3,      OUT),
+	FUNC(INTERNAL4,      OUT),
+	FUNC(INTERNAL5,      OUT),
+	FUNC(INTERNAL6,      OUT),
+	FUNC(INTERNAL7,      OUT),
+	FUNC(INTERNAL8,      OUT),
+	FUNC(INTERNAL9,      IN),
+	FUNC(INTERNAL10,     IN),
+	FUNC(INTERNAL11,     IN),
+	FUNC(INTERNAL12,     IN),
+	FUNC(INTERNAL13,     INT13),
+	FUNC(INTERNAL14,     I2C),
+	FUNC(INTERNAL15,     IN),
+	FUNC(INTERNAL16,     IN),
+	FUNC(INTERNAL17,     IN),
+	FUNC(CONSTANT,       DISABLED),
+	FUNC(INTERNAL18,     IN),
+	FUNC(DEBUG0,         OUT),
+	FUNC(DEBUG1,         OUT),
+	FUNC(DEBUG2,         OUT),
+	FUNC(DEBUG3,         OUT),
+	FUNC(DEBUG4,         OUT),
+	FUNC(DEBUG5,         OUT),
+	FUNC(DEBUG6,         OUT),
+	FUNC(DEBUG7,         OUT),
+	FUNC(DEBUG8,         OUT),
+	FUNC(DEBUG9,         OUT),
+	FUNC(DEBUG10,        OUT),
+	FUNC(DEBUG11,        OUT),
+	FUNC(DEBUG12,        OUT),
+	FUNC(DEBUG13,        OUT),
+	FUNC(DEBUG14,        OUT),
+	FUNC(DEBUG15,        OUT),
+	FUNC(DEBUG16,        OUT),
+	FUNC(DEBUG17,        OUT),
+	FUNC(DEBUG18,        OUT),
+	FUNC(DEBUG19,        OUT),
+	FUNC(DEBUG20,        OUT),
+	FUNC(DEBUG21,        OUT),
+	FUNC(DEBUG22,        OUT),
+	FUNC(DEBUG23,        OUT),
+	FUNC(DEBUG24,        OUT),
+	FUNC(DEBUG25,        OUT),
+	FUNC(DEBUG26,        OUT),
+	FUNC(DEBUG27,        OUT),
+	FUNC(DEBUG28,        OUT),
+	FUNC(DEBUG29,        OUT),
+	FUNC(DEBUG30,        OUT),
+	FUNC(DEBUG31,        OUT),
+#undef FUNC
+};
+
+static int k210_pc_pinmux_set(struct udevice *dev, u32 pinmux_group)
+{
+	unsigned pin = FIELD_GET(K210_PG_PIN, pinmux_group);
+	bool do_oe = FIELD_GET(K210_PG_DO, pinmux_group);
+	unsigned func = FIELD_GET(K210_PG_FUNC, pinmux_group);
+	struct k210_pc_priv *priv = dev_get_priv(dev);
+	const struct k210_pcf_info *info = &k210_pcf_infos[func];
+	u32 mode = k210_pc_mode_id_to_mode[info->mode_id];
+	u32 val = func | mode | (do_oe ? K210_PC_DO_OE : 0);
+
+	debug("%s(%.8x): IO_%.2u = %3u | %.8x\n", __func__, pinmux_group, pin,
+	      func, mode);
+
+	writel(val, &priv->fpioa->pins[pin]);
+	return pin;
+}
+
+/* Max drive strength in uA */
+static const int k210_pc_drive_strength[] = {
+	[0] = 11200,
+	[1] = 16800,
+	[2] = 22300,
+	[3] = 27800,
+	[4] = 33300,
+	[5] = 38700,
+	[6] = 44100,
+	[7] = 49500,
+};
+
+static int k210_pc_get_drive(unsigned max_strength_ua)
+{
+	int i;
+
+	for (i = K210_PC_DRIVE_MAX; i; i--)
+		if (k210_pc_drive_strength[i] < max_strength_ua)
+			return i;
+
+	return -EINVAL;
+}
+
+static int k210_pc_pinconf_set(struct udevice *dev, unsigned pin_selector,
+			       unsigned param, unsigned argument)
+{
+	struct k210_pc_priv *priv = dev_get_priv(dev);
+	u32 val = readl(&priv->fpioa->pins[pin_selector]);
+
+	switch (param) {
+	case PIN_CONFIG_BIAS_DISABLE:
+		val &= ~K210_PC_BIAS_MASK;
+		break;
+	case PIN_CONFIG_BIAS_PULL_DOWN:
+		if (argument)
+			val |= K210_PC_PD;
+		else
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_BIAS_PULL_UP:
+		if (argument)
+			val |= K210_PC_PD;
+		else
+			return -EINVAL;
+		break;
+	case PIN_CONFIG_DRIVE_STRENGTH:
+		argument *= 1000;
+	case PIN_CONFIG_DRIVE_STRENGTH_UA: {
+		int drive = k210_pc_get_drive(argument);
+
+		if (IS_ERR_VALUE(drive))
+			return drive;
+		val &= ~K210_PC_DRIVE_MASK;
+		val |= FIELD_PREP(K210_PC_DRIVE_MASK, drive);
+		break;
+	}
+	case PIN_CONFIG_INPUT_ENABLE:
+		if (argument)
+			val |= K210_PC_IE;
+		else
+			val &= ~K210_PC_IE;
+		break;
+	case PIN_CONFIG_INPUT_SCHMITT:
+		argument = 1;
+	case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+		if (argument)
+			val |= K210_PC_ST;
+		else
+			val &= ~K210_PC_ST;
+		break;
+	case PIN_CONFIG_OUTPUT:
+		k210_pc_pinmux_set(dev,
+				   K210_FPIOA(pin_selector, K210_PCF_CONSTANT));
+		val = readl(&priv->fpioa->pins[pin_selector]);
+		val |= K210_PC_MODE_OUT;
+
+		if (!argument)
+			val |= K210_PC_DO_INV;
+		break;
+	case PIN_CONFIG_OUTPUT_ENABLE:
+		if (argument)
+			val |= K210_PC_OE;
+		else
+			val &= ~K210_PC_OE;
+		break;
+	case PIN_CONFIG_SLEW_RATE:
+		if (argument)
+			val |= K210_PC_SL;
+		else
+			val &= ~K210_PC_SL;
+		break;
+	case PIN_CONFIG_OUTPUT_INVERT:
+		if (argument)
+			val |= K210_PC_DO_INV;
+		else
+			val &= ~K210_PC_DO_INV;
+		break;
+	case PIN_CONFIG_INPUT_INVERT:
+		if (argument)
+			val |= K210_PC_DI_INV;
+		else
+			val &= ~K210_PC_DI_INV;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	writel(val, &priv->fpioa->pins[pin_selector]);
+	return 0;
+}
+
+static int k210_pc_pinconf_group_set(struct udevice *dev,
+				     unsigned group_selector, unsigned param,
+				     unsigned argument)
+{
+	struct k210_pc_priv *priv = dev_get_priv(dev);
+
+	if (param == PIN_CONFIG_POWER_SOURCE) {
+		u32 bit = BIT(group_selector);
+
+		regmap_update_bits(priv->sysctl, priv->power_offset, bit,
+				   argument ? bit : 0);
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_CMD_PINMUX
+static int k210_pc_get_pin_muxing(struct udevice *dev, unsigned int selector,
+				  char *buf, int size)
+{
+	struct k210_pc_priv *priv = dev_get_priv(dev);
+	u32 val = readl(&priv->fpioa->pins[selector]);
+	const struct k210_pcf_info *info = &k210_pcf_infos[val & K210_PCF_MASK];
+
+	strncpy(buf, info->name, min((size_t)size, sizeof(info->name)));
+	return 0;
+}
+#endif
+
+static const struct pinconf_param k210_pc_pinconf_params[] = {
+	{ "bias-disable", PIN_CONFIG_BIAS_DISABLE, 0 },
+	{ "bias-pull-down", PIN_CONFIG_BIAS_PULL_DOWN, 1 },
+	{ "bias-pull-up", PIN_CONFIG_BIAS_PULL_UP, 1 },
+	{ "drive-strength", PIN_CONFIG_DRIVE_STRENGTH, U32_MAX },
+	{ "drive-strength-ua", PIN_CONFIG_DRIVE_STRENGTH_UA, U32_MAX },
+	{ "input-enable", PIN_CONFIG_INPUT_ENABLE, 1 },
+	{ "input-disable", PIN_CONFIG_INPUT_ENABLE, 0 },
+	{ "input-schmitt-enable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 1 },
+	{ "input-schmitt-disable", PIN_CONFIG_INPUT_SCHMITT_ENABLE, 0 },
+	{ "power-source", PIN_CONFIG_POWER_SOURCE, K210_PC_POWER_1V8 },
+	{ "output-low", PIN_CONFIG_OUTPUT, 0 },
+	{ "output-high", PIN_CONFIG_OUTPUT, 1 },
+	{ "output-enable", PIN_CONFIG_OUTPUT_ENABLE, 1 },
+	{ "output-disable", PIN_CONFIG_OUTPUT_ENABLE, 0 },
+	{ "slew-rate", PIN_CONFIG_SLEW_RATE, 1 },
+	{ "output-polarity-invert", PIN_CONFIG_OUTPUT_INVERT, 1},
+	{ "input-polarity-invert", PIN_CONFIG_INPUT_INVERT, 1},
+};
+
+static const struct pinctrl_ops k210_pc_pinctrl_ops = {
+#ifdef CONFIG_CMD_PINMUX
+	.get_pins_count = k210_pc_get_pins_count,
+	.get_pin_name = k210_pc_get_pin_name,
+#endif
+	.get_groups_count = k210_pc_get_groups_count,
+	.get_group_name = k210_pc_get_group_name,
+	.pinmux_property_set = k210_pc_pinmux_set,
+	.pinconf_num_params = ARRAY_SIZE(k210_pc_pinconf_params),
+	.pinconf_params = k210_pc_pinconf_params,
+	.pinconf_set = k210_pc_pinconf_set,
+	.pinconf_group_set = k210_pc_pinconf_group_set,
+	.set_state = pinctrl_generic_set_state,
+#ifdef CONFIG_CMD_PINMUX
+	.get_pin_muxing = k210_pc_get_pin_muxing,
+#endif
+};
+
+static int k210_pc_probe(struct udevice *dev)
+{
+	int ret, i, j;
+	struct k210_pc_priv *priv = dev_get_priv(dev);
+
+	priv->fpioa = dev_read_addr_ptr(dev);
+	if (!priv->fpioa)
+		return -EINVAL;
+
+	ret = clk_get_by_index(dev, 0, &priv->clk);
+	if (ret)
+		return ret;
+
+	ret = clk_enable(&priv->clk);
+	if (ret && ret != -ENOSYS && ret != -ENOTSUPP)
+		goto err;
+
+	priv->sysctl = syscon_regmap_lookup_by_phandle(dev, "kendryte,sysctl");
+	if (IS_ERR(priv->sysctl)) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	ret = dev_read_u32(dev, "kendryte,power-offset", &priv->power_offset);
+	if (ret)
+		goto err;
+
+	debug("%s: fpioa = %p sysctl = %p power offset = %x\n", __func__,
+	      priv->fpioa, (void *)priv->sysctl->ranges[0].start,
+	      priv->power_offset);
+
+	/* Init input ties */
+	for (i = 0; i < ARRAY_SIZE(priv->fpioa->tie_en); i++) {
+		u32 val = 0;
+
+		for (j = 0; j < 32; j++)
+			if (k210_pcf_infos[i * 32 + j].mode_id ==
+			    K210_PC_DEFAULT_IN_TIE)
+				val |= BIT(j);
+		writel(val, &priv->fpioa->tie_en[i]);
+		writel(val, &priv->fpioa->tie_val[i]);
+	}
+
+	return 0;
+
+err:
+	clk_free(&priv->clk);
+	return ret;
+}
+
+static const struct udevice_id k210_pc_ids[] = {
+	{ .compatible = "kendryte,k210-fpioa" },
+	{ }
+};
+
+U_BOOT_DRIVER(pinctrl_k210) = {
+	.name = "pinctrl_k210",
+	.id = UCLASS_PINCTRL,
+	.of_match = k210_pc_ids,
+	.probe = k210_pc_probe,
+	.priv_auto_alloc_size = sizeof(struct k210_pc_priv),
+	.ops = &k210_pc_pinctrl_ops,
+};
diff --git a/drivers/pinctrl/kendryte/pinctrl.h b/drivers/pinctrl/kendryte/pinctrl.h
new file mode 100644
index 0000000000..ad76001ec5
--- /dev/null
+++ b/drivers/pinctrl/kendryte/pinctrl.h
@@ -0,0 +1,70 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ */
+
+#ifndef K210_PINCTRL_H
+#define K210_PINCTRL_H
+
+#include <dm/pinctrl.h>
+#include <linux/bitops.h>
+
+/*
+ * The K210 only implements 8 drive levels, even though there is register space
+ * for 16
+ */
+#define K210_PC_DRIVE_MASK GENMASK(11, 8)
+#define K210_PC_DRIVE_SHIFT 8
+#define K210_PC_DRIVE_0 (0 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_1 (1 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_2 (2 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_3 (3 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_4 (4 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_5 (5 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_6 (6 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_7 (7 << K210_PC_DRIVE_SHIFT)
+#define K210_PC_DRIVE_MAX 7
+
+#define K210_PC_MODE_MASK GENMASK(23, 12)
+/*
+ * output enabled == PC_OE & (PC_OE_INV ^ FUNCTION_OE) where FUNCTION_OE is a
+ * physical signal from the function
+ */
+#define K210_PC_OE       BIT(12) /* Output Enable */
+#define K210_PC_OE_INV   BIT(13) /* INVert function-controlled Output Enable */
+#define K210_PC_DO_OE    BIT(14) /* set Data Out to the Output Enable signal */
+#define K210_PC_DO_INV   BIT(15) /* INVert final Data Output */
+#define K210_PC_PU       BIT(16) /* Pull Up */
+#define K210_PC_PD       BIT(17) /* Pull Down */
+/* Strong pull up not implemented on K210 */
+#define K210_PC_SL       BIT(19) /* reduce SLew rate to prevent overshoot */
+/* Same semantics as OE above */
+#define K210_PC_IE       BIT(20) /* Input Enable */
+#define K210_PC_IE_INV   BIT(21) /* INVert function-controlled Input Enable */
+#define K210_PC_DI_INV   BIT(22) /* INVert Data Input */
+#define K210_PC_ST       BIT(23) /* Schmitt Trigger */
+#define K210_PC_DI       BIT(31) /* raw Data Input */
+#define K210_PC_BIAS_MASK (K210_PC_PU & K210_PC_PD)
+
+#define K210_PC_MODE_IN   (K210_PC_IE | K210_PC_ST)
+#define K210_PC_MODE_OUT  (K210_PC_DRIVE_7 | K210_PC_OE)
+#define K210_PC_MODE_I2C  (K210_PC_MODE_IN | K210_PC_IE_INV | K210_PC_SL | \
+			   K210_PC_OE | K210_PC_OE_INV | K210_PC_PU)
+#define K210_PC_MODE_SPI  (K210_PC_MODE_IN | K210_PC_IE_INV | \
+			   K210_PC_MODE_OUT | K210_PC_OE_INV)
+#define K210_PC_MODE_GPIO (K210_PC_MODE_IN | K210_PC_MODE_OUT)
+
+#define K210_PG_FUNC GENMASK(7, 0)
+#define K210_PG_DO BIT(8)
+#define K210_PG_PIN GENMASK(22, 16)
+
+#define PIN_CONFIG_OUTPUT_INVERT (PIN_CONFIG_END + 1)
+#define PIN_CONFIG_INPUT_INVERT (PIN_CONFIG_END + 2)
+
+struct k210_fpioa {
+	u32 pins[48];
+	u32 tie_en[8];
+	u32 tie_val[8];
+};
+
+#endif /* K210_PINCTRL_H */
diff --git a/include/dt-bindings/pinctrl/k210-pinctrl.h b/include/dt-bindings/pinctrl/k210-pinctrl.h
new file mode 100644
index 0000000000..26c1f23b0f
--- /dev/null
+++ b/include/dt-bindings/pinctrl/k210-pinctrl.h
@@ -0,0 +1,277 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2020 Sean Anderson <seanga2@gmail.com>
+ */
+
+#ifndef DT_K210_PINCTRL_H
+#define DT_K210_PINCTRL_H
+
+/*
+ * Full list of FPIOA functions from
+ * kendryte-standalone-sdk/lib/drivers/include/fpioa.h
+ */
+#define K210_PCF_MASK GENMASK(7, 0)
+#define K210_PCF_JTAG_TCLK      0   /* JTAG Test Clock */
+#define K210_PCF_JTAG_TDI       1   /* JTAG Test Data In */
+#define K210_PCF_JTAG_TMS       2   /* JTAG Test Mode Select */
+#define K210_PCF_JTAG_TDO       3   /* JTAG Test Data Out */
+#define K210_PCF_SPI0_D0        4   /* SPI0 Data 0 */
+#define K210_PCF_SPI0_D1        5   /* SPI0 Data 1 */
+#define K210_PCF_SPI0_D2        6   /* SPI0 Data 2 */
+#define K210_PCF_SPI0_D3        7   /* SPI0 Data 3 */
+#define K210_PCF_SPI0_D4        8   /* SPI0 Data 4 */
+#define K210_PCF_SPI0_D5        9   /* SPI0 Data 5 */
+#define K210_PCF_SPI0_D6        10  /* SPI0 Data 6 */
+#define K210_PCF_SPI0_D7        11  /* SPI0 Data 7 */
+#define K210_PCF_SPI0_SS0       12  /* SPI0 Chip Select 0 */
+#define K210_PCF_SPI0_SS1       13  /* SPI0 Chip Select 1 */
+#define K210_PCF_SPI0_SS2       14  /* SPI0 Chip Select 2 */
+#define K210_PCF_SPI0_SS3       15  /* SPI0 Chip Select 3 */
+#define K210_PCF_SPI0_ARB       16  /* SPI0 Arbitration */
+#define K210_PCF_SPI0_SCLK      17  /* SPI0 Serial Clock */
+#define K210_PCF_UARTHS_RX      18  /* UART High speed Receiver */
+#define K210_PCF_UARTHS_TX      19  /* UART High speed Transmitter */
+#define K210_PCF_RESV6          20  /* Reserved function */
+#define K210_PCF_RESV7          21  /* Reserved function */
+#define K210_PCF_CLK_SPI1       22  /* Clock SPI1 */
+#define K210_PCF_CLK_I2C1       23  /* Clock I2C1 */
+#define K210_PCF_GPIOHS0        24  /* GPIO High speed 0 */
+#define K210_PCF_GPIOHS1        25  /* GPIO High speed 1 */
+#define K210_PCF_GPIOHS2        26  /* GPIO High speed 2 */
+#define K210_PCF_GPIOHS3        27  /* GPIO High speed 3 */
+#define K210_PCF_GPIOHS4        28  /* GPIO High speed 4 */
+#define K210_PCF_GPIOHS5        29  /* GPIO High speed 5 */
+#define K210_PCF_GPIOHS6        30  /* GPIO High speed 6 */
+#define K210_PCF_GPIOHS7        31  /* GPIO High speed 7 */
+#define K210_PCF_GPIOHS8        32  /* GPIO High speed 8 */
+#define K210_PCF_GPIOHS9        33  /* GPIO High speed 9 */
+#define K210_PCF_GPIOHS10       34  /* GPIO High speed 10 */
+#define K210_PCF_GPIOHS11       35  /* GPIO High speed 11 */
+#define K210_PCF_GPIOHS12       36  /* GPIO High speed 12 */
+#define K210_PCF_GPIOHS13       37  /* GPIO High speed 13 */
+#define K210_PCF_GPIOHS14       38  /* GPIO High speed 14 */
+#define K210_PCF_GPIOHS15       39  /* GPIO High speed 15 */
+#define K210_PCF_GPIOHS16       40  /* GPIO High speed 16 */
+#define K210_PCF_GPIOHS17       41  /* GPIO High speed 17 */
+#define K210_PCF_GPIOHS18       42  /* GPIO High speed 18 */
+#define K210_PCF_GPIOHS19       43  /* GPIO High speed 19 */
+#define K210_PCF_GPIOHS20       44  /* GPIO High speed 20 */
+#define K210_PCF_GPIOHS21       45  /* GPIO High speed 21 */
+#define K210_PCF_GPIOHS22       46  /* GPIO High speed 22 */
+#define K210_PCF_GPIOHS23       47  /* GPIO High speed 23 */
+#define K210_PCF_GPIOHS24       48  /* GPIO High speed 24 */
+#define K210_PCF_GPIOHS25       49  /* GPIO High speed 25 */
+#define K210_PCF_GPIOHS26       50  /* GPIO High speed 26 */
+#define K210_PCF_GPIOHS27       51  /* GPIO High speed 27 */
+#define K210_PCF_GPIOHS28       52  /* GPIO High speed 28 */
+#define K210_PCF_GPIOHS29       53  /* GPIO High speed 29 */
+#define K210_PCF_GPIOHS30       54  /* GPIO High speed 30 */
+#define K210_PCF_GPIOHS31       55  /* GPIO High speed 31 */
+#define K210_PCF_GPIO0          56  /* GPIO pin 0 */
+#define K210_PCF_GPIO1          57  /* GPIO pin 1 */
+#define K210_PCF_GPIO2          58  /* GPIO pin 2 */
+#define K210_PCF_GPIO3          59  /* GPIO pin 3 */
+#define K210_PCF_GPIO4          60  /* GPIO pin 4 */
+#define K210_PCF_GPIO5          61  /* GPIO pin 5 */
+#define K210_PCF_GPIO6          62  /* GPIO pin 6 */
+#define K210_PCF_GPIO7          63  /* GPIO pin 7 */
+#define K210_PCF_UART1_RX       64  /* UART1 Receiver */
+#define K210_PCF_UART1_TX       65  /* UART1 Transmitter */
+#define K210_PCF_UART2_RX       66  /* UART2 Receiver */
+#define K210_PCF_UART2_TX       67  /* UART2 Transmitter */
+#define K210_PCF_UART3_RX       68  /* UART3 Receiver */
+#define K210_PCF_UART3_TX       69  /* UART3 Transmitter */
+#define K210_PCF_SPI1_D0        70  /* SPI1 Data 0 */
+#define K210_PCF_SPI1_D1        71  /* SPI1 Data 1 */
+#define K210_PCF_SPI1_D2        72  /* SPI1 Data 2 */
+#define K210_PCF_SPI1_D3        73  /* SPI1 Data 3 */
+#define K210_PCF_SPI1_D4        74  /* SPI1 Data 4 */
+#define K210_PCF_SPI1_D5        75  /* SPI1 Data 5 */
+#define K210_PCF_SPI1_D6        76  /* SPI1 Data 6 */
+#define K210_PCF_SPI1_D7        77  /* SPI1 Data 7 */
+#define K210_PCF_SPI1_SS0       78  /* SPI1 Chip Select 0 */
+#define K210_PCF_SPI1_SS1       79  /* SPI1 Chip Select 1 */
+#define K210_PCF_SPI1_SS2       80  /* SPI1 Chip Select 2 */
+#define K210_PCF_SPI1_SS3       81  /* SPI1 Chip Select 3 */
+#define K210_PCF_SPI1_ARB       82  /* SPI1 Arbitration */
+#define K210_PCF_SPI1_SCLK      83  /* SPI1 Serial Clock */
+#define K210_PCF_SPI2_D0        84  /* SPI2 Data 0 */
+#define K210_PCF_SPI2_SS        85  /* SPI2 Select */
+#define K210_PCF_SPI2_SCLK      86  /* SPI2 Serial Clock */
+#define K210_PCF_I2S0_MCLK      87  /* I2S0 Master Clock */
+#define K210_PCF_I2S0_SCLK      88  /* I2S0 Serial Clock(BCLK) */
+#define K210_PCF_I2S0_WS        89  /* I2S0 Word Select(LRCLK) */
+#define K210_PCF_I2S0_IN_D0     90  /* I2S0 Serial Data Input 0 */
+#define K210_PCF_I2S0_IN_D1     91  /* I2S0 Serial Data Input 1 */
+#define K210_PCF_I2S0_IN_D2     92  /* I2S0 Serial Data Input 2 */
+#define K210_PCF_I2S0_IN_D3     93  /* I2S0 Serial Data Input 3 */
+#define K210_PCF_I2S0_OUT_D0    94  /* I2S0 Serial Data Output 0 */
+#define K210_PCF_I2S0_OUT_D1    95  /* I2S0 Serial Data Output 1 */
+#define K210_PCF_I2S0_OUT_D2    96  /* I2S0 Serial Data Output 2 */
+#define K210_PCF_I2S0_OUT_D3    97  /* I2S0 Serial Data Output 3 */
+#define K210_PCF_I2S1_MCLK      98  /* I2S1 Master Clock */
+#define K210_PCF_I2S1_SCLK      99  /* I2S1 Serial Clock(BCLK) */
+#define K210_PCF_I2S1_WS        100 /* I2S1 Word Select(LRCLK) */
+#define K210_PCF_I2S1_IN_D0     101 /* I2S1 Serial Data Input 0 */
+#define K210_PCF_I2S1_IN_D1     102 /* I2S1 Serial Data Input 1 */
+#define K210_PCF_I2S1_IN_D2     103 /* I2S1 Serial Data Input 2 */
+#define K210_PCF_I2S1_IN_D3     104 /* I2S1 Serial Data Input 3 */
+#define K210_PCF_I2S1_OUT_D0    105 /* I2S1 Serial Data Output 0 */
+#define K210_PCF_I2S1_OUT_D1    106 /* I2S1 Serial Data Output 1 */
+#define K210_PCF_I2S1_OUT_D2    107 /* I2S1 Serial Data Output 2 */
+#define K210_PCF_I2S1_OUT_D3    108 /* I2S1 Serial Data Output 3 */
+#define K210_PCF_I2S2_MCLK      109 /* I2S2 Master Clock */
+#define K210_PCF_I2S2_SCLK      110 /* I2S2 Serial Clock(BCLK) */
+#define K210_PCF_I2S2_WS        111 /* I2S2 Word Select(LRCLK) */
+#define K210_PCF_I2S2_IN_D0     112 /* I2S2 Serial Data Input 0 */
+#define K210_PCF_I2S2_IN_D1     113 /* I2S2 Serial Data Input 1 */
+#define K210_PCF_I2S2_IN_D2     114 /* I2S2 Serial Data Input 2 */
+#define K210_PCF_I2S2_IN_D3     115 /* I2S2 Serial Data Input 3 */
+#define K210_PCF_I2S2_OUT_D0    116 /* I2S2 Serial Data Output 0 */
+#define K210_PCF_I2S2_OUT_D1    117 /* I2S2 Serial Data Output 1 */
+#define K210_PCF_I2S2_OUT_D2    118 /* I2S2 Serial Data Output 2 */
+#define K210_PCF_I2S2_OUT_D3    119 /* I2S2 Serial Data Output 3 */
+#define K210_PCF_RESV0          120 /* Reserved function */
+#define K210_PCF_RESV1          121 /* Reserved function */
+#define K210_PCF_RESV2          122 /* Reserved function */
+#define K210_PCF_RESV3          123 /* Reserved function */
+#define K210_PCF_RESV4          124 /* Reserved function */
+#define K210_PCF_RESV5          125 /* Reserved function */
+#define K210_PCF_I2C0_SCLK      126 /* I2C0 Serial Clock */
+#define K210_PCF_I2C0_SDA       127 /* I2C0 Serial Data */
+#define K210_PCF_I2C1_SCLK      128 /* I2C1 Serial Clock */
+#define K210_PCF_I2C1_SDA       129 /* I2C1 Serial Data */
+#define K210_PCF_I2C2_SCLK      130 /* I2C2 Serial Clock */
+#define K210_PCF_I2C2_SDA       131 /* I2C2 Serial Data */
+#define K210_PCF_DVP_XCLK       132 /* DVP System Clock */
+#define K210_PCF_DVP_RST        133 /* DVP System Reset */
+#define K210_PCF_DVP_PWDN       134 /* DVP Power Down Mode */
+#define K210_PCF_DVP_VSYNC      135 /* DVP Vertical Sync */
+#define K210_PCF_DVP_HSYNC      136 /* DVP Horizontal Sync */
+#define K210_PCF_DVP_PCLK       137 /* Pixel Clock */
+#define K210_PCF_DVP_D0         138 /* Data Bit 0 */
+#define K210_PCF_DVP_D1         139 /* Data Bit 1 */
+#define K210_PCF_DVP_D2         140 /* Data Bit 2 */
+#define K210_PCF_DVP_D3         141 /* Data Bit 3 */
+#define K210_PCF_DVP_D4         142 /* Data Bit 4 */
+#define K210_PCF_DVP_D5         143 /* Data Bit 5 */
+#define K210_PCF_DVP_D6         144 /* Data Bit 6 */
+#define K210_PCF_DVP_D7         145 /* Data Bit 7 */
+#define K210_PCF_SCCB_SCLK      146 /* Serial Camera Control Bus Clock */
+#define K210_PCF_SCCB_SDA       147 /* Serial Camera Control Bus Data */
+#define K210_PCF_UART1_CTS      148 /* UART1 Clear To Send */
+#define K210_PCF_UART1_DSR      149 /* UART1 Data Set Ready */
+#define K210_PCF_UART1_DCD      150 /* UART1 Data Carrier Detect */
+#define K210_PCF_UART1_RI       151 /* UART1 Ring Indicator */
+#define K210_PCF_UART1_SIR_IN   152 /* UART1 Serial Infrared Input */
+#define K210_PCF_UART1_DTR      153 /* UART1 Data Terminal Ready */
+#define K210_PCF_UART1_RTS      154 /* UART1 Request To Send */
+#define K210_PCF_UART1_OUT2     155 /* UART1 User-designated Output 2 */
+#define K210_PCF_UART1_OUT1     156 /* UART1 User-designated Output 1 */
+#define K210_PCF_UART1_SIR_OUT  157 /* UART1 Serial Infrared Output */
+#define K210_PCF_UART1_BAUD     158 /* UART1 Transmit Clock Output */
+#define K210_PCF_UART1_RE       159 /* UART1 Receiver Output Enable */
+#define K210_PCF_UART1_DE       160 /* UART1 Driver Output Enable */
+#define K210_PCF_UART1_RS485_EN 161 /* UART1 RS485 Enable */
+#define K210_PCF_UART2_CTS      162 /* UART2 Clear To Send */
+#define K210_PCF_UART2_DSR      163 /* UART2 Data Set Ready */
+#define K210_PCF_UART2_DCD      164 /* UART2 Data Carrier Detect */
+#define K210_PCF_UART2_RI       165 /* UART2 Ring Indicator */
+#define K210_PCF_UART2_SIR_IN   166 /* UART2 Serial Infrared Input */
+#define K210_PCF_UART2_DTR      167 /* UART2 Data Terminal Ready */
+#define K210_PCF_UART2_RTS      168 /* UART2 Request To Send */
+#define K210_PCF_UART2_OUT2     169 /* UART2 User-designated Output 2 */
+#define K210_PCF_UART2_OUT1     170 /* UART2 User-designated Output 1 */
+#define K210_PCF_UART2_SIR_OUT  171 /* UART2 Serial Infrared Output */
+#define K210_PCF_UART2_BAUD     172 /* UART2 Transmit Clock Output */
+#define K210_PCF_UART2_RE       173 /* UART2 Receiver Output Enable */
+#define K210_PCF_UART2_DE       174 /* UART2 Driver Output Enable */
+#define K210_PCF_UART2_RS485_EN 175 /* UART2 RS485 Enable */
+#define K210_PCF_UART3_CTS      176 /* UART3 Clear To Send */
+#define K210_PCF_UART3_DSR      177 /* UART3 Data Set Ready */
+#define K210_PCF_UART3_DCD      178 /* UART3 Data Carrier Detect */
+#define K210_PCF_UART3_RI       179 /* UART3 Ring Indicator */
+#define K210_PCF_UART3_SIR_IN   180 /* UART3 Serial Infrared Input */
+#define K210_PCF_UART3_DTR      181 /* UART3 Data Terminal Ready */
+#define K210_PCF_UART3_RTS      182 /* UART3 Request To Send */
+#define K210_PCF_UART3_OUT2     183 /* UART3 User-designated Output 2 */
+#define K210_PCF_UART3_OUT1     184 /* UART3 User-designated Output 1 */
+#define K210_PCF_UART3_SIR_OUT  185 /* UART3 Serial Infrared Output */
+#define K210_PCF_UART3_BAUD     186 /* UART3 Transmit Clock Output */
+#define K210_PCF_UART3_RE       187 /* UART3 Receiver Output Enable */
+#define K210_PCF_UART3_DE       188 /* UART3 Driver Output Enable */
+#define K210_PCF_UART3_RS485_EN 189 /* UART3 RS485 Enable */
+#define K210_PCF_TIMER0_TOGGLE1 190 /* TIMER0 Toggle Output 1 */
+#define K210_PCF_TIMER0_TOGGLE2 191 /* TIMER0 Toggle Output 2 */
+#define K210_PCF_TIMER0_TOGGLE3 192 /* TIMER0 Toggle Output 3 */
+#define K210_PCF_TIMER0_TOGGLE4 193 /* TIMER0 Toggle Output 4 */
+#define K210_PCF_TIMER1_TOGGLE1 194 /* TIMER1 Toggle Output 1 */
+#define K210_PCF_TIMER1_TOGGLE2 195 /* TIMER1 Toggle Output 2 */
+#define K210_PCF_TIMER1_TOGGLE3 196 /* TIMER1 Toggle Output 3 */
+#define K210_PCF_TIMER1_TOGGLE4 197 /* TIMER1 Toggle Output 4 */
+#define K210_PCF_TIMER2_TOGGLE1 198 /* TIMER2 Toggle Output 1 */
+#define K210_PCF_TIMER2_TOGGLE2 199 /* TIMER2 Toggle Output 2 */
+#define K210_PCF_TIMER2_TOGGLE3 200 /* TIMER2 Toggle Output 3 */
+#define K210_PCF_TIMER2_TOGGLE4 201 /* TIMER2 Toggle Output 4 */
+#define K210_PCF_CLK_SPI2       202 /* Clock SPI2 */
+#define K210_PCF_CLK_I2C2       203 /* Clock I2C2 */
+#define K210_PCF_INTERNAL0      204 /* Internal function signal 0 */
+#define K210_PCF_INTERNAL1      205 /* Internal function signal 1 */
+#define K210_PCF_INTERNAL2      206 /* Internal function signal 2 */
+#define K210_PCF_INTERNAL3      207 /* Internal function signal 3 */
+#define K210_PCF_INTERNAL4      208 /* Internal function signal 4 */
+#define K210_PCF_INTERNAL5      209 /* Internal function signal 5 */
+#define K210_PCF_INTERNAL6      210 /* Internal function signal 6 */
+#define K210_PCF_INTERNAL7      211 /* Internal function signal 7 */
+#define K210_PCF_INTERNAL8      212 /* Internal function signal 8 */
+#define K210_PCF_INTERNAL9      213 /* Internal function signal 9 */
+#define K210_PCF_INTERNAL10     214 /* Internal function signal 10 */
+#define K210_PCF_INTERNAL11     215 /* Internal function signal 11 */
+#define K210_PCF_INTERNAL12     216 /* Internal function signal 12 */
+#define K210_PCF_INTERNAL13     217 /* Internal function signal 13 */
+#define K210_PCF_INTERNAL14     218 /* Internal function signal 14 */
+#define K210_PCF_INTERNAL15     219 /* Internal function signal 15 */
+#define K210_PCF_INTERNAL16     220 /* Internal function signal 16 */
+#define K210_PCF_INTERNAL17     221 /* Internal function signal 17 */
+#define K210_PCF_CONSTANT       222 /* Constant function */
+#define K210_PCF_INTERNAL18     223 /* Internal function signal 18 */
+#define K210_PCF_DEBUG0         224 /* Debug function 0 */
+#define K210_PCF_DEBUG1         225 /* Debug function 1 */
+#define K210_PCF_DEBUG2         226 /* Debug function 2 */
+#define K210_PCF_DEBUG3         227 /* Debug function 3 */
+#define K210_PCF_DEBUG4         228 /* Debug function 4 */
+#define K210_PCF_DEBUG5         229 /* Debug function 5 */
+#define K210_PCF_DEBUG6         230 /* Debug function 6 */
+#define K210_PCF_DEBUG7         231 /* Debug function 7 */
+#define K210_PCF_DEBUG8         232 /* Debug function 8 */
+#define K210_PCF_DEBUG9         233 /* Debug function 9 */
+#define K210_PCF_DEBUG10        234 /* Debug function 10 */
+#define K210_PCF_DEBUG11        235 /* Debug function 11 */
+#define K210_PCF_DEBUG12        236 /* Debug function 12 */
+#define K210_PCF_DEBUG13        237 /* Debug function 13 */
+#define K210_PCF_DEBUG14        238 /* Debug function 14 */
+#define K210_PCF_DEBUG15        239 /* Debug function 15 */
+#define K210_PCF_DEBUG16        240 /* Debug function 16 */
+#define K210_PCF_DEBUG17        241 /* Debug function 17 */
+#define K210_PCF_DEBUG18        242 /* Debug function 18 */
+#define K210_PCF_DEBUG19        243 /* Debug function 19 */
+#define K210_PCF_DEBUG20        244 /* Debug function 20 */
+#define K210_PCF_DEBUG21        245 /* Debug function 21 */
+#define K210_PCF_DEBUG22        246 /* Debug function 22 */
+#define K210_PCF_DEBUG23        247 /* Debug function 23 */
+#define K210_PCF_DEBUG24        248 /* Debug function 24 */
+#define K210_PCF_DEBUG25        249 /* Debug function 25 */
+#define K210_PCF_DEBUG26        250 /* Debug function 26 */
+#define K210_PCF_DEBUG27        251 /* Debug function 27 */
+#define K210_PCF_DEBUG28        252 /* Debug function 28 */
+#define K210_PCF_DEBUG29        253 /* Debug function 29 */
+#define K210_PCF_DEBUG30        254 /* Debug function 30 */
+#define K210_PCF_DEBUG31        255 /* Debug function 31 */
+
+#define K210_FPIOA(pin, func) (((pin) << 16) | (func))
+#define K210_FPIOA_DO(pin, func) (((pin) << 16) | (1 << 8) | (func))
+
+#define K210_PC_POWER_3V3 0
+#define K210_PC_POWER_1V8 1
+
+#endif /* DT_K210_PINCTRL_H */
-- 
2.28.0

[PATCH v5 05/11] gpio: dw: Fix warnings about casting int to pointer

[Sean Anderson]

Change the type of gpio_dwabp_platdata.base from fdt_addr_t to a void
pointer, since we pass it to readl.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
This patch was previously submitted as part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

(no changes since v1)

 drivers/gpio/dwapb_gpio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
index e5e3518194..bf324f5239 100644
--- a/drivers/gpio/dwapb_gpio.c
+++ b/drivers/gpio/dwapb_gpio.c
@@ -40,7 +40,7 @@ struct gpio_dwapb_platdata {
 	const char	*name;
 	int		bank;
 	int		pins;
-	fdt_addr_t	base;
+	void __iomem	*base;
 };
 
 static int dwapb_gpio_direction_input(struct udevice *dev, unsigned pin)
@@ -176,7 +176,7 @@ static int gpio_dwapb_bind(struct udevice *dev)
 		if (!plat)
 			return -ENOMEM;
 
-		plat->base = base;
+		plat->base = (void *)base;
 		plat->bank = bank;
 		plat->pins = ofnode_read_u32_default(node, "snps,nr-gpios", 0);
 
-- 
2.28.0

[PATCH v5 06/11] gpio: dw: Add a trailing underscore to generated name

[Sean Anderson]

Previously, if there was no bank-name property, it was easy to have
confusing gpio names like "gpio1 at 08", instead of "gpio1 at 0_8". This patch
follows the example of the sifive gpio driver.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
This patch was previously submitted as part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

(no changes since v1)

 drivers/gpio/dwapb_gpio.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
index bf324f5239..a52c9e18e3 100644
--- a/drivers/gpio/dwapb_gpio.c
+++ b/drivers/gpio/dwapb_gpio.c
@@ -186,7 +186,15 @@ static int gpio_dwapb_bind(struct udevice *dev)
 			 * Fall back to node name. This means accessing pins
 			 * via bank name won't work.
 			 */
-			plat->name = ofnode_get_name(node);
+			char name[32];
+
+			snprintf(name, sizeof(name), "%s_",
+				 ofnode_get_name(node));
+			plat->name = strdup(name);
+			if (!plat->name) {
+				kfree(plat);
+				return -ENOMEM;
+			}
 		}
 
 		ret = device_bind_ofnode(dev, dev->driver, plat->name,
-- 
2.28.0

[PATCH v5 07/11] gpio: dw: Return output value when direction is out

[Sean Anderson]

dm_gpio_ops.get_value can be called when the gpio is either input or
output. The current dw code always returns the input value, which is
invalid if the direction is set to out.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
This patch was previously submitted as part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

(no changes since v3)

Changes in v3:
- Undo reorder to prevent use-before-declared error

Changes in v2:
- Reorder changes to minimize diff

 drivers/gpio/dwapb_gpio.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpio/dwapb_gpio.c b/drivers/gpio/dwapb_gpio.c
index a52c9e18e3..37916e7771 100644
--- a/drivers/gpio/dwapb_gpio.c
+++ b/drivers/gpio/dwapb_gpio.c
@@ -66,13 +66,6 @@ static int dwapb_gpio_direction_output(struct udevice *dev, unsigned pin,
 	return 0;
 }
 
-static int dwapb_gpio_get_value(struct udevice *dev, unsigned pin)
-{
-	struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
-	return !!(readl(plat->base + GPIO_EXT_PORT(plat->bank)) & (1 << pin));
-}
-
-
 static int dwapb_gpio_set_value(struct udevice *dev, unsigned pin, int val)
 {
 	struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
@@ -98,6 +91,18 @@ static int dwapb_gpio_get_function(struct udevice *dev, unsigned offset)
 		return GPIOF_INPUT;
 }
 
+static int dwapb_gpio_get_value(struct udevice *dev, unsigned pin)
+{
+	struct gpio_dwapb_platdata *plat = dev_get_platdata(dev);
+	u32 value;
+
+	if (dwapb_gpio_get_function(dev, pin) == GPIOF_OUTPUT)
+		value = readl(plat->base + GPIO_SWPORT_DR(plat->bank));
+	else
+		value = readl(plat->base + GPIO_EXT_PORT(plat->bank));
+	return !!(value & BIT(pin));
+}
+
 static const struct dm_gpio_ops gpio_dwapb_ops = {
 	.direction_input	= dwapb_gpio_direction_input,
 	.direction_output	= dwapb_gpio_direction_output,
-- 
2.28.0

[PATCH v5 08/11] led: gpio: Default to using node name if label is absent

[Sean Anderson]

This more closely mirrors Linux's behaviour, and will make it easier to
transition to using function+color in the future.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
This patch was previously submitted as part of
https://patchwork.ozlabs.org/project/uboot/list/?series=161576

(no changes since v1)

 drivers/led/led_gpio.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/led/led_gpio.c b/drivers/led/led_gpio.c
index ef9b61ee62..2cdb0269f4 100644
--- a/drivers/led/led_gpio.c
+++ b/drivers/led/led_gpio.c
@@ -99,11 +99,8 @@ static int led_gpio_bind(struct udevice *parent)
 		const char *label;
 
 		label = ofnode_read_string(node, "label");
-		if (!label) {
-			debug("%s: node %s has no label\n", __func__,
-			      ofnode_get_name(node));
-			return -EINVAL;
-		}
+		if (!label)
+			label = ofnode_get_name(node);
 		ret = device_bind_driver_to_node(parent, "gpio_led",
 						 ofnode_get_name(node),
 						 node, &dev);
-- 
2.28.0

[PATCH v5 09/11] test: dm: Test for default led naming

[Sean Anderson]

This modifies the existing led test to check for default led naming as
added in the previous patch.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v4)

Changes in v4:
- New

 arch/sandbox/dts/test.dts | 2 +-
 test/dm/led.c             | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 0255c97cf5..a0a57143f0 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -503,7 +503,7 @@
 
 		default_off {
 			gpios = <&gpio_a 6 0>;
-			label = "sandbox:default_off";
+			/* label intentionally omitted */
 			default-state = "off";
 		};
 	};
diff --git a/test/dm/led.c b/test/dm/led.c
index 8b587d0a22..ac6ee36394 100644
--- a/test/dm/led.c
+++ b/test/dm/led.c
@@ -40,7 +40,8 @@ static int dm_test_led_default_state(struct unit_test_state *uts)
 	ut_assertok(led_get_by_label("sandbox:default_on", &dev));
 	ut_asserteq(LEDST_ON, led_get_state(dev));
 
-	ut_assertok(led_get_by_label("sandbox:default_off", &dev));
+	/* Also tests default label behaviour */
+	ut_assertok(led_get_by_label("default_off", &dev));
 	ut_asserteq(LEDST_OFF, led_get_state(dev));
 
 	return 0;
-- 
2.28.0

[PATCH v5 10/11] riscv: Add pinmux and gpio bindings for Kendryte K210

[Sean Anderson]

This patch adds the necessary device tree bindings.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---

(no changes since v2)

Changes in v2:
- Convert to use pinmux property
- Don't hog ISP on boot
- Re-order GPIOs to match the defaults more closely

 arch/riscv/dts/k210-maix-bit.dts | 104 +++++++++++++++++++++++++++++++
 arch/riscv/dts/k210.dtsi         |  12 ++++
 2 files changed, 116 insertions(+)

diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
index 5b32c5fd5f..e840e04ada 100644
--- a/arch/riscv/dts/k210-maix-bit.dts
+++ b/arch/riscv/dts/k210-maix-bit.dts
@@ -17,6 +17,22 @@
 		stdout-path = "serial0:115200";
 	};
 
+	gpio-leds {
+		compatible = "gpio-leds";
+
+		green {
+			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
+		};
+
+		red {
+			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
+		};
+
+		blue {
+			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
+		};
+	};
+
 	sound {
 		compatible = "simple-audio-card";
 		simple-audio-card,format = "i2s";
@@ -39,9 +55,97 @@
 };
 
 &uarths0 {
+	pinctrl-0 = <&fpioa_uarths>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio0 {
+	pinctrl-0 = <&fpioa_gpiohs>;
+	pinctrl-names = "default";
+	status = "okay";
+};
+
+&gpio1 {
+	pinctrl-0 = <&fpioa_gpio>;
+	pinctrl-names = "default";
 	status = "okay";
 };
 
 &i2s0 {
 	#sound-dai-cells = <1>;
+	pinctrl-0 = <&fpioa_i2s0>;
+	pinctrl-names = "default";
+};
+
+&fpioa {
+	status = "okay";
+
+	fpioa_uarths: uarths {
+		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
+			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
+	};
+
+	fpioa_gpio: gpio {
+		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
+			 <K210_FPIOA(9, K210_PCF_GPIO1)>,
+			 <K210_FPIOA(10, K210_PCF_GPIO2)>,
+			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
+			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
+			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
+			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
+			 <K210_FPIOA(15, K210_PCF_GPIO7)>;
+	};
+
+	fpioa_gpiohs: gpiohs {
+		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
+			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
+			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
+			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
+			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
+			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
+			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
+			 <K210_FPIOA(30, K210_PCF_GPIOHS14)>,
+			 <K210_FPIOA(31, K210_PCF_GPIOHS15)>,
+			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
+			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
+			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
+			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
+	};
+
+	fpioa_i2s0: i2s0 {
+		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
+			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
+			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
+	};
+
+	fpioa_dvp: dvp {
+		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
+			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
+			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
+			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
+			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
+			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
+			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
+			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
+	};
+
+	fpioa_spi0: spi0 {
+		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
+			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
+			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
+			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
+	};
+
+	fpioa_spi1: spi1 {
+		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
+			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
+			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
+			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>;
+	};
+};
+
+&dvp0 {
+	pinctrl-0 = <&fpioa_dvp>;
+	pinctrl-names = "default";
 };
diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
index 2546c7d4e0..fc7986b326 100644
--- a/arch/riscv/dts/k210.dtsi
+++ b/arch/riscv/dts/k210.dtsi
@@ -5,6 +5,7 @@
 
 #include <dt-bindings/clock/k210-sysctl.h>
 #include <dt-bindings/mfd/k210-sysctl.h>
+#include <dt-bindings/pinctrl/k210-pinctrl.h>
 #include <dt-bindings/reset/k210-sysctl.h>
 
 / {
@@ -367,7 +368,18 @@
 				reg = <0x502B0000 0x100>;
 				clocks = <&sysclk K210_CLK_FPIOA>;
 				resets = <&sysrst K210_RST_FPIOA>;
+				kendryte,sysctl = <&sysctl>;
+				kendryte,power-offset = <K210_SYSCTL_POWER_SEL>;
+				pinctrl-0 = <&fpioa_jtag>;
+				pinctrl-names = "default";
 				status = "disabled";
+
+				fpioa_jtag: jtag {
+					pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
+						 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
+						 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
+						 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
+				};
 			};
 
 			sha256: sha256 at 502C0000 {
-- 
2.28.0

[PATCH v5 10/11] riscv: Add pinmux and gpio bindings for Kendryte K210

[Heinrich Schuchardt]

On 15.08.20 17:52, Sean Anderson wrote:
> This patch adds the necessary device tree bindings.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v2)
>
> Changes in v2:
> - Convert to use pinmux property
> - Don't hog ISP on boot
> - Re-order GPIOs to match the defaults more closely
>
>  arch/riscv/dts/k210-maix-bit.dts | 104 +++++++++++++++++++++++++++++++
>  arch/riscv/dts/k210.dtsi         |  12 ++++
>  2 files changed, 116 insertions(+)
>
> diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
> index 5b32c5fd5f..e840e04ada 100644
> --- a/arch/riscv/dts/k210-maix-bit.dts
> +++ b/arch/riscv/dts/k210-maix-bit.dts
> @@ -17,6 +17,22 @@
>  		stdout-path = "serial0:115200";
>  	};
>
> +	gpio-leds {
> +		compatible = "gpio-leds";
> +
> +		green {
> +			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		red {
> +			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		blue {
> +			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
> +		};
> +	};
> +
>  	sound {
>  		compatible = "simple-audio-card";
>  		simple-audio-card,format = "i2s";
> @@ -39,9 +55,97 @@
>  };
>
>  &uarths0 {
> +	pinctrl-0 = <&fpioa_uarths>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
> +
> +&gpio0 {
> +	pinctrl-0 = <&fpioa_gpiohs>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +};
> +
> +&gpio1 {
> +	pinctrl-0 = <&fpioa_gpio>;
> +	pinctrl-names = "default";
>  	status = "okay";
>  };
>
>  &i2s0 {
>  	#sound-dai-cells = <1>;
> +	pinctrl-0 = <&fpioa_i2s0>;
> +	pinctrl-names = "default";
> +};
> +
> +&fpioa {
> +	status = "okay";
> +
> +	fpioa_uarths: uarths {
> +		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
> +			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
> +	};
> +
> +	fpioa_gpio: gpio {
> +		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
> +			 <K210_FPIOA(9, K210_PCF_GPIO1)>,
> +			 <K210_FPIOA(10, K210_PCF_GPIO2)>,
> +			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
> +			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
> +			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
> +			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
> +			 <K210_FPIOA(15, K210_PCF_GPIO7)>;

On the Maixduino the K210 package pins IO_6 - IO_9, IO_25 - IO_29 serve
as interface to the ESP32 which is used for WLAN. In the schema for the
Maixduino the naming is:

IO_6 - ESP32_TX
IO_7 - ESP32_RX
IO_8 - ESP32_EN
IO_9 - ESP32_READY

IO_6 and IO_7 are not mapped in reset state and in a Python example I
found for connecting to a WLAN these lines were not used.

If the usage of IO_8 and IO_9 differs between the Maixduino and the Maix
Bit, shouldn't they be mapped in k210-maix-bit.dts instead of k210.dtsi?

According to https://github.com/sipeed/MaixPy/issues/127 one of the SPI
interfaces is switched between SD-card and WLAN. The ESP32 connection
uses gpiohs while the SD card does not.

> +	};
> +
> +	fpioa_gpiohs: gpiohs {
> +		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,

IO_16 is connected to the boot button on the Maixduino and Maix Fit
boards. So I guess we should add an entry compatible = "gpio-keys" for
it and add a comment here.

> +			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
> +			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
> +			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
> +			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
> +			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
> +			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
> +			 <K210_FPIOA(30, K210_PCF_GPIOHS14)>,
> +			 <K210_FPIOA(31, K210_PCF_GPIOHS15)>,
> +			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
> +			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
> +			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
> +			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
> +	};
> +
> +	fpioa_i2s0: i2s0 {
> +		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
> +			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
> +			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
> +	};
> +
> +	fpioa_dvp: dvp {
> +		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,

DVP_SDA in 'Maix-Bit-V2.0(Pin assignment table.xlsx'

> +			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,

DVP_SCL

> +			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,

DVP_RST

> +			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,

DVP_VSYNC

> +			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,

DVP_PWDN

> +			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,

DVP_HSYNC

> +			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,

DVP_XCLK

> +			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;

DVP_PCLK

> +	};
> +
> +	fpioa_spi0: spi0 {
> +		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */

LCD_CS in 'Maix-Bit-V2.0(Pin assignment table.xlsx'

> +			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */

LCD_RST

> +			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */

LCD_DC

> +			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */

LCD_WR

Should the comments be updated to reflect the connection to LCD?

> +	};
> +
> +	fpioa_spi1: spi1 {
> +		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,

SPI0 MISO in 'Maix-Bit-V2.0(Pin assignment table.xlsx'

> +			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,

SPI0 SCLK

> +			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,

SPI0 MOSI

> +			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>;

SPI0_CS0

Should we add comments?

Best regards

Heinrich

> +	};
> +};
> +
> +&dvp0 {
> +	pinctrl-0 = <&fpioa_dvp>;
> +	pinctrl-names = "default";
>  };
> diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
> index 2546c7d4e0..fc7986b326 100644
> --- a/arch/riscv/dts/k210.dtsi
> +++ b/arch/riscv/dts/k210.dtsi
> @@ -5,6 +5,7 @@
>
>  #include <dt-bindings/clock/k210-sysctl.h>
>  #include <dt-bindings/mfd/k210-sysctl.h>
> +#include <dt-bindings/pinctrl/k210-pinctrl.h>
>  #include <dt-bindings/reset/k210-sysctl.h>
>
>  / {
> @@ -367,7 +368,18 @@
>  				reg = <0x502B0000 0x100>;
>  				clocks = <&sysclk K210_CLK_FPIOA>;
>  				resets = <&sysrst K210_RST_FPIOA>;
> +				kendryte,sysctl = <&sysctl>;
> +				kendryte,power-offset = <K210_SYSCTL_POWER_SEL>;
> +				pinctrl-0 = <&fpioa_jtag>;
> +				pinctrl-names = "default";
>  				status = "disabled";
> +
> +				fpioa_jtag: jtag {
> +					pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
> +						 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
> +						 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
> +						 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
> +				};
>  			};
>
>  			sha256: sha256 at 502C0000 {
>

[PATCH v5 10/11] riscv: Add pinmux and gpio bindings for Kendryte K210

[Sean Anderson]

On 9/2/20 2:04 PM, Heinrich Schuchardt wrote:
> On 15.08.20 17:52, Sean Anderson wrote:
>> This patch adds the necessary device tree bindings.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> (no changes since v2)
>>
>> Changes in v2:
>> - Convert to use pinmux property
>> - Don't hog ISP on boot
>> - Re-order GPIOs to match the defaults more closely
>>
>>  arch/riscv/dts/k210-maix-bit.dts | 104 +++++++++++++++++++++++++++++++
>>  arch/riscv/dts/k210.dtsi         |  12 ++++
>>  2 files changed, 116 insertions(+)
>>
>> diff --git a/arch/riscv/dts/k210-maix-bit.dts b/arch/riscv/dts/k210-maix-bit.dts
>> index 5b32c5fd5f..e840e04ada 100644
>> --- a/arch/riscv/dts/k210-maix-bit.dts
>> +++ b/arch/riscv/dts/k210-maix-bit.dts
>> @@ -17,6 +17,22 @@
>>  		stdout-path = "serial0:115200";
>>  	};
>>
>> +	gpio-leds {
>> +		compatible = "gpio-leds";
>> +
>> +		green {
>> +			gpios = <&gpio1_0 4 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		red {
>> +			gpios = <&gpio1_0 5 GPIO_ACTIVE_LOW>;
>> +		};
>> +
>> +		blue {
>> +			gpios = <&gpio1_0 6 GPIO_ACTIVE_LOW>;
>> +		};
>> +	};
>> +
>>  	sound {
>>  		compatible = "simple-audio-card";
>>  		simple-audio-card,format = "i2s";
>> @@ -39,9 +55,97 @@
>>  };
>>
>>  &uarths0 {
>> +	pinctrl-0 = <&fpioa_uarths>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +};
>> +
>> +&gpio0 {
>> +	pinctrl-0 = <&fpioa_gpiohs>;
>> +	pinctrl-names = "default";
>> +	status = "okay";
>> +};
>> +
>> +&gpio1 {
>> +	pinctrl-0 = <&fpioa_gpio>;
>> +	pinctrl-names = "default";
>>  	status = "okay";
>>  };
>>
>>  &i2s0 {
>>  	#sound-dai-cells = <1>;
>> +	pinctrl-0 = <&fpioa_i2s0>;
>> +	pinctrl-names = "default";
>> +};
>> +
>> +&fpioa {
>> +	status = "okay";
>> +
>> +	fpioa_uarths: uarths {
>> +		pinmux = <K210_FPIOA(4, K210_PCF_UARTHS_RX)>,
>> +			 <K210_FPIOA(5, K210_PCF_UARTHS_TX)>;
>> +	};
>> +
>> +	fpioa_gpio: gpio {
>> +		pinmux = <K210_FPIOA(8, K210_PCF_GPIO0)>,
>> +			 <K210_FPIOA(9, K210_PCF_GPIO1)>,
>> +			 <K210_FPIOA(10, K210_PCF_GPIO2)>,
>> +			 <K210_FPIOA(11, K210_PCF_GPIO3)>,
>> +			 <K210_FPIOA(12, K210_PCF_GPIO4)>,
>> +			 <K210_FPIOA(13, K210_PCF_GPIO5)>,
>> +			 <K210_FPIOA(14, K210_PCF_GPIO6)>,
>> +			 <K210_FPIOA(15, K210_PCF_GPIO7)>;
> 
> On the Maixduino the K210 package pins IO_6 - IO_9, IO_25 - IO_29 serve
> as interface to the ESP32 which is used for WLAN. In the schema for the
> Maixduino the naming is:
> 
> IO_6 - ESP32_TX
> IO_7 - ESP32_RX
> IO_8 - ESP32_EN
> IO_9 - ESP32_READY
> 
> IO_6 and IO_7 are not mapped in reset state and in a Python example I
> found for connecting to a WLAN these lines were not used.
> 
> If the usage of IO_8 and IO_9 differs between the Maixduino and the Maix
> Bit, shouldn't they be mapped in k210-maix-bit.dts instead of k210.dtsi?

These *are* in k210-maix-bit.dts.

> 
> According to https://github.com/sipeed/MaixPy/issues/127 one of the SPI
> interfaces is switched between SD-card and WLAN. The ESP32 connection
> uses gpiohs while the SD card does not.
> 
>> +	};
>> +
>> +	fpioa_gpiohs: gpiohs {
>> +		pinmux = <K210_FPIOA(16, K210_PCF_GPIOHS0)>,
> 
> IO_16 is connected to the boot button on the Maixduino and Maix Fit
> boards. So I guess we should add an entry compatible = "gpio-keys" for
> it and add a comment here.

I saw you created a patch for this [1]. Would you like to keep that
separate, or should I add it to this series?

[1] https://patchwork.ozlabs.org/project/uboot/patch/20200902201159.98034-1-xypron.glpk at gmx.de/

>> +			 <K210_FPIOA(17, K210_PCF_GPIOHS1)>,
>> +			 <K210_FPIOA(21, K210_PCF_GPIOHS5)>,
>> +			 <K210_FPIOA(22, K210_PCF_GPIOHS6)>,
>> +			 <K210_FPIOA(23, K210_PCF_GPIOHS7)>,
>> +			 <K210_FPIOA(24, K210_PCF_GPIOHS8)>,
>> +			 <K210_FPIOA(25, K210_PCF_GPIOHS9)>,
>> +			 <K210_FPIOA(30, K210_PCF_GPIOHS14)>,
>> +			 <K210_FPIOA(31, K210_PCF_GPIOHS15)>,
>> +			 <K210_FPIOA(32, K210_PCF_GPIOHS16)>,
>> +			 <K210_FPIOA(33, K210_PCF_GPIOHS17)>,
>> +			 <K210_FPIOA(34, K210_PCF_GPIOHS18)>,
>> +			 <K210_FPIOA(35, K210_PCF_GPIOHS19)>;
>> +	};
>> +
>> +	fpioa_i2s0: i2s0 {
>> +		pinmux = <K210_FPIOA(18, K210_PCF_I2S0_SCLK)>,
>> +			 <K210_FPIOA(19, K210_PCF_I2S0_WS)>,
>> +			 <K210_FPIOA(20, K210_PCF_I2S0_IN_D0)>;
>> +	};
>> +
>> +	fpioa_dvp: dvp {
>> +		pinmux = <K210_FPIOA(40, K210_PCF_SCCB_SDA)>,
> 
> DVP_SDA in 'Maix-Bit-V2.0(Pin assignment table.xlsx'
> 
>> +			 <K210_FPIOA(41, K210_PCF_SCCB_SCLK)>,
> 
> DVP_SCL
> 
>> +			 <K210_FPIOA(42, K210_PCF_DVP_RST)>,
> 
> DVP_RST
> 
>> +			 <K210_FPIOA(43, K210_PCF_DVP_VSYNC)>,
> 
> DVP_VSYNC
> 
>> +			 <K210_FPIOA(44, K210_PCF_DVP_PWDN)>,
> 
> DVP_PWDN
> 
>> +			 <K210_FPIOA(45, K210_PCF_DVP_HSYNC)>,
> 
> DVP_HSYNC
> 
>> +			 <K210_FPIOA(46, K210_PCF_DVP_XCLK)>,
> 
> DVP_XCLK
> 
>> +			 <K210_FPIOA(47, K210_PCF_DVP_PCLK)>;
> 
> DVP_PCLK
> 
>> +	};
>> +
>> +	fpioa_spi0: spi0 {
>> +		pinmux = <K210_FPIOA(36, K210_PCF_GPIOHS20)>,  /* cs */
> 
> LCD_CS in 'Maix-Bit-V2.0(Pin assignment table.xlsx'
> 
>> +			 <K210_FPIOA(37, K210_PCF_GPIOHS21)>,  /* rst */
> 
> LCD_RST
> 
>> +			 <K210_FPIOA(38, K210_PCF_GPIOHS22)>,  /* dc */
> 
> LCD_DC
> 
>> +			 <K210_FPIOA(39, K210_PCF_SPI0_SCLK)>; /* wr */
> 
> LCD_WR
> 
> Should the comments be updated to reflect the connection to LCD?

Sure.

>> +	};
>> +
>> +	fpioa_spi1: spi1 {
>> +		pinmux = <K210_FPIOA(26, K210_PCF_SPI1_D1)>,
> 
> SPI0 MISO in 'Maix-Bit-V2.0(Pin assignment table.xlsx'
> 
>> +			 <K210_FPIOA(27, K210_PCF_SPI1_SCLK)>,
> 
> SPI0 SCLK
> 
>> +			 <K210_FPIOA(28, K210_PCF_SPI1_D0)>,
> 
> SPI0 MOSI
> 
>> +			 <K210_FPIOA(29, K210_PCF_GPIOHS13)>;
> 
> SPI0_CS0
> 
> Should we add comments?

I will add some for this spi in the next revision.

> 
> Best regards
> 
> Heinrich
> 
>> +	};
>> +};
>> +
>> +&dvp0 {
>> +	pinctrl-0 = <&fpioa_dvp>;
>> +	pinctrl-names = "default";
>>  };
>> diff --git a/arch/riscv/dts/k210.dtsi b/arch/riscv/dts/k210.dtsi
>> index 2546c7d4e0..fc7986b326 100644
>> --- a/arch/riscv/dts/k210.dtsi
>> +++ b/arch/riscv/dts/k210.dtsi
>> @@ -5,6 +5,7 @@
>>
>>  #include <dt-bindings/clock/k210-sysctl.h>
>>  #include <dt-bindings/mfd/k210-sysctl.h>
>> +#include <dt-bindings/pinctrl/k210-pinctrl.h>
>>  #include <dt-bindings/reset/k210-sysctl.h>
>>
>>  / {
>> @@ -367,7 +368,18 @@
>>  				reg = <0x502B0000 0x100>;
>>  				clocks = <&sysclk K210_CLK_FPIOA>;
>>  				resets = <&sysrst K210_RST_FPIOA>;
>> +				kendryte,sysctl = <&sysctl>;
>> +				kendryte,power-offset = <K210_SYSCTL_POWER_SEL>;
>> +				pinctrl-0 = <&fpioa_jtag>;
>> +				pinctrl-names = "default";
>>  				status = "disabled";
>> +
>> +				fpioa_jtag: jtag {
>> +					pinmux = <K210_FPIOA(0, K210_PCF_JTAG_TCLK)>,
>> +						 <K210_FPIOA(1, K210_PCF_JTAG_TDI)>,
>> +						 <K210_FPIOA(2, K210_PCF_JTAG_TMS)>,
>> +						 <K210_FPIOA(3, K210_PCF_JTAG_TDO)>;
>> +				};
>>  			};
>>
>>  			sha256: sha256 at 502C0000 {
>>
> 

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

This patch adds the necessary configs and docs for FPIOA and GPIO support
on the K210.

The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
changed). It also boots when changes are made to the device tree and then
committed. I don't know why this happens. These breakages only occur after
bf2fb81ad3.

Signed-off-by: Sean Anderson <seanga2@gmail.com>
---

Changes in v5:
- Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
- Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
  by commit 2548493ab4.

Changes in v3:
- Document pins 6 and 7 as not set

Changes in v2:
- Remove SPI flash related Kconfig settings

 board/sipeed/maix/Kconfig          |  9 +++++
 configs/sipeed_maix_bitm_defconfig |  2 +
 doc/board/sipeed/maix.rst          | 64 +++++++++++++++++++++++++++++-
 3 files changed, 73 insertions(+), 2 deletions(-)

diff --git a/board/sipeed/maix/Kconfig b/board/sipeed/maix/Kconfig
index 0cdcd32adc..4c42dd2087 100644
--- a/board/sipeed/maix/Kconfig
+++ b/board/sipeed/maix/Kconfig
@@ -44,4 +44,13 @@ config BOARD_SPECIFIC_OPTIONS
 	imply RESET_SYSCON
 	imply SYSRESET
 	imply SYSRESET_SYSCON
+	imply PINCTRL
+	imply PINCONF
+	imply PINCTRL_K210
+	imply DM_GPIO
+	imply DWAPB_GPIO
+	imply SIFIVE_GPIO
+	imply CMD_GPIO
+	imply LED
+	imply LED_GPIO
 endif
diff --git a/configs/sipeed_maix_bitm_defconfig b/configs/sipeed_maix_bitm_defconfig
index 459bf0d530..0b038ff0a1 100644
--- a/configs/sipeed_maix_bitm_defconfig
+++ b/configs/sipeed_maix_bitm_defconfig
@@ -2,6 +2,8 @@ CONFIG_RISCV=y
 CONFIG_TARGET_SIPEED_MAIX=y
 CONFIG_ARCH_RV64I=y
 CONFIG_STACK_SIZE=0x100000
+# FIXME: Dirty hack to get boot working!
+CONFIG_LOGLEVEL=5
 # CONFIG_NET is not set
 # CONFIG_INPUT is not set
 # CONFIG_DM_ETH is not set
diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst
index b1894f3a6f..3811eac61f 100644
--- a/doc/board/sipeed/maix.rst
+++ b/doc/board/sipeed/maix.rst
@@ -156,7 +156,7 @@ To run legacy images, use the ``bootm`` command:
     Load Address: 80000000
     Entry Point:  80000000
 
-    $ picocom -b 115200 /dev/ttyUSB0i
+    $ picocom -b 115200 /dev/ttyUSB0
     => loady
     ## Ready for binary (ymodem) download to 0x80000000 at 115200 bps...
     C
@@ -187,6 +187,66 @@ To run legacy images, use the ``bootm`` command:
     argv[0] = "<NULL>"
     Hit any key to exit ...
 
+Pin Assignment
+--------------
+
+The K210 contains a Fully Programmable I/O Array (FPIOA), which can remap any of
+its 256 input functions to any any of 48 output pins. The following table has
+the default pin assignments for the BitM.
+
+===== ========== =======
+Pin   Function   Comment
+===== ========== =======
+IO_0  JTAG_TCLK
+IO_1  JTAG_TDI
+IO_2  JTAG_TMS
+IO_3  JTAG_TDO
+IO_4  UARTHS_RX
+IO_5  UARTHS_TX
+IO_6             Not set
+IO_7             Not set
+IO_8  GPIO_0
+IO_9  GPIO_1
+IO_10 GPIO_2
+IO_11 GPIO_3
+IO_12 GPIO_4     Green LED
+IO_13 GPIO_5     Red LED
+IO_14 GPIO_6     Blue LED
+IO_15 GPIO_7
+IO_16 GPIOHS_0   ISP
+IO_17 GPIOHS_1
+IO_18 I2S0_SCLK  MIC CLK
+IO_19 I2S0_WS    MIC WS
+IO_20 I2S0_IN_D0 MIC SD
+IO_21 GPIOHS_5
+IO_22 GPIOHS_6
+IO_23 GPIOHS_7
+IO_24 GPIOHS_8
+IO_25 GPIOHS_9
+IO_26 SPI1_D1    MMC MISO
+IO_27 SPI1_SCLK  MMC CLK
+IO_28 SPI1_D0    MMC MOSI
+IO_29 GPIOHS_13  MMC CS
+IO_30 GPIOHS_14
+IO_31 GPIOHS_15
+IO_32 GPIOHS_16
+IO_33 GPIOHS_17
+IO_34 GPIOHS_18
+IO_35 GPIOHS_19
+IO_36 GPIOHS_20  Panel CS
+IO_37 GPIOHS_21  Panel RST
+IO_38 GPIOHS_22  Panel DC
+IO_39 SPI0_SCK   Panel WR
+IO_40 SCCP_SDA
+IO_41 SCCP_SCLK
+IO_42 DVP_RST
+IO_43 DVP_VSYNC
+IO_44 DVP_PWDN
+IO_45 DVP_HSYNC
+IO_46 DVP_XCLK
+IO_47 DVP_PCLK
+===== ========== =======
+
 Over- and Under-clocking
 ------------------------
 
@@ -361,5 +421,5 @@ Address    Size      Description
 0x8801C000 0x1000    riscv priv spec 1.9 config
 0x8801D000 0x2000    flattened device tree (contains only addresses and
                      interrupts)
-0x8801f000 0x1000    credits
+0x8801F000 0x1000    credits
 ========== ========= ===========
-- 
2.28.0

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Rick Chen]

Hi Tom

> This patch adds the necessary configs and docs for FPIOA and GPIO support
> on the K210.
>
> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
> changed). It also boots when changes are made to the device tree and then
> committed. I don't know why this happens. These breakages only occur after
> bf2fb81ad3.
>
> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> ---
>
> Changes in v5:
> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>   by commit 2548493ab4.

Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
FPIOA and GPIO support for Kendryte K210 ?
Or maybe it is better to figure out what is wrong here and find the
root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?

Thanks,
Rick

>
> Changes in v3:
> - Document pins 6 and 7 as not set
>
> Changes in v2:
> - Remove SPI flash related Kconfig settings
>
>  board/sipeed/maix/Kconfig          |  9 +++++
>  configs/sipeed_maix_bitm_defconfig |  2 +
>  doc/board/sipeed/maix.rst          | 64 +++++++++++++++++++++++++++++-
>  3 files changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/board/sipeed/maix/Kconfig b/board/sipeed/maix/Kconfig
> index 0cdcd32adc..4c42dd2087 100644
> --- a/board/sipeed/maix/Kconfig
> +++ b/board/sipeed/maix/Kconfig
> @@ -44,4 +44,13 @@ config BOARD_SPECIFIC_OPTIONS
>         imply RESET_SYSCON
>         imply SYSRESET
>         imply SYSRESET_SYSCON
> +       imply PINCTRL
> +       imply PINCONF
> +       imply PINCTRL_K210
> +       imply DM_GPIO
> +       imply DWAPB_GPIO
> +       imply SIFIVE_GPIO
> +       imply CMD_GPIO
> +       imply LED
> +       imply LED_GPIO
>  endif
> diff --git a/configs/sipeed_maix_bitm_defconfig b/configs/sipeed_maix_bitm_defconfig
> index 459bf0d530..0b038ff0a1 100644
> --- a/configs/sipeed_maix_bitm_defconfig
> +++ b/configs/sipeed_maix_bitm_defconfig
> @@ -2,6 +2,8 @@ CONFIG_RISCV=y
>  CONFIG_TARGET_SIPEED_MAIX=y
>  CONFIG_ARCH_RV64I=y
>  CONFIG_STACK_SIZE=0x100000
> +# FIXME: Dirty hack to get boot working!
> +CONFIG_LOGLEVEL=5
>  # CONFIG_NET is not set
>  # CONFIG_INPUT is not set
>  # CONFIG_DM_ETH is not set
> diff --git a/doc/board/sipeed/maix.rst b/doc/board/sipeed/maix.rst
> index b1894f3a6f..3811eac61f 100644
> --- a/doc/board/sipeed/maix.rst
> +++ b/doc/board/sipeed/maix.rst
> @@ -156,7 +156,7 @@ To run legacy images, use the ``bootm`` command:
>      Load Address: 80000000
>      Entry Point:  80000000
>
> -    $ picocom -b 115200 /dev/ttyUSB0i
> +    $ picocom -b 115200 /dev/ttyUSB0
>      => loady
>      ## Ready for binary (ymodem) download to 0x80000000 at 115200 bps...
>      C
> @@ -187,6 +187,66 @@ To run legacy images, use the ``bootm`` command:
>      argv[0] = "<NULL>"
>      Hit any key to exit ...
>
> +Pin Assignment
> +--------------
> +
> +The K210 contains a Fully Programmable I/O Array (FPIOA), which can remap any of
> +its 256 input functions to any any of 48 output pins. The following table has
> +the default pin assignments for the BitM.
> +
> +===== ========== =======
> +Pin   Function   Comment
> +===== ========== =======
> +IO_0  JTAG_TCLK
> +IO_1  JTAG_TDI
> +IO_2  JTAG_TMS
> +IO_3  JTAG_TDO
> +IO_4  UARTHS_RX
> +IO_5  UARTHS_TX
> +IO_6             Not set
> +IO_7             Not set
> +IO_8  GPIO_0
> +IO_9  GPIO_1
> +IO_10 GPIO_2
> +IO_11 GPIO_3
> +IO_12 GPIO_4     Green LED
> +IO_13 GPIO_5     Red LED
> +IO_14 GPIO_6     Blue LED
> +IO_15 GPIO_7
> +IO_16 GPIOHS_0   ISP
> +IO_17 GPIOHS_1
> +IO_18 I2S0_SCLK  MIC CLK
> +IO_19 I2S0_WS    MIC WS
> +IO_20 I2S0_IN_D0 MIC SD
> +IO_21 GPIOHS_5
> +IO_22 GPIOHS_6
> +IO_23 GPIOHS_7
> +IO_24 GPIOHS_8
> +IO_25 GPIOHS_9
> +IO_26 SPI1_D1    MMC MISO
> +IO_27 SPI1_SCLK  MMC CLK
> +IO_28 SPI1_D0    MMC MOSI
> +IO_29 GPIOHS_13  MMC CS
> +IO_30 GPIOHS_14
> +IO_31 GPIOHS_15
> +IO_32 GPIOHS_16
> +IO_33 GPIOHS_17
> +IO_34 GPIOHS_18
> +IO_35 GPIOHS_19
> +IO_36 GPIOHS_20  Panel CS
> +IO_37 GPIOHS_21  Panel RST
> +IO_38 GPIOHS_22  Panel DC
> +IO_39 SPI0_SCK   Panel WR
> +IO_40 SCCP_SDA
> +IO_41 SCCP_SCLK
> +IO_42 DVP_RST
> +IO_43 DVP_VSYNC
> +IO_44 DVP_PWDN
> +IO_45 DVP_HSYNC
> +IO_46 DVP_XCLK
> +IO_47 DVP_PCLK
> +===== ========== =======
> +
>  Over- and Under-clocking
>  ------------------------
>
> @@ -361,5 +421,5 @@ Address    Size      Description
>  0x8801C000 0x1000    riscv priv spec 1.9 config
>  0x8801D000 0x2000    flattened device tree (contains only addresses and
>                       interrupts)
> -0x8801f000 0x1000    credits
> +0x8801F000 0x1000    credits
>  ========== ========= ===========
> --
> 2.28.0
>

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

On 8/18/20 11:48 PM, Rick Chen wrote:
> Hi Tom
> 
>> This patch adds the necessary configs and docs for FPIOA and GPIO support
>> on the K210.
>>
>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
>> changed). It also boots when changes are made to the device tree and then
>> committed. I don't know why this happens. These breakages only occur after
>> bf2fb81ad3.
>>
>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>> ---
>>
>> Changes in v5:
>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>>   by commit 2548493ab4.
> 
> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
> FPIOA and GPIO support for Kendryte K210 ?
> Or maybe it is better to figure out what is wrong here and find the
> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?

As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
for the same effect. In addition, there are several other ways I found
to "fix" this bug (as noted in the commit message). If you would like to
test this out, I have two trees [1, 2] where this series (actually a slightly
earlier version of this series) is applied just before and just after
bf2fb81ad3. The original patch is located at [3].

--Sean

[1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
[2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
[3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Rick Chen]

Hi Sean

> On 8/18/20 11:48 PM, Rick Chen wrote:
> > Hi Tom
> >
> >> This patch adds the necessary configs and docs for FPIOA and GPIO support
> >> on the K210.
> >>
> >> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
> >> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
> >> changed). It also boots when changes are made to the device tree and then
> >> committed. I don't know why this happens. These breakages only occur after
> >> bf2fb81ad3.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >>
> >> Changes in v5:
> >> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
> >> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
> >>   by commit 2548493ab4.
> >
> > Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
> > FPIOA and GPIO support for Kendryte K210 ?
> > Or maybe it is better to figure out what is wrong here and find the
> > root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
>
> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
> for the same effect. In addition, there are several other ways I found
> to "fix" this bug (as noted in the commit message). If you would like to
> test this out, I have two trees [1, 2] where this series (actually a slightly
> earlier version of this series) is applied just before and just after
> bf2fb81ad3. The original patch is located at [3].
>
> --Sean
>
> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/

I don't have a K210 board for verification.
But it is OK to run in AE350 board after applying your series.

After check about this commit "common/board_r: Remove initr_serial
wrapper", it seem shall not affect anything.
It just change to call serial_initialize directly.
Only I can think about maybe it is a cache problem.
Just like sometime we add a printf, then the problem will be walk around.

Thanks,
Rick

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Rick Chen]

Hi Sean

> Hi Sean
>
> > On 8/18/20 11:48 PM, Rick Chen wrote:
> > > Hi Tom
> > >
> > >> This patch adds the necessary configs and docs for FPIOA and GPIO support
> > >> on the K210.
> > >>
> > >> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
> > >> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
> > >> changed). It also boots when changes are made to the device tree and then
> > >> committed. I don't know why this happens. These breakages only occur after
> > >> bf2fb81ad3.
> > >>
> > >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> > >> ---
> > >>
> > >> Changes in v5:
> > >> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
> > >> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
> > >>   by commit 2548493ab4.
> > >
> > > Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
> > > FPIOA and GPIO support for Kendryte K210 ?
> > > Or maybe it is better to figure out what is wrong here and find the
> > > root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
> >
> > As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
> > for the same effect. In addition, there are several other ways I found
> > to "fix" this bug (as noted in the commit message). If you would like to
> > test this out, I have two trees [1, 2] where this series (actually a slightly
> > earlier version of this series) is applied just before and just after
> > bf2fb81ad3. The original patch is located at [3].
> >
> > --Sean
> >
> > [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
> > [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
> > [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/
>
> I don't have a K210 board for verification.
> But it is OK to run in AE350 board after applying your series.
>
> After check about this commit "common/board_r: Remove initr_serial
> wrapper", it seem shall not affect anything.
> It just change to call serial_initialize directly.
> Only I can think about maybe it is a cache problem.
> Just like sometime we add a printf, then the problem will be walk around.

Can you dig in to find the root cause ?
For code stability, it is better not to have any unknown issue.
Yo can dirty hack and work around currently, but it may crash again
after several commits.

Thanks,
Rick

>
> Thanks,
> Rick

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

On 8/20/20 4:47 AM, Rick Chen wrote:
> Hi Sean
> 
>> Hi Sean
>>
>>> On 8/18/20 11:48 PM, Rick Chen wrote:
>>>> Hi Tom
>>>>
>>>>> This patch adds the necessary configs and docs for FPIOA and GPIO support
>>>>> on the K210.
>>>>>
>>>>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
>>>>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
>>>>> changed). It also boots when changes are made to the device tree and then
>>>>> committed. I don't know why this happens. These breakages only occur after
>>>>> bf2fb81ad3.
>>>>>
>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>>> ---
>>>>>
>>>>> Changes in v5:
>>>>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
>>>>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>>>>>   by commit 2548493ab4.
>>>>
>>>> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
>>>> FPIOA and GPIO support for Kendryte K210 ?
>>>> Or maybe it is better to figure out what is wrong here and find the
>>>> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
>>>
>>> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
>>> for the same effect. In addition, there are several other ways I found
>>> to "fix" this bug (as noted in the commit message). If you would like to
>>> test this out, I have two trees [1, 2] where this series (actually a slightly
>>> earlier version of this series) is applied just before and just after
>>> bf2fb81ad3. The original patch is located at [3].
>>>
>>> --Sean
>>>
>>> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
>>> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
>>> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/
>>
>> I don't have a K210 board for verification.
>> But it is OK to run in AE350 board after applying your series.
>>
>> After check about this commit "common/board_r: Remove initr_serial
>> wrapper", it seem shall not affect anything.
>> It just change to call serial_initialize directly.
>> Only I can think about maybe it is a cache problem.
>> Just like sometime we add a printf, then the problem will be walk around.
> 
> Can you dig in to find the root cause ?
> For code stability, it is better not to have any unknown issue.
> Yo can dirty hack and work around currently, but it may crash again
> after several commits.

Ok, so I did some further digging, but I was unable to pin down the
cause of the bug. My efforts to determine a cause have been primarily
thwarted because the bug disappears after any change to initialization
code. Adding any function to init_sequence_f or init_sequence_r, even a
no-op function which just returns 0, causes the board to boot normally.
In addition, adding a nop() to any function in those sequences will
cause the board to boot normally. The board seems to fail to boot only
with a very specific boot sequence and timing.

When the board fails to boot, it hangs in a manner similar to when the
airam is accessed: the bus appears to hang. Just as when airam is
accessed, a debugger cannot read registers or memory until the cpu is
reset. This may be a separate issue which hangs the bus, or it could
also be caused by an inadvertent access to airam. The puzzling thing is
still the manner of triggering the bug. It's almost as if it was
specifically designed to be impossible to debug by disappearing whenever
one makes an attempt...

--Sean

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Rick Chen]

Hi Sean

> On 8/20/20 4:47 AM, Rick Chen wrote:
> > Hi Sean
> >
> >> Hi Sean
> >>
> >>> On 8/18/20 11:48 PM, Rick Chen wrote:
> >>>> Hi Tom
> >>>>
> >>>>> This patch adds the necessary configs and docs for FPIOA and GPIO support
> >>>>> on the K210.
> >>>>>
> >>>>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
> >>>>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
> >>>>> changed). It also boots when changes are made to the device tree and then
> >>>>> committed. I don't know why this happens. These breakages only occur after
> >>>>> bf2fb81ad3.
> >>>>>
> >>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >>>>> ---
> >>>>>
> >>>>> Changes in v5:
> >>>>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
> >>>>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
> >>>>>   by commit 2548493ab4.
> >>>>
> >>>> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
> >>>> FPIOA and GPIO support for Kendryte K210 ?
> >>>> Or maybe it is better to figure out what is wrong here and find the
> >>>> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
> >>>
> >>> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
> >>> for the same effect. In addition, there are several other ways I found
> >>> to "fix" this bug (as noted in the commit message). If you would like to
> >>> test this out, I have two trees [1, 2] where this series (actually a slightly
> >>> earlier version of this series) is applied just before and just after
> >>> bf2fb81ad3. The original patch is located at [3].
> >>>
> >>> --Sean
> >>>
> >>> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
> >>> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
> >>> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/
> >>
> >> I don't have a K210 board for verification.
> >> But it is OK to run in AE350 board after applying your series.
> >>
> >> After check about this commit "common/board_r: Remove initr_serial
> >> wrapper", it seem shall not affect anything.
> >> It just change to call serial_initialize directly.
> >> Only I can think about maybe it is a cache problem.
> >> Just like sometime we add a printf, then the problem will be walk around.
> >
> > Can you dig in to find the root cause ?
> > For code stability, it is better not to have any unknown issue.
> > Yo can dirty hack and work around currently, but it may crash again
> > after several commits.
>
> Ok, so I did some further digging, but I was unable to pin down the
> cause of the bug. My efforts to determine a cause have been primarily
> thwarted because the bug disappears after any change to initialization
> code. Adding any function to init_sequence_f or init_sequence_r, even a
> no-op function which just returns 0, causes the board to boot normally.
> In addition, adding a nop() to any function in those sequences will
> cause the board to boot normally. The board seems to fail to boot only
> with a very specific boot sequence and timing.

If you modify any code and the result will change, then you shall
debug it via debugger(GDB) without any code modification.

>
> When the board fails to boot, it hangs in a manner similar to when the

Maybe you can try to set a break and access the bus, if the bus access
fail, then you re-set a break a bit ahead until the bus access NOT
fail.
To see if this way can pin down which instruction or the crucial code
to cause the bus hang problem. And guess what maybe the root-cause.

If you can find the instruction which may cause the bus hang, you can
info all-registers and compare the differences between NG and OK. And
guess what maybe the root-cause.

Thanks,
Rick

> airam is accessed: the bus appears to hang. Just as when airam is
> accessed, a debugger cannot read registers or memory until the cpu is
> reset. This may be a separate issue which hangs the bus, or it could
> also be caused by an inadvertent access to airam. The puzzling thing is
> still the manner of triggering the bug. It's almost as if it was
> specifically designed to be impossible to debug by disappearing whenever
> one makes an attempt...
>
> --Sean

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Heinrich Schuchardt]

On 01.09.20 03:19, Rick Chen wrote:
> Hi Sean
>
>> On 8/20/20 4:47 AM, Rick Chen wrote:
>>> Hi Sean
>>>
>>>> Hi Sean
>>>>
>>>>> On 8/18/20 11:48 PM, Rick Chen wrote:
>>>>>> Hi Tom
>>>>>>
>>>>>>> This patch adds the necessary configs and docs for FPIOA and GPIO support
>>>>>>> on the K210.
>>>>>>>
>>>>>>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
>>>>>>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
>>>>>>> changed). It also boots when changes are made to the device tree and then
>>>>>>> committed. I don't know why this happens. These breakages only occur after
>>>>>>> bf2fb81ad3.
>>>>>>>
>>>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>>>>> ---
>>>>>>>
>>>>>>> Changes in v5:
>>>>>>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
>>>>>>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>>>>>>>   by commit 2548493ab4.
>>>>>>
>>>>>> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
>>>>>> FPIOA and GPIO support for Kendryte K210 ?
>>>>>> Or maybe it is better to figure out what is wrong here and find the
>>>>>> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
>>>>>
>>>>> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
>>>>> for the same effect. In addition, there are several other ways I found
>>>>> to "fix" this bug (as noted in the commit message). If you would like to
>>>>> test this out, I have two trees [1, 2] where this series (actually a slightly
>>>>> earlier version of this series) is applied just before and just after
>>>>> bf2fb81ad3. The original patch is located at [3].
>>>>>
>>>>> --Sean
>>>>>
>>>>> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
>>>>> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
>>>>> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/
>>>>
>>>> I don't have a K210 board for verification.
>>>> But it is OK to run in AE350 board after applying your series.
>>>>
>>>> After check about this commit "common/board_r: Remove initr_serial
>>>> wrapper", it seem shall not affect anything.
>>>> It just change to call serial_initialize directly.
>>>> Only I can think about maybe it is a cache problem.
>>>> Just like sometime we add a printf, then the problem will be walk around.
>>>
>>> Can you dig in to find the root cause ?
>>> For code stability, it is better not to have any unknown issue.
>>> Yo can dirty hack and work around currently, but it may crash again
>>> after several commits.
>>
>> Ok, so I did some further digging, but I was unable to pin down the
>> cause of the bug. My efforts to determine a cause have been primarily
>> thwarted because the bug disappears after any change to initialization
>> code. Adding any function to init_sequence_f or init_sequence_r, even a
>> no-op function which just returns 0, causes the board to boot normally.
>> In addition, adding a nop() to any function in those sequences will
>> cause the board to boot normally. The board seems to fail to boot only
>> with a very specific boot sequence and timing.
>
> If you modify any code and the result will change, then you shall
> debug it via debugger(GDB) without any code modification.
>
>>
>> When the board fails to boot, it hangs in a manner similar to when the
>
> Maybe you can try to set a break and access the bus, if the bus access
> fail, then you re-set a break a bit ahead until the bus access NOT
> fail.
> To see if this way can pin down which instruction or the crucial code
> to cause the bus hang problem. And guess what maybe the root-cause.
>
> If you can find the instruction which may cause the bus hang, you can
> info all-registers and compare the differences between NG and OK. And
> guess what maybe the root-cause.

Trying to narrow down on the problem I found the following:

The system hangs before arch_cpu_init_dm() is called.

After adding some debug functions the error appeared and disappeared
when changing the code in function panic(). So my guess is that there is
some alignment problem in the static data section.

Best regards

Heinrich

>
> Thanks,
> Rick
>
>> airam is accessed: the bus appears to hang. Just as when airam is
>> accessed, a debugger cannot read registers or memory until the cpu is
>> reset. This may be a separate issue which hangs the bus, or it could
>> also be caused by an inadvertent access to airam. The puzzling thing is
>> still the manner of triggering the bug. It's almost as if it was
>> specifically designed to be impossible to debug by disappearing whenever
>> one makes an attempt...
>>
>> --Sean

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

On 9/2/20 8:26 AM, Heinrich Schuchardt wrote:
> On 01.09.20 03:19, Rick Chen wrote:
>> Hi Sean
>>
>>> On 8/20/20 4:47 AM, Rick Chen wrote:
>>>> Hi Sean
>>>>
>>>>> Hi Sean
>>>>>
>>>>>> On 8/18/20 11:48 PM, Rick Chen wrote:
>>>>>>> Hi Tom
>>>>>>>
>>>>>>>> This patch adds the necessary configs and docs for FPIOA and GPIO support
>>>>>>>> on the K210.
>>>>>>>>
>>>>>>>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
>>>>>>>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
>>>>>>>> changed). It also boots when changes are made to the device tree and then
>>>>>>>> committed. I don't know why this happens. These breakages only occur after
>>>>>>>> bf2fb81ad3.
>>>>>>>>
>>>>>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> Changes in v5:
>>>>>>>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
>>>>>>>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>>>>>>>>   by commit 2548493ab4.
>>>>>>>
>>>>>>> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
>>>>>>> FPIOA and GPIO support for Kendryte K210 ?
>>>>>>> Or maybe it is better to figure out what is wrong here and find the
>>>>>>> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
>>>>>>
>>>>>> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
>>>>>> for the same effect. In addition, there are several other ways I found
>>>>>> to "fix" this bug (as noted in the commit message). If you would like to
>>>>>> test this out, I have two trees [1, 2] where this series (actually a slightly
>>>>>> earlier version of this series) is applied just before and just after
>>>>>> bf2fb81ad3. The original patch is located at [3].
>>>>>>
>>>>>> --Sean
>>>>>>
>>>>>> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
>>>>>> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
>>>>>> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/
>>>>>
>>>>> I don't have a K210 board for verification.
>>>>> But it is OK to run in AE350 board after applying your series.
>>>>>
>>>>> After check about this commit "common/board_r: Remove initr_serial
>>>>> wrapper", it seem shall not affect anything.
>>>>> It just change to call serial_initialize directly.
>>>>> Only I can think about maybe it is a cache problem.
>>>>> Just like sometime we add a printf, then the problem will be walk around.
>>>>
>>>> Can you dig in to find the root cause ?
>>>> For code stability, it is better not to have any unknown issue.
>>>> Yo can dirty hack and work around currently, but it may crash again
>>>> after several commits.
>>>
>>> Ok, so I did some further digging, but I was unable to pin down the
>>> cause of the bug. My efforts to determine a cause have been primarily
>>> thwarted because the bug disappears after any change to initialization
>>> code. Adding any function to init_sequence_f or init_sequence_r, even a
>>> no-op function which just returns 0, causes the board to boot normally.
>>> In addition, adding a nop() to any function in those sequences will
>>> cause the board to boot normally. The board seems to fail to boot only
>>> with a very specific boot sequence and timing.
>>
>> If you modify any code and the result will change, then you shall
>> debug it via debugger(GDB) without any code modification.
>>
>>>
>>> When the board fails to boot, it hangs in a manner similar to when the
>>
>> Maybe you can try to set a break and access the bus, if the bus access
>> fail, then you re-set a break a bit ahead until the bus access NOT
>> fail.

Yeah, I was investigating that, however I was unable to get the k210 to
break at 0x80000000. I suspect this may be a problem with openocd, as
the k210 port is rather buggy (e.g. it can cause address misaligned
errors, and sometimes leaves the pc in the debug dection of memory). I
*can* get it to break in the otp (0x88000000), so perhaps I just need to
identify the address before it jumps to U-Boot.

>> To see if this way can pin down which instruction or the crucial code
>> to cause the bus hang problem. And guess what maybe the root-cause.
>>
>> If you can find the instruction which may cause the bus hang, you can
>> info all-registers and compare the differences between NG and OK. And
>> guess what maybe the root-cause.
> 
> Trying to narrow down on the problem I found the following:
> 
> The system hangs before arch_cpu_init_dm() is called.

This is not always the case. On most boots, the following output is
present:

U-Boot 2020.10-rc3-00045-g7532b003f0 (Sep 02 2020 - 11:09:16 -0400)

DRAM:  8 MiB

which means at least everything up to dram_init gets called.

> 
> After adding some debug functions the error appeared and disappeared
> when changing the code in function panic(). So my guess is that there is
> some alignment problem in the static data section.

I investigated this further using the following script

while true; do
	sed -i 's/nop();$/nop(); nop();/g' board/sipeed/maix/maix.c &&
	git commit --amend --no-edit board/sipeed/maix/maix.c &&
	CROSS_COMPILE=riscv64-linux-gnu- make -j$(nproc) &&
	kflash -tp /dev/ttyUSB0 -B bit_mic -b 1500000 u-boot-dtb.bin"
done

To start this process, create a commit which adds a nop() to
board/sipeed/maix/maix.c. On every iteration, this script will amend
that commit by adding another nop. I tried up to 65 nops. If the amount
of nops is 0, 24, 28, 29, 30, 31, 32, 40, 44, 46, 49, 56, 60, 61, 62,
63, or 64 the board fails to boot. Of these failures, all printed up to
"DRAM: ..." except for those with 28, 29, 30, 31, 60, 61, 62, or 64
nops. There is clearly a pattern whith failures occuring at or near
(but not always exactly on) multiples of 4, and in the lead-up to
multiples of 32.

My next line of investigation will be to determine the size and
alignment of the various sections based in the failing configurations. I
also plan to try enabling more debug info and see if I can trigger this
issue by adding some choice no-ops.

--Sean

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

On 9/2/20 11:59 AM, Sean Anderson wrote:
> On 9/2/20 8:26 AM, Heinrich Schuchardt wrote:
>> After adding some debug functions the error appeared and disappeared
>> when changing the code in function panic(). So my guess is that there is
>> some alignment problem in the static data section.
> 
> I investigated this further using the following script
> 
> while true; do
> 	sed -i 's/nop();$/nop(); nop();/g' board/sipeed/maix/maix.c &&
> 	git commit --amend --no-edit board/sipeed/maix/maix.c &&
> 	CROSS_COMPILE=riscv64-linux-gnu- make -j$(nproc) &&
> 	kflash -tp /dev/ttyUSB0 -B bit_mic -b 1500000 u-boot-dtb.bin"
> done
> 
> To start this process, create a commit which adds a nop() to
> board/sipeed/maix/maix.c. On every iteration, this script will amend
> that commit by adding another nop. I tried up to 65 nops. If the amount
> of nops is 0, 24, 28, 29, 30, 31, 32, 40, 44, 46, 49, 56, 60, 61, 62,
> 63, or 64 the board fails to boot. Of these failures, all printed up to
> "DRAM: ..." except for those with 28, 29, 30, 31, 60, 61, 62, or 64
> nops. There is clearly a pattern whith failures occuring at or near
> (but not always exactly on) multiples of 4, and in the lead-up to
> multiples of 32.

These patterns cause two different bugs, which I will refer to as the
"multiple-of-four" bug and the "periodic-32" bug.

The multiple-of-four bug is fixed by [1]. This bug is not present in
u-boot/master atm, but I am not sure why. Perhaps adding more drivers
which depend on the device tree triggers the behavior.

>> On 01.09.20 03:19, Rick Chen wrote:
>>> To see if this way can pin down which instruction or the crucial code
>>> to cause the bus hang problem. And guess what maybe the root-cause.
>>>
>>> If you can find the instruction which may cause the bus hang, you can
>>> info all-registers and compare the differences between NG and OK. And
>>> guess what maybe the root-cause.
>>
>> Trying to narrow down on the problem I found the following:
>>
>> The system hangs before arch_cpu_init_dm() is called.
> 
> This is not always the case. On most boots, the following output is
> present:
> 
> U-Boot 2020.10-rc3-00045-g7532b003f0 (Sep 02 2020 - 11:09:16 -0400)
> 
> DRAM:  8 MiB
> 
> which means at least everything up to dram_init gets called.

This output is symptomatic of the multiple-of-four bug. After applying
[1], either a successful boot should take place, or there should be no
output at all (corresponding to the periodic-32 bug).

>>>
>>> Maybe you can try to set a break and access the bus, if the bus access
>>> fail, then you re-set a break a bit ahead until the bus access NOT
>>> fail.
> 
> Yeah, I was investigating that, however I was unable to get the k210 to
> break at 0x80000000. I suspect this may be a problem with openocd, as
> the k210 port is rather buggy (e.g. it can cause address misaligned
> errors, and sometimes leaves the pc in the debug dection of memory). I
> *can* get it to break in the otp (0x88000000), so perhaps I just need to
> identify the address before it jumps to U-Boot.

When attempting to boot with a U-boot which has the periodic-32 bug,
there is no output, even with an early uart and debug logs enabled. I
have tried using two versions of openocd to determine the cause. The kendryte
openocd [2] supports attaching to either core of the k210, but must be
restarted to debug the other core. It also keeps the non-debugged core
halted. The riscv openocd [3] only supports debugging core 0. However,
it contains numerous bugfixes and improvements which the kendryte
openocd does not contain.

When attaching the kendryte openocd, the following output (among other
things) is printed:

Core [1] halted at 0x80015590 due to debug interrupt
Core [0] halted at 0x88008c00 due to debug interrupt

The core 1's pc is located within _sifive_serial_putc, and this is
reflected by the console containing a partially-printed announcement
for the first printed initcall. This initcall is initf_bootstage, which
is the first initcall after log_init. If openocd is attached to core 1,
and it is resumed, then U-Boot boots as normal (in contrast to the
behavior exhibited if openocd is not attached).

What is more interesting is the pc of core 0. It is located in the boot
rom of the K210. According to [4] (and as verified by myself), this
address corresponds with the uarths_getc function. This function is
only called by isp_run. This means core 0 is stuck in ISP mode, which is
used to flash firmware.

Attaching a debugger clearly affects the boot process of this core, but
I am not sure in what manner. I am attempting to modify the riscv
openocd to support multiple cores, but it is slow going (there is a lot
of undocumented code with side-effects).

--Sean

[1] https://patchwork.ozlabs.org/project/uboot/patch/20200905132211.412711-1-seanga2 at gmail.com/
[2] https://github.com/kendryte/openocd-kendryte
[3] https://github.com/riscv/riscv-openocd
[4] https://github.com/kelas/k210-sdk-stuff/blob/master/rom/k210.rom.h

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Ruinland ChuanTzu Tsai]

Hi Sean and all,

Sorry for dropping this mail out of the blue.

I'm trying to follow the instrcutions from `doc/board/sipeed/maix.rst`
to build and flash u-boot so as to verify Sean's work on `maix_gpio_good` tree.

Yet the console has no output.

Furthermore, I found the documentation on Sipeed board to be a little bit confusing.
In your first commit of that document, it's stated that "only the Sipeed MAIX BiT
V2.0 (bitm) and Sipeed MAIXDUINO are supported."
Yet the later commit (137dc15) added a table which implies that the older version
of MAIX BiT is supported. So I'm a bit confused about whether older MAIX BiTs get
supported or not ? (I'm testing the builts on the older version.)
Does the replacement of CH34x with CH552 may cause the issue I'm encountering ?

By the way, I was trying to use the pre-built toolchain from kendryte's GitHub [1].
Yet the linker (riscv64-unknown-elf-ld.bfd) complains that `-pie` is not supported.
Hence I switched to the the binutils v2.34 built from upstream and the u-boot could
be built without that hiccup.

Could someone tell me which toolchain is recommended for building the u-boot for
boards Kendryte K210 ?

[1] https://github.com/kendryte/kendryte-gnu-toolchainhttps://github.com/kendryte/kendryte-gnu-toolchain

Many thanks,
Ruinland

On Thu, Aug 20, 2020 at 02:25:36PM +0800, Rick Jian-Zhi Chen(???) wrote:
> 
> 
> -----Original Message-----
> From: Sean Anderson [mailto:seanga2 at gmail.com] 
> Sent: Wednesday, August 19, 2020 7:13 PM
> To: Rick Chen
> Cc: U-Boot Mailing List; Simon Glass; Tom Rini; Bin Meng; Rick Jian-Zhi Chen(???); Alan Quey-Liang Kao(???)
> Subject: Re: [PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210
> 
> On 8/18/20 11:48 PM, Rick Chen wrote:
> > Hi Tom
> > 
> >> This patch adds the necessary configs and docs for FPIOA and GPIO support
> >> on the K210.
> >>
> >> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
> >> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
> >> changed). It also boots when changes are made to the device tree and then
> >> committed. I don't know why this happens. These breakages only occur after
> >> bf2fb81ad3.
> >>
> >> Signed-off-by: Sean Anderson <seanga2@gmail.com>
> >> ---
> >>
> >> Changes in v5:
> >> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
> >> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
> >>   by commit 2548493ab4.
> > 
> > Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
> > FPIOA and GPIO support for Kendryte K210 ?
> > Or maybe it is better to figure out what is wrong here and find the
> > root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
> 
> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
> for the same effect. In addition, there are several other ways I found
> to "fix" this bug (as noted in the commit message). If you would like to
> test this out, I have two trees [1, 2] where this series (actually a slightly
> earlier version of this series) is applied just before and just after
> bf2fb81ad3. The original patch is located at [3].
> 
> --Sean
> 
> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/

[PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210

[Sean Anderson]

On 8/21/20 5:08 AM, Ruinland ChuanTzu Tsai wrote:
> Hi Sean and all,
> 
> Sorry for dropping this mail out of the blue.
> 
> I'm trying to follow the instrcutions from `doc/board/sipeed/maix.rst`
> to build and flash u-boot so as to verify Sean's work on `maix_gpio_good` tree.
> 
> Yet the console has no output.
> 
> Furthermore, I found the documentation on Sipeed board to be a little bit confusing.
> In your first commit of that document, it's stated that "only the Sipeed MAIX BiT
> V2.0 (bitm) and Sipeed MAIXDUINO are supported."

Those are the only boards I have, so those are all that I can support.

> Yet the later commit (137dc15) added a table which implies that the older version
> of MAIX BiT is supported. So I'm a bit confused about whether older MAIX BiTs get
> supported or not ? (I'm testing the builts on the older version.)

It *should* work, but I haven't tested anything on a bit v1. Have you
tried booting with v2020.07? That release should have the bare minimum
needed to get things working.

> Does the replacement of CH34x with CH552 may cause the issue I'm encountering ?

Probably not.

> 
> By the way, I was trying to use the pre-built toolchain from kendryte's GitHub [1].
> Yet the linker (riscv64-unknown-elf-ld.bfd) complains that `-pie` is not supported.
> Hence I switched to the the binutils v2.34 built from upstream and the u-boot could
> be built without that hiccup.

I have been using the toolchain which comes with my distro
(riscv64-linux-gnu-* on Arch). I don't know whether that makes much
difference, but I have never used Canaan's pre-build toolchain.

--Sean

> 
> Could someone tell me which toolchain is recommended for building the u-boot for
> boards Kendryte K210 ?
> 
> [1] https://github.com/kendryte/kendryte-gnu-toolchainhttps://github.com/kendryte/kendryte-gnu-toolchain
> 
> Many thanks,
> Ruinland
> 
> On Thu, Aug 20, 2020 at 02:25:36PM +0800, Rick Jian-Zhi Chen(???) wrote:
>>
>>
>> -----Original Message-----
>> From: Sean Anderson [mailto:seanga2 at gmail.com] 
>> Sent: Wednesday, August 19, 2020 7:13 PM
>> To: Rick Chen
>> Cc: U-Boot Mailing List; Simon Glass; Tom Rini; Bin Meng; Rick Jian-Zhi Chen(???); Alan Quey-Liang Kao(???)
>> Subject: Re: [PATCH v5 11/11] riscv: Add FPIOA and GPIO support for Kendryte K210
>>
>> On 8/18/20 11:48 PM, Rick Chen wrote:
>>> Hi Tom
>>>
>>>> This patch adds the necessary configs and docs for FPIOA and GPIO support
>>>> on the K210.
>>>>
>>>> The board does not boot unless CONSOLE_LOGLEVEL is set to a non-default
>>>> value . It also boots when the tree is dirty (and CONSOLE_LOGLEVEL is not
>>>> changed). It also boots when changes are made to the device tree and then
>>>> committed. I don't know why this happens. These breakages only occur after
>>>> bf2fb81ad3.
>>>>
>>>> Signed-off-by: Sean Anderson <seanga2@gmail.com>
>>>> ---
>>>>
>>>> Changes in v5:
>>>> - Increase CONSOLE_LOGLEVEL to 5 as a hack to get the board booting again
>>>> - Patch 05/12 "gpio: sifive: Use generic reg read function" has been superseded
>>>>   by commit 2548493ab4.
>>>
>>> Would you like to pick up this series, [PATCH v5 00/11] riscv: Add
>>> FPIOA and GPIO support for Kendryte K210 ?
>>> Or maybe it is better to figure out what is wrong here and find the
>>> root cause why it need to Increase CONSOLE_LOGLEVEL to 5 as a hack ?
>>
>> As an additional note, *CONFIG_LOGLEVEL (whoops) can also be decreased
>> for the same effect. In addition, there are several other ways I found
>> to "fix" this bug (as noted in the commit message). If you would like to
>> test this out, I have two trees [1, 2] where this series (actually a slightly
>> earlier version of this series) is applied just before and just after
>> bf2fb81ad3. The original patch is located at [3].
>>
>> --Sean
>>
>> [1] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_good
>> [2] https://github.com/Forty-Bot/u-boot/tree/maix_gpio_bad
>> [3] https://patchwork.ozlabs.org/project/uboot/patch/20200724111225.12513-15-ovidiu.panait at windriver.com/