pinctrl: aspeed: Split out pinmux from general pinctrl

ASPEED have completely rearranged the System Control Unit register
layout with the AST2600. The existing code took advantage of the fact
that the AST2400 and AST2500 had layouts that were similar enough to
have little impact on the pinmux infrastructure (though there is a wart
with read-modify-write vs write-1-clear semantics of the hardware
strapping registers between the two).

Given that any similarity has been thrown out with the AST2600, separate
out the function applying an expression state to be driver-specific.
With it, extract out the pinmux macro jungle to its own header and
implementation so the pieces can be composed without dependency cycles.

Cc: Johnny Huang <johnny_huang@aspeedtech.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Link: https://lore.kernel.org/r/20190628023838.15426-8-andrew@aj.id.au
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

7 files changed, 888 insertions, 255 deletions

diff --git a/drivers/pinctrl/aspeed/Makefile b/drivers/pinctrl/aspeed/Makefile
index 790b406aae19..3a94e4db0c8e 100644
--- a/drivers/pinctrl/aspeed/Makefile
+++ b/drivers/pinctrl/aspeed/Makefile
@@ -1,6 +1,6 @@
 # Aspeed pinctrl support
 
 ccflags-y += $(call cc-option,-Woverride-init)
-obj-$(CONFIG_PINCTRL_ASPEED)	+= pinctrl-aspeed.o
+obj-$(CONFIG_PINCTRL_ASPEED)	+= pinctrl-aspeed.o pinmux-aspeed.o
 obj-$(CONFIG_PINCTRL_ASPEED_G4)	+= pinctrl-aspeed-g4.o
 obj-$(CONFIG_PINCTRL_ASPEED_G5)	+= pinctrl-aspeed-g5.o
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
index 05b153034517..37f325fb5d7c 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g4.c
@@ -22,9 +22,35 @@
 
 #include "../core.h"
 #include "../pinctrl-utils.h"
+#include "pinmux-aspeed.h"
 #include "pinctrl-aspeed.h"
 
 /*
+ * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
+ * references registers by the device/offset mnemonic. The register macros
+ * below are named the same way to ease transcription and verification (as
+ * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
+ * reference registers beyond those dedicated to pinmux, such as the system
+ * reset control and MAC clock configuration registers.
+ */
+#define SCU2C           0x2C /* Misc. Control Register */
+#define SCU3C           0x3C /* System Reset Control/Status Register */
+#define SCU48           0x48 /* MAC Interface Clock Delay Setting */
+#define HW_STRAP1       0x70 /* AST2400 strapping is 33 bits, is split */
+#define HW_REVISION_ID  0x7C /* Silicon revision ID register */
+#define SCU80           0x80 /* Multi-function Pin Control #1 */
+#define SCU84           0x84 /* Multi-function Pin Control #2 */
+#define SCU88           0x88 /* Multi-function Pin Control #3 */
+#define SCU8C           0x8C /* Multi-function Pin Control #4 */
+#define SCU90           0x90 /* Multi-function Pin Control #5 */
+#define SCU94           0x94 /* Multi-function Pin Control #6 */
+#define SCUA0           0xA0 /* Multi-function Pin Control #7 */
+#define SCUA4           0xA4 /* Multi-function Pin Control #8 */
+#define SCUA8           0xA8 /* Multi-function Pin Control #9 */
+#define SCUAC           0xAC /* Multi-function Pin Control #10 */
+#define HW_STRAP2       0xD0 /* Strapping */
+
+/*
  * Uses undefined macros for symbol naming and references, eg GPIOA0, MAC1LINK,
  * TIMER3 etc.
  *
@@ -2390,13 +2416,73 @@ static const struct aspeed_pin_config aspeed_g4_configs[] = {
 	{ PIN_CONFIG_INPUT_DEBOUNCE, { C14, B14 }, SCUA8, 27 },
 };
 
+static int aspeed_g4_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+				  const struct aspeed_sig_expr *expr,
+				  bool enable)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < expr->ndescs; i++) {
+		const struct aspeed_sig_desc *desc = &expr->descs[i];
+		u32 pattern = enable ? desc->enable : desc->disable;
+		u32 val = (pattern << __ffs(desc->mask));
+
+		if (!ctx->maps[desc->ip])
+			return -ENODEV;
+
+		/*
+		 * Strap registers are configured in hardware or by early-boot
+		 * firmware. Treat them as read-only despite that we can write
+		 * them. This may mean that certain functions cannot be
+		 * deconfigured and is the reason we re-evaluate after writing
+		 * all descriptor bits.
+		 *
+		 * Port D and port E GPIO loopback modes are the only exception
+		 * as those are commonly used with front-panel buttons to allow
+		 * normal operation of the host when the BMC is powered off or
+		 * fails to boot. Once the BMC has booted, the loopback mode
+		 * must be disabled for the BMC to control host power-on and
+		 * reset.
+		 */
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+		    !(desc->mask & (BIT(21) | BIT(22))))
+			continue;
+
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
+			continue;
+
+		ret = regmap_update_bits(ctx->maps[desc->ip], desc->reg,
+					 desc->mask, val);
+
+		if (ret)
+			return ret;
+	}
+
+	ret = aspeed_sig_expr_eval(ctx, expr, enable);
+	if (ret < 0)
+		return ret;
+
+	if (!ret)
+		return -EPERM;
+
+	return 0;
+}
+
+static const struct aspeed_pinmux_ops aspeed_g4_ops = {
+	.set = aspeed_g4_sig_expr_set,
+};
+
 static struct aspeed_pinctrl_data aspeed_g4_pinctrl_data = {
 	.pins = aspeed_g4_pins,
 	.npins = ARRAY_SIZE(aspeed_g4_pins),
-	.groups = aspeed_g4_groups,
-	.ngroups = ARRAY_SIZE(aspeed_g4_groups),
-	.functions = aspeed_g4_functions,
-	.nfunctions = ARRAY_SIZE(aspeed_g4_functions),
+	.pinmux = {
+		.ops = &aspeed_g4_ops,
+		.groups = aspeed_g4_groups,
+		.ngroups = ARRAY_SIZE(aspeed_g4_groups),
+		.functions = aspeed_g4_functions,
+		.nfunctions = ARRAY_SIZE(aspeed_g4_functions),
+	},
 	.configs = aspeed_g4_configs,
 	.nconfigs = ARRAY_SIZE(aspeed_g4_configs),
 };
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
index 0c89647f166f..ec48c59e84ae 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g5.c
@@ -25,6 +25,32 @@
 #include "../pinctrl-utils.h"
 #include "pinctrl-aspeed.h"
 
+/*
+ * The "Multi-function Pins Mapping and Control" table in the SoC datasheet
+ * references registers by the device/offset mnemonic. The register macros
+ * below are named the same way to ease transcription and verification (as
+ * opposed to naming them e.g. PINMUX_CTRL_[0-9]). Further, signal expressions
+ * reference registers beyond those dedicated to pinmux, such as the system
+ * reset control and MAC clock configuration registers. The AST2500 goes a step
+ * further and references registers in the graphics IP block.
+ */
+#define SCU2C           0x2C /* Misc. Control Register */
+#define SCU3C           0x3C /* System Reset Control/Status Register */
+#define SCU48           0x48 /* MAC Interface Clock Delay Setting */
+#define HW_STRAP1       0x70 /* AST2400 strapping is 33 bits, is split */
+#define HW_REVISION_ID  0x7C /* Silicon revision ID register */
+#define SCU80           0x80 /* Multi-function Pin Control #1 */
+#define SCU84           0x84 /* Multi-function Pin Control #2 */
+#define SCU88           0x88 /* Multi-function Pin Control #3 */
+#define SCU8C           0x8C /* Multi-function Pin Control #4 */
+#define SCU90           0x90 /* Multi-function Pin Control #5 */
+#define SCU94           0x94 /* Multi-function Pin Control #6 */
+#define SCUA0           0xA0 /* Multi-function Pin Control #7 */
+#define SCUA4           0xA4 /* Multi-function Pin Control #8 */
+#define SCUA8           0xA8 /* Multi-function Pin Control #9 */
+#define SCUAC           0xAC /* Multi-function Pin Control #10 */
+#define HW_STRAP2       0xD0 /* Strapping */
+
 #define ASPEED_G5_NR_PINS 236
 
 #define COND1		{ ASPEED_IP_SCU, SCU90, BIT(6), 0, 0 }
@@ -2485,13 +2511,98 @@ static struct aspeed_pin_config aspeed_g5_configs[] = {
 	{ PIN_CONFIG_INPUT_DEBOUNCE, { A20, B19 }, SCUA8, 27 },
 };
 
+/**
+ * Configure a pin's signal by applying an expression's descriptor state for
+ * all descriptors in the expression.
+ *
+ * @ctx: The pinmux context
+ * @expr: The expression associated with the function whose signal is to be
+ *        configured
+ * @enable: true to enable an function's signal through a pin's signal
+ *          expression, false to disable the function's signal
+ *
+ * Return: 0 if the expression is configured as requested and a negative error
+ * code otherwise
+ */
+static int aspeed_g5_sig_expr_set(const struct aspeed_pinmux_data *ctx,
+				  const struct aspeed_sig_expr *expr,
+				  bool enable)
+{
+	int ret;
+	int i;
+
+	for (i = 0; i < expr->ndescs; i++) {
+		const struct aspeed_sig_desc *desc = &expr->descs[i];
+		u32 pattern = enable ? desc->enable : desc->disable;
+		u32 val = (pattern << __ffs(desc->mask));
+
+		if (!ctx->maps[desc->ip])
+			return -ENODEV;
+
+		/*
+		 * Strap registers are configured in hardware or by early-boot
+		 * firmware. Treat them as read-only despite that we can write
+		 * them. This may mean that certain functions cannot be
+		 * deconfigured and is the reason we re-evaluate after writing
+		 * all descriptor bits.
+		 *
+		 * Port D and port E GPIO loopback modes are the only exception
+		 * as those are commonly used with front-panel buttons to allow
+		 * normal operation of the host when the BMC is powered off or
+		 * fails to boot. Once the BMC has booted, the loopback mode
+		 * must be disabled for the BMC to control host power-on and
+		 * reset.
+		 */
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
+		    !(desc->mask & (BIT(21) | BIT(22))))
+			continue;
+
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
+			continue;
+
+		/* On AST2500, Set bits in SCU70 are cleared from SCU7C */
+		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
+			u32 value = ~val & desc->mask;
+
+			if (value) {
+				ret = regmap_write(ctx->maps[desc->ip],
+						   HW_REVISION_ID, value);
+				if (ret < 0)
+					return ret;
+			}
+		}
+
+		ret = regmap_update_bits(ctx->maps[desc->ip], desc->reg,
+					 desc->mask, val);
+
+		if (ret)
+			return ret;
+	}
+
+	ret = aspeed_sig_expr_eval(ctx, expr, enable);
+	if (ret < 0)
+		return ret;
+
+	if (!ret)
+		return -EPERM;
+
+	return 0;
+}
+
+static const struct aspeed_pinmux_ops aspeed_g5_ops = {
+	.set = aspeed_g5_sig_expr_set,
+};
+
 static struct aspeed_pinctrl_data aspeed_g5_pinctrl_data = {
 	.pins = aspeed_g5_pins,
 	.npins = ARRAY_SIZE(aspeed_g5_pins),
-	.groups = aspeed_g5_groups,
-	.ngroups = ARRAY_SIZE(aspeed_g5_groups),
-	.functions = aspeed_g5_functions,
-	.nfunctions = ARRAY_SIZE(aspeed_g5_functions),
+	.pinmux = {
+		.ops = &aspeed_g5_ops,
+		.groups = aspeed_g5_groups,
+		.ngroups = ARRAY_SIZE(aspeed_g5_groups),
+		.functions = aspeed_g5_functions,
+		.nfunctions = ARRAY_SIZE(aspeed_g5_functions),
+	},
 	.configs = aspeed_g5_configs,
 	.nconfigs = ARRAY_SIZE(aspeed_g5_configs),
 };
@@ -2547,7 +2658,7 @@ static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
 		dev_warn(&pdev->dev, "No GFX phandle found, some mux configurations may fail\n");
 		map = NULL;
 	}
-	aspeed_g5_pinctrl_data.maps[ASPEED_IP_GFX] = map;
+	aspeed_g5_pinctrl_data.pinmux.maps[ASPEED_IP_GFX] = map;
 
 	node = of_parse_phandle(pdev->dev.of_node, "aspeed,external-nodes", 1);
 	if (node) {
@@ -2561,7 +2672,7 @@ static int aspeed_g5_pinctrl_probe(struct platform_device *pdev)
 		map = NULL;
 	}
 	of_node_put(node);
-	aspeed_g5_pinctrl_data.maps[ASPEED_IP_LPC] = map;
+	aspeed_g5_pinctrl_data.pinmux.maps[ASPEED_IP_LPC] = map;
 
 	return aspeed_pinctrl_probe(pdev, &aspeed_g5_pinctrl_desc,
 			&aspeed_g5_pinctrl_data);
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
index 89cf61b764cb..0a162ca24b9a 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
@@ -14,17 +14,11 @@
 #include "../core.h"
 #include "pinctrl-aspeed.h"
 
-static const char *const aspeed_pinmux_ips[] = {
-	[ASPEED_IP_SCU] = "SCU",
-	[ASPEED_IP_GFX] = "GFX",
-	[ASPEED_IP_LPC] = "LPC",
-};
-
 int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev)
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	return pdata->ngroups;
+	return pdata->pinmux.ngroups;
 }
 
 const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
@@ -32,7 +26,7 @@ const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	return pdata->groups[group].name;
+	return pdata->pinmux.groups[group].name;
 }
 
 int aspeed_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
@@ -41,8 +35,8 @@ int aspeed_pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	*pins = &pdata->groups[group].pins[0];
-	*npins = pdata->groups[group].npins;
+	*pins = &pdata->pinmux.groups[group].pins[0];
+	*npins = pdata->pinmux.groups[group].npins;
 
 	return 0;
 }
@@ -57,7 +51,7 @@ int aspeed_pinmux_get_fn_count(struct pinctrl_dev *pctldev)
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	return pdata->nfunctions;
+	return pdata->pinmux.nfunctions;
 }
 
 const char *aspeed_pinmux_get_fn_name(struct pinctrl_dev *pctldev,
@@ -65,7 +59,7 @@ const char *aspeed_pinmux_get_fn_name(struct pinctrl_dev *pctldev,
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	return pdata->functions[function].name;
+	return pdata->pinmux.functions[function].name;
 }
 
 int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
@@ -75,208 +69,38 @@ int aspeed_pinmux_get_fn_groups(struct pinctrl_dev *pctldev,
 {
 	struct aspeed_pinctrl_data *pdata = pinctrl_dev_get_drvdata(pctldev);
 
-	*groups = pdata->functions[function].groups;
-	*num_groups = pdata->functions[function].ngroups;
+	*groups = pdata->pinmux.functions[function].groups;
+	*num_groups = pdata->pinmux.functions[function].ngroups;
 
 	return 0;
 }
 
-static inline void aspeed_sig_desc_print_val(
-		const struct aspeed_sig_desc *desc, bool enable, u32 rv)
-{
-	pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
-			aspeed_pinmux_ips[desc->ip], desc->reg,
-			desc->mask, enable ? desc->enable : desc->disable,
-			(rv & desc->mask) >> __ffs(desc->mask), rv);
-}
-
-/**
- * Query the enabled or disabled state of a signal descriptor
- *
- * @desc: The signal descriptor of interest
- * @enabled: True to query the enabled state, false to query disabled state
- * @map: The IP block's regmap instance
- *
- * Return: 1 if the descriptor's bitfield is configured to the state
- * selected by @enabled, 0 if not, and less than zero if an unrecoverable
- * failure occurred
- *
- * Evaluation of descriptor state is non-trivial in that it is not a binary
- * outcome: The bitfields can be greater than one bit in size and thus can take
- * a value that is neither the enabled nor disabled state recorded in the
- * descriptor (typically this means a different function to the one of interest
- * is enabled). Thus we must explicitly test for either condition as required.
- */
-static int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
-				 bool enabled, struct regmap *map)
+static int aspeed_sig_expr_enable(const struct aspeed_pinmux_data *ctx,
+				  const struct aspeed_sig_expr *expr)
 {
 	int ret;
-	unsigned int raw;
-	u32 want;
 
-	if (!map)
-		return -ENODEV;
-
-	ret = regmap_read(map, desc->reg, &raw);
-	if (ret)
-		return ret;
-
-	aspeed_sig_desc_print_val(desc, enabled, raw);
-	want = enabled ? desc->enable : desc->disable;
-
-	return ((raw & desc->mask) >> __ffs(desc->mask)) == want;
-}
-
-/**
- * Query the enabled or disabled state for a mux function's signal on a pin
- *
- * @expr: An expression controlling the signal for a mux function on a pin
- * @enabled: True to query the enabled state, false to query disabled state
- * @maps: The list of regmap instances
- *
- * Return: 1 if the expression composed by @enabled evaluates true, 0 if not,
- * and less than zero if an unrecoverable failure occurred.
- *
- * A mux function is enabled or disabled if the function's signal expression
- * for each pin in the function's pin group evaluates true for the desired
- * state. An signal expression evaluates true if all of its associated signal
- * descriptors evaluate true for the desired state.
- *
- * If an expression's state is described by more than one bit, either through
- * multi-bit bitfields in a single signal descriptor or through multiple signal
- * descriptors of a single bit then it is possible for the expression to be in
- * neither the enabled nor disabled state. Thus we must explicitly test for
- * either condition as required.
- */
-static int aspeed_sig_expr_eval(const struct aspeed_sig_expr *expr,
-				 bool enabled, struct regmap * const *maps)
-{
-	int i;
-	int ret;
-
-	for (i = 0; i < expr->ndescs; i++) {
-		const struct aspeed_sig_desc *desc = &expr->descs[i];
-
-		ret = aspeed_sig_desc_eval(desc, enabled, maps[desc->ip]);
-		if (ret <= 0)
-			return ret;
-	}
-
-	return 1;
-}
-
-/**
- * Configure a pin's signal by applying an expression's descriptor state for
- * all descriptors in the expression.
- *
- * @expr: The expression associated with the function whose signal is to be
- *        configured
- * @enable: true to enable an function's signal through a pin's signal
- *          expression, false to disable the function's signal
- * @maps: The list of regmap instances for pinmux register access.
- *
- * Return: 0 if the expression is configured as requested and a negative error
- * code otherwise
- */
-static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
-				bool enable, struct regmap * const *maps)
-{
-	int ret;
-	int i;
-
-	for (i = 0; i < expr->ndescs; i++) {
-		const struct aspeed_sig_desc *desc = &expr->descs[i];
-		u32 pattern = enable ? desc->enable : desc->disable;
-		u32 val = (pattern << __ffs(desc->mask));
-
-		if (!maps[desc->ip])
-			return -ENODEV;
-
-		/*
-		 * Strap registers are configured in hardware or by early-boot
-		 * firmware. Treat them as read-only despite that we can write
-		 * them. This may mean that certain functions cannot be
-		 * deconfigured and is the reason we re-evaluate after writing
-		 * all descriptor bits.
-		 *
-		 * Port D and port E GPIO loopback modes are the only exception
-		 * as those are commonly used with front-panel buttons to allow
-		 * normal operation of the host when the BMC is powered off or
-		 * fails to boot. Once the BMC has booted, the loopback mode
-		 * must be disabled for the BMC to control host power-on and
-		 * reset.
-		 */
-		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1 &&
-		    !(desc->mask & (BIT(21) | BIT(22))))
-			continue;
-
-		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
-			continue;
-
-		/* On AST2500, Set bits in SCU70 are cleared from SCU7C */
-		if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
-			unsigned int rev_id;
-
-			ret = regmap_read(maps[ASPEED_IP_SCU],
-				HW_REVISION_ID, &rev_id);
-			if (ret < 0)
-				return ret;
-
-			if (0x04 == (rev_id >> 24)) {
-				u32 value = ~val & desc->mask;
-
-				if (value) {
-					ret = regmap_write(maps[desc->ip],
-						HW_REVISION_ID, value);
-					if (ret < 0)
-						return ret;
-				}
-			}
-		}
-
-		ret = regmap_update_bits(maps[desc->ip], desc->reg,
-					 desc->mask, val);
-
-		if (ret)
-			return ret;
-	}
-
-	ret = aspeed_sig_expr_eval(expr, enable, maps);
-	if (ret < 0)
-		return ret;
-
-	if (!ret)
-		return -EPERM;
-
-	return 0;
-}
-
-static int aspeed_sig_expr_enable(const struct aspeed_sig_expr *expr,
-				   struct regmap * const *maps)
-{
-	int ret;
-
-	ret = aspeed_sig_expr_eval(expr, true, maps);
+	ret = aspeed_sig_expr_eval(ctx, expr, true);
 	if (ret < 0)
 		return ret;
 
 	if (!ret)
-		return aspeed_sig_expr_set(expr, true, maps);
+		return aspeed_sig_expr_set(ctx, expr, true);
 
 	return 0;
 }
 
-static int aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
-				    struct regmap * const *maps)
+static int aspeed_sig_expr_disable(const struct aspeed_pinmux_data *ctx,
+				   const struct aspeed_sig_expr *expr)
 {
 	int ret;
 
-	ret = aspeed_sig_expr_eval(expr, true, maps);
+	ret = aspeed_sig_expr_eval(ctx, expr, true);
 	if (ret < 0)
 		return ret;
 
 	if (ret)
-		return aspeed_sig_expr_set(expr, false, maps);
+		return aspeed_sig_expr_set(ctx, expr, false);
 
 	return 0;
 }
@@ -284,13 +108,13 @@ static int aspeed_sig_expr_disable(const struct aspeed_sig_expr *expr,
 /**
  * Disable a signal on a pin by disabling all provided signal expressions.
  *
+ * @ctx: The pinmux context
  * @exprs: The list of signal expressions (from a priority level on a pin)
- * @maps: The list of regmap instances for pinmux register access.
  *
  * Return: 0 if all expressions are disabled, otherwise a negative error code
  */
-static int aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
-			       struct regmap * const *maps)
+static int aspeed_disable_sig(const struct aspeed_pinmux_data *ctx,
+			      const struct aspeed_sig_expr **exprs)
 {
 	int ret = 0;
 
@@ -298,7 +122,7 @@ static int aspeed_disable_sig(const struct aspeed_sig_expr **exprs,
 		return true;
 
 	while (*exprs && !ret) {
-		ret = aspeed_sig_expr_disable(*exprs, maps);
+		ret = aspeed_sig_expr_disable(ctx, *exprs);
 		exprs++;
 	}
 
@@ -399,9 +223,9 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 	int ret;
 	const struct aspeed_pinctrl_data *pdata =
 		pinctrl_dev_get_drvdata(pctldev);
-	const struct aspeed_pin_group *pgroup = &pdata->groups[group];
+	const struct aspeed_pin_group *pgroup = &pdata->pinmux.groups[group];
 	const struct aspeed_pin_function *pfunc =
-		&pdata->functions[function];
+		&pdata->pinmux.functions[function];
 
 	for (i = 0; i < pgroup->npins; i++) {
 		int pin = pgroup->pins[i];
@@ -427,7 +251,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 			if (expr)
 				break;
 
-			ret = aspeed_disable_sig(funcs, pdata->maps);
+			ret = aspeed_disable_sig(&pdata->pinmux, funcs);
 			if (ret)
 				return ret;
 
@@ -447,7 +271,7 @@ int aspeed_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned int function,
 			return -ENXIO;
 		}
 
-		ret = aspeed_sig_expr_enable(expr, pdata->maps);
+		ret = aspeed_sig_expr_enable(&pdata->pinmux, expr);
 		if (ret)
 			return ret;
 	}
@@ -504,7 +328,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
 		if (aspeed_gpio_in_exprs(funcs))
 			break;
 
-		ret = aspeed_disable_sig(funcs, pdata->maps);
+		ret = aspeed_disable_sig(&pdata->pinmux, funcs);
 		if (ret)
 			return ret;
 
@@ -535,7 +359,7 @@ int aspeed_gpio_request_enable(struct pinctrl_dev *pctldev,
 	 * If GPIO is not the lowest priority signal type, assume there is only
 	 * one expression defined to enable the GPIO function
 	 */
-	return aspeed_sig_expr_enable(expr, pdata->maps);
+	return aspeed_sig_expr_enable(&pdata->pinmux, expr);
 }
 
 int aspeed_pinctrl_probe(struct platform_device *pdev,
@@ -551,12 +375,14 @@ int aspeed_pinctrl_probe(struct platform_device *pdev,
 		return -ENODEV;
 	}
 
-	pdata->maps[ASPEED_IP_SCU] = syscon_node_to_regmap(parent->of_node);
-	if (IS_ERR(pdata->maps[ASPEED_IP_SCU])) {
+	pdata->scu = syscon_node_to_regmap(parent->of_node);
+	if (IS_ERR(pdata->scu)) {
 		dev_err(&pdev->dev, "No regmap for syscon pincontroller parent\n");
-		return PTR_ERR(pdata->maps[ASPEED_IP_SCU]);
+		return PTR_ERR(pdata->scu);
 	}
 
+	pdata->pinmux.maps[ASPEED_IP_SCU] = pdata->scu;
+
 	pctl = pinctrl_register(pdesc, &pdev->dev, pdata);
 
 	if (IS_ERR(pctl)) {
@@ -591,7 +417,9 @@ static inline const struct aspeed_pin_config *find_pinconf_config(
 	return NULL;
 }
 
-/**
+/*
+ * Aspeed pin configuration description.
+ *
  * @param: pinconf configuration parameter
  * @arg: The supported argument for @param, or -1 if any value is supported
  * @val: The register value to write to configure @arg for @param
@@ -665,7 +493,7 @@ int aspeed_pin_config_get(struct pinctrl_dev *pctldev, unsigned int offset,
 	if (!pconf)
 		return -ENOTSUPP;
 
-	rc = regmap_read(pdata->maps[ASPEED_IP_SCU], pconf->reg, &val);
+	rc = regmap_read(pdata->scu, pconf->reg, &val);
 	if (rc < 0)
 		return rc;
 
@@ -720,8 +548,8 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset,
 
 		val = pmap->val << pconf->bit;
 
-		rc = regmap_update_bits(pdata->maps[ASPEED_IP_SCU], pconf->reg,
-				BIT(pconf->bit), val);
+		rc = regmap_update_bits(pdata->scu, pconf->reg,
+					BIT(pconf->bit), val);
 
 		if (rc < 0)
 			return rc;
diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
index f3ee28a24d8c..9b20b1c03802 100644
--- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
+++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
@@ -528,22 +528,6 @@ struct aspeed_pin_config {
 	u8 value;
 };
 
-struct aspeed_pinctrl_data {
-	struct regmap *maps[ASPEED_NR_PINMUX_IPS];
-
-	const struct pinctrl_pin_desc *pins;
-	const unsigned int npins;
-
-	const struct aspeed_pin_group *groups;
-	const unsigned int ngroups;
-
-	const struct aspeed_pin_function *functions;
-	const unsigned int nfunctions;
-
-	const struct aspeed_pin_config *configs;
-	const unsigned int nconfigs;
-};
-
 #define ASPEED_PINCTRL_PIN(name_) \
 	[name_] = { \
 		.number = name_, \
@@ -551,30 +535,19 @@ struct aspeed_pinctrl_data {
 		.drv_data = (void *) &(PIN_SYM(name_)) \
 	}
 
-struct aspeed_pin_group {
-	const char *name;
-	const unsigned int *pins;
+struct aspeed_pinctrl_data {
+	struct regmap *scu;
+
+	const struct pinctrl_pin_desc *pins;
 	const unsigned int npins;
-};
 
-#define ASPEED_PINCTRL_GROUP(name_) { \
-	.name = #name_, \
-	.pins = &(PIN_GROUP_SYM(name_))[0], \
-	.npins = ARRAY_SIZE(PIN_GROUP_SYM(name_)), \
-}
+	const struct aspeed_pin_config *configs;
+	const unsigned int nconfigs;
 
-struct aspeed_pin_function {
-	const char *name;
-	const char *const *groups;
-	unsigned int ngroups;
+	struct aspeed_pinmux_data pinmux;
 };
 
-#define ASPEED_PINCTRL_FUNC(name_, ...) { \
-	.name = #name_, \
-	.groups = &FUNC_GROUP_SYM(name_)[0], \
-	.ngroups = ARRAY_SIZE(FUNC_GROUP_SYM(name_)), \
-}
-
+/* Aspeed pinctrl helpers */
 int aspeed_pinctrl_get_groups_count(struct pinctrl_dev *pctldev);
 const char *aspeed_pinctrl_get_group_name(struct pinctrl_dev *pctldev,
 		unsigned int group);
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.c b/drivers/pinctrl/aspeed/pinmux-aspeed.c
new file mode 100644
index 000000000000..5b0fe178ccf2
--- /dev/null
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.c
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (C) 2019 IBM Corp. */
+
+/* Pieces to enable drivers to implement the .set callback */
+
+#include "pinmux-aspeed.h"
+
+const char *const aspeed_pinmux_ips[] = {
+	[ASPEED_IP_SCU] = "SCU",
+	[ASPEED_IP_GFX] = "GFX",
+	[ASPEED_IP_LPC] = "LPC",
+};
+
+static inline void aspeed_sig_desc_print_val(
+		const struct aspeed_sig_desc *desc, bool enable, u32 rv)
+{
+	pr_debug("Want %s%X[0x%08X]=0x%X, got 0x%X from 0x%08X\n",
+			aspeed_pinmux_ips[desc->ip], desc->reg,
+			desc->mask, enable ? desc->enable : desc->disable,
+			(rv & desc->mask) >> __ffs(desc->mask), rv);
+}
+
+/**
+ * Query the enabled or disabled state of a signal descriptor
+ *
+ * @desc: The signal descriptor of interest
+ * @enabled: True to query the enabled state, false to query disabled state
+ * @map: The IP block's regmap instance
+ *
+ * Return: 1 if the descriptor's bitfield is configured to the state
+ * selected by @enabled, 0 if not, and less than zero if an unrecoverable
+ * failure occurred
+ *
+ * Evaluation of descriptor state is non-trivial in that it is not a binary
+ * outcome: The bitfields can be greater than one bit in size and thus can take
+ * a value that is neither the enabled nor disabled state recorded in the
+ * descriptor (typically this means a different function to the one of interest
+ * is enabled). Thus we must explicitly test for either condition as required.
+ */
+int aspeed_sig_desc_eval(const struct aspeed_sig_desc *desc,
+			 bool enabled, struct regmap *map)
+{
+	int ret;
+	unsigned int raw;
+	u32 want;
+
+	if (!map)
+		return -ENODEV;
+
+	ret = regmap_read(map, desc->reg, &raw);
+	if (ret)
+		return ret;
+
+	aspeed_sig_desc_print_val(desc, enabled, raw);
+	want = enabled ? desc->enable : desc->disable;
+
+	return ((raw & desc->mask) >> __ffs(desc->mask)) == want;
+}
+
+/**
+ * Query the enabled or disabled state for a mux function's signal on a pin
+ *
+ * @ctx: The driver context for the pinctrl IP
+ * @expr: An expression controlling the signal for a mux function on a pin
+ * @enabled: True to query the enabled state, false to query disabled state
+ *
+ * Return: 1 if the expression composed by @enabled evaluates true, 0 if not,
+ * and less than zero if an unrecoverable failure occurred.
+ *
+ * A mux function is enabled or disabled if the function's signal expression
+ * for each pin in the function's pin group evaluates true for the desired
+ * state. An signal expression evaluates true if all of its associated signal
+ * descriptors evaluate true for the desired state.
+ *
+ * If an expression's state is described by more than one bit, either through
+ * multi-bit bitfields in a single signal descriptor or through multiple signal
+ * descriptors of a single bit then it is possible for the expression to be in
+ * neither the enabled nor disabled state. Thus we must explicitly test for
+ * either condition as required.
+ */
+int aspeed_sig_expr_eval(const struct aspeed_pinmux_data *ctx,
+			 const struct aspeed_sig_expr *expr, bool enabled)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < expr->ndescs; i++) {
+		const struct aspeed_sig_desc *desc = &expr->descs[i];
+
+		ret = aspeed_sig_desc_eval(desc, enabled, ctx->maps[desc->ip]);
+		if (ret <= 0)
+			return ret;
+	}
+
+	return 1;
+}
diff --git a/drivers/pinctrl/aspeed/pinmux-aspeed.h b/drivers/pinctrl/aspeed/pinmux-aspeed.h
new file mode 100644
index 000000000000..a036ce8f1571
--- /dev/null
+++ b/drivers/pinctrl/aspeed/pinmux-aspeed.h
@@ -0,0 +1,539 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/* Copyright (C) 2019 IBM Corp.  */
+
+#ifndef ASPEED_PINMUX_H
+#define ASPEED_PINMUX_H
+
+#include <linux/regmap.h>
+#include <stdbool.h>
+
+/*
+ * The ASPEED SoCs provide typically more than 200 pins for GPIO and other
+ * functions. The SoC function enabled on a pin is determined on a priority
+ * basis where a given pin can provide a number of different signal types.
+ *
+ * The signal active on a pin is described by both a priority level and
+ * compound logical expressions involving multiple operators, registers and
+ * bits. Some difficulty arises as the pin's function bit masks for each
+ * priority level are frequently not the same (i.e. cannot just flip a bit to
+ * change from a high to low priority signal), or even in the same register.
+ * Further, not all signals can be unmuxed, as some expressions depend on
+ * values in the hardware strapping register (which is treated as read-only).
+ *
+ * SoC Multi-function Pin Expression Examples
+ * ------------------------------------------
+ *
+ * Here are some sample mux configurations from the AST2400 and AST2500
+ * datasheets to illustrate the corner cases, roughly in order of least to most
+ * corner. The signal priorities are in decending order from P0 (highest).
+ *
+ * D6 is a pin with a single function (beside GPIO); a high priority signal
+ * that participates in one function:
+ *
+ * Ball | Default | P0 Signal | P0 Expression               | P1 Signal | P1 Expression | Other
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *  D6    GPIOA0    MAC1LINK    SCU80[0]=1                                                GPIOA0
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * C5 is a multi-signal pin (high and low priority signals). Here we touch
+ * different registers for the different functions that enable each signal:
+ *
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *  C5    GPIOA4    SCL9        SCU90[22]=1                   TIMER5      SCU80[4]=1      GPIOA4
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * E19 is a single-signal pin with two functions that influence the active
+ * signal. In this case both bits have the same meaning - enable a dedicated
+ * LPC reset pin. However it's not always the case that the bits in the
+ * OR-relationship have the same meaning.
+ *
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *  E19   GPIOB4    LPCRST#     SCU80[12]=1 | Strap[14]=1                                 GPIOB4
+ * -----+---------+-----------+-----------------------------+-----------+---------------+----------
+ *
+ * For example, pin B19 has a low-priority signal that's enabled by two
+ * distinct SoC functions: A specific SIOPBI bit in register SCUA4, and an ACPI
+ * bit in the STRAP register. The ACPI bit configures signals on pins in
+ * addition to B19. Both of the low priority functions as well as the high
+ * priority function must be disabled for GPIOF1 to be used.
+ *
+ * Ball | Default | P0 Signal | P0 Expression                           | P1 Signal | P1 Expression                          | Other
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ *  B19   GPIOF1    NDCD4       SCU80[25]=1                               SIOPBI#     SCUA4[12]=1 | Strap[19]=0                GPIOF1
+ * -----+---------+-----------+-----------------------------------------+-----------+----------------------------------------+----------
+ *
+ * For pin E18, the SoC ANDs the expected state of three bits to determine the
+ * pin's active signal:
+ *
+ * * SCU3C[3]: Enable external SOC reset function
+ * * SCU80[15]: Enable SPICS1# or EXTRST# function pin
+ * * SCU90[31]: Select SPI interface CS# output
+ *