[PATCH v2 01/10] pinctrl: Add pinmux property support to pinctrl-generic

Sean Anderson
2020-06-08T01:26:42Z

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>
---

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.26.2