[PATCH 0/6] arm64: a37xx: pinctrl: Fix requesting GPIOs and pinmux command

[PATCH 0/6] arm64: a37xx: pinctrl: Fix requesting GPIOs and pinmux command

[Pali Rohár]

This patch series cleanup pinctrl-armada-37xx.c driver, add missing pin
muxes into the list to allow usage all MPP pins as GPIOs, implement
gpio_request_enable callback for correctly setting MPP pins to GPIO
mode.

For debugging purposes are implemented also get_pins_count, get_pin_name
and get_pin_muxing functions which are required for U-Boot command:
pinmux status -a

Pali Rohár (6):
  arm64: a37xx: pinctrl: Remove unused grp->pins fields
  arm64: a37xx: pinctrl: Remove duplicate info->groups and info->ngroups
    fields
  arm64: a37xx: pinctrl: Mark all functions and structures as static
  arm64: a37xx: pinctrl: Add missing pinmuxes into the list
  arm64: a37xx: pinctrl: Implement gpio_request_enable for gpio
    functionality
  arm64: a37xx: pinctrl: Implement get_pins_count, get_pin_name and
    get_pin_muxing functions

 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 189 ++++++++++++++++----
 1 file changed, 153 insertions(+), 36 deletions(-)

-- 
2.20.1

[PATCH 1/6] arm64: a37xx: pinctrl: Remove unused grp->pins fields

[Pali Rohár]

grp->pins is just filled and never used. Remove it.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index e76ef153e604..610535fa2392 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -65,7 +65,6 @@ DECLARE_GLOBAL_DATA_PTR;
  *		belonging to the group
  * @npins:	Number of pins included in the second optional range
  * @funcs:	A list of pinmux functions that can be selected for this group.
- * @pins:	List of the pins included in the group
  */
 struct armada_37xx_pin_group {
 	const char	*name;
@@ -76,7 +75,6 @@ struct armada_37xx_pin_group {
 	unsigned int	extra_pin;
 	unsigned int	extra_npins;
 	const char	*funcs[NB_FUNCS];
-	unsigned int	*pins;
 };
 
 struct armada_37xx_pin_data {
@@ -354,19 +352,7 @@ static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info)
 
 	for (n = 0; n < info->ngroups; n++) {
 		struct armada_37xx_pin_group *grp = &info->groups[n];
-		int i, j, f;
-
-		grp->pins = devm_kzalloc(info->dev,
-					 (grp->npins + grp->extra_npins) *
-					 sizeof(*grp->pins), GFP_KERNEL);
-		if (!grp->pins)
-			return -ENOMEM;
-
-		for (i = 0; i < grp->npins; i++)
-			grp->pins[i] = grp->start_pin + i;
-
-		for (j = 0; j < grp->extra_npins; j++)
-			grp->pins[i+j] = grp->extra_pin + j;
+		int f;
 
 		for (f = 0; (f < NB_FUNCS) && grp->funcs[f]; f++) {
 			int ret;
-- 
2.20.1

[PATCH 2/6] arm64: a37xx: pinctrl: Remove duplicate info->groups and info->ngroups fields

[Pali Rohár]

They are available in pin_data structure.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 610535fa2392..e1cde53a0243 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -95,8 +95,6 @@ struct armada_37xx_pinctrl {
 	const struct armada_37xx_pin_data	*data;
 	struct udevice			*dev;
 	struct pinctrl_dev		*pctl_dev;
-	struct armada_37xx_pin_group	*groups;
-	unsigned int			ngroups;
 	struct armada_37xx_pmx_func	*funcs;
 	unsigned int			nfuncs;
 };
@@ -235,7 +233,7 @@ static int armada_37xx_pmx_get_groups_count(struct udevice *dev)
 {
 	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
 
-	return info->ngroups;
+	return info->data->ngroups;
 }
 
 static const char *armada_37xx_pmx_dummy_name = "_dummy";
@@ -245,10 +243,10 @@ static const char *armada_37xx_pmx_get_group_name(struct udevice *dev,
 {
 	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
 
-	if (!info->groups[selector].name)
+	if (!info->data->groups[selector].name)
 		return armada_37xx_pmx_dummy_name;
 
-	return info->groups[selector].name;
+	return info->data->groups[selector].name;
 }
 
 static int armada_37xx_pmx_get_funcs_count(struct udevice *dev)
@@ -295,7 +293,7 @@ static int armada_37xx_pmx_group_set(struct udevice *dev,
 				     unsigned func_selector)
 {
 	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
-	struct armada_37xx_pin_group *grp = &info->groups[group_selector];
+	struct armada_37xx_pin_group *grp = &info->data->groups[group_selector];
 	const char *name = info->funcs[func_selector].name;
 
 	return armada_37xx_pmx_set_by_name(dev, name, grp);
@@ -350,8 +348,8 @@ static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info)
 {
 	int n, num = 0, funcsize = info->data->nr_pins;
 
-	for (n = 0; n < info->ngroups; n++) {
-		struct armada_37xx_pin_group *grp = &info->groups[n];
+	for (n = 0; n < info->data->ngroups; n++) {
+		struct armada_37xx_pin_group *grp = &info->data->groups[n];
 		int f;
 
 		for (f = 0; (f < NB_FUNCS) && grp->funcs[f]; f++) {
@@ -402,8 +400,8 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
 
 		groups = funcs[n].groups;
 
-		for (g = 0; g < info->ngroups; g++) {
-			struct armada_37xx_pin_group *gp = &info->groups[g];
+		for (g = 0; g < info->data->ngroups; g++) {
+			struct armada_37xx_pin_group *gp = &info->data->groups[g];
 			int f;
 
 			for (f = 0; (f < NB_FUNCS) && gp->funcs[f]; f++) {
@@ -584,9 +582,6 @@ int armada_37xx_pinctrl_probe(struct udevice *dev)
 		return -ENODEV;
 	}
 
-	info->groups = pin_data->groups;
-	info->ngroups = pin_data->ngroups;
-
 	/*
 	 * we allocate functions for number of pins and hope there are
 	 * fewer unique functions than pins available
-- 
2.20.1

[PATCH 3/6] arm64: a37xx: pinctrl: Mark all functions and structures as static

[Pali Rohár]

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index e1cde53a0243..e0445e3e2b3a 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -193,14 +193,14 @@ static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
 		       "mii", "mii_err"),
 };
 
-const struct armada_37xx_pin_data armada_37xx_pin_nb = {
+static const struct armada_37xx_pin_data armada_37xx_pin_nb = {
 	.nr_pins = 36,
 	.name = "GPIO1",
 	.groups = armada_37xx_nb_groups,
 	.ngroups = ARRAY_SIZE(armada_37xx_nb_groups),
 };
 
-const struct armada_37xx_pin_data armada_37xx_pin_sb = {
+static const struct armada_37xx_pin_data armada_37xx_pin_sb = {
 	.nr_pins = 30,
 	.name = "GPIO2",
 	.groups = armada_37xx_sb_groups,
@@ -558,7 +558,7 @@ static int armada_37xx_gpiochip_register(struct udevice *parent,
 	return 0;
 }
 
-const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
+static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
 	.get_groups_count = armada_37xx_pmx_get_groups_count,
 	.get_group_name = armada_37xx_pmx_get_group_name,
 	.get_functions_count = armada_37xx_pmx_get_funcs_count,
@@ -567,7 +567,7 @@ const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
 	.set_state = pinctrl_generic_set_state,
 };
 
-int armada_37xx_pinctrl_probe(struct udevice *dev)
+static int armada_37xx_pinctrl_probe(struct udevice *dev)
 {
 	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
 	const struct armada_37xx_pin_data *pin_data;
-- 
2.20.1

[PATCH 4/6] arm64: a37xx: pinctrl: Add missing pinmuxes into the list

[Pali Rohár]

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index e0445e3e2b3a..d2abe67fe5be 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -109,6 +109,16 @@ struct armada_37xx_pinctrl {
 		.funcs = {_func1, _func2}	\
 	}
 
+#define PIN_GRP_GPIO_0(_name, _start, _nr)	\
+	{					\
+		.name = _name,			\
+		.start_pin = _start,		\
+		.npins = _nr,			\
+		.reg_mask = 0,			\
+		.val = {0},			\
+		.funcs = {"gpio"}		\
+	}
+
 #define PIN_GRP_GPIO(_name, _start, _nr, _mask, _func1)	\
 	{					\
 		.name = _name,			\
@@ -166,6 +176,7 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
 		       "pwm", "led"),
 	PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"),
 	PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"),
+	PIN_GRP_GPIO_0("gpio1_5", 5, 1),
 	PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"),
 	PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"),
 	PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"),
@@ -182,10 +193,13 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
 static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
 	PIN_GRP_GPIO("usb32_drvvbus0", 0, 1, BIT(0), "drvbus"),
 	PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"),
+	PIN_GRP_GPIO_0("gpio2_2", 2, 1),
 	PIN_GRP_GPIO("sdio_sb", 24, 6, BIT(2), "sdio"),
 	PIN_GRP_GPIO("rgmii", 6, 12, BIT(3), "mii"),
 	PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"),
-	PIN_GRP_GPIO("pcie1", 3, 3, BIT(5) | BIT(9) | BIT(10), "pcie"),
+	PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"), /* this actually controls "pcie1_reset" */
+	PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"),
+	PIN_GRP_GPIO("pcie1_wakeup", 5, 1, BIT(10), "pcie"),
 	PIN_GRP_GPIO("ptp", 20, 3, BIT(11) | BIT(12) | BIT(13), "ptp"),
 	PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"),
 	PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"),
-- 
2.20.1

[PATCH 5/6] arm64: a37xx: pinctrl: Implement gpio_request_enable for gpio functionality

[Pali Rohár]

To automatically enable GPIO functionality of some MPP pin, it is required
to implement .gpio_request_enable and .gpio_disable_free callbacks in
pinctrl driver and set .request and .rfree callbacks in GPIO driver to
pinctrl_gpio_request / pinctrl_gpio_free functions.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 50 +++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index d2abe67fe5be..74d915950a6e 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -280,12 +280,13 @@ static const char *armada_37xx_pmx_get_func_name(struct udevice *dev,
 
 static int armada_37xx_pmx_set_by_name(struct udevice *dev,
 				       const char *name,
-				       struct armada_37xx_pin_group *grp)
+				       struct armada_37xx_pin_group *grp,
+				       bool warn_on_change)
 {
 	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
 	unsigned int reg = SELECTION;
 	unsigned int mask = grp->reg_mask;
-	int func, val;
+	int func, val, old_func;
 
 	dev_dbg(info->dev, "enable function %s group %s\n",
 		name, grp->name);
@@ -297,6 +298,18 @@ static int armada_37xx_pmx_set_by_name(struct udevice *dev,
 
 	val = grp->val[func];
 
+	if (warn_on_change && val != (readl(info->base + reg) & mask)) {
+		for (old_func = 0; (old_func < NB_FUNCS) && grp->funcs[old_func]; old_func++) {
+			if (grp->val[old_func] == val)
+				break;
+		}
+		dev_warn(info->dev, "Warning: Changing MPPs %u-%u function from %s to %s...\n",
+			 grp->start_pin, grp->start_pin + grp->npins - 1,
+			 ((old_func < NB_FUNCS && grp->funcs[old_func]) ?
+			  grp->funcs[old_func] : "unknown"),
+			 name);
+	}
+
 	clrsetbits_le32(info->base + reg, mask, val);
 
 	return 0;
@@ -310,7 +323,34 @@ static int armada_37xx_pmx_group_set(struct udevice *dev,
 	struct armada_37xx_pin_group *grp = &info->data->groups[group_selector];
 	const char *name = info->funcs[func_selector].name;
 
-	return armada_37xx_pmx_set_by_name(dev, name, grp);
+	return armada_37xx_pmx_set_by_name(dev, name, grp, false);
+}
+
+static int armada_37xx_pmx_gpio_request_enable(struct udevice *dev, unsigned int selector)
+{
+	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
+	int ret = -ENOTSUPP;
+	int n;
+
+	/* Find all groups where is requested selector pin and set each group to gpio function */
+	for (n = 0; n < info->data->ngroups; n++) {
+		struct armada_37xx_pin_group *grp = &info->data->groups[n];
+
+		if ((selector >= grp->start_pin && selector < grp->start_pin + grp->npins) ||
+		    (selector >= grp->extra_pin && selector < grp->extra_pin + grp->extra_npins)) {
+			ret = armada_37xx_pmx_set_by_name(dev, "gpio", grp, true);
+			if (ret)
+				return ret;
+		}
+	}
+
+	return ret;
+}
+
+static int armada_37xx_pmx_gpio_disable_free(struct udevice *dev, unsigned int selector)
+{
+	/* nothing to do */
+	return 0;
 }
 
 /**
@@ -520,6 +560,8 @@ static int armada_37xx_gpio_probe(struct udevice *dev)
 }
 
 static const struct dm_gpio_ops armada_37xx_gpio_ops = {
+	.request = pinctrl_gpio_request,
+	.rfree = pinctrl_gpio_free,
 	.set_value = armada_37xx_gpio_set,
 	.get_value = armada_37xx_gpio_get,
 	.get_function = armada_37xx_gpio_get_direction,
@@ -578,6 +620,8 @@ static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
 	.get_functions_count = armada_37xx_pmx_get_funcs_count,
 	.get_function_name = armada_37xx_pmx_get_func_name,
 	.pinmux_group_set = armada_37xx_pmx_group_set,
+	.gpio_request_enable = armada_37xx_pmx_gpio_request_enable,
+	.gpio_disable_free = armada_37xx_pmx_gpio_disable_free,
 	.set_state = pinctrl_generic_set_state,
 };
 
-- 
2.20.1

[PATCH 6/6] arm64: a37xx: pinctrl: Implement get_pins_count, get_pin_name and get_pin_muxing functions

[Pali Rohár]

These functions are required for 'pinmux status -a' command to print
current configuration of each MPP pin.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 78 +++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 74d915950a6e..bb7a76baed1f 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -353,6 +353,81 @@ static int armada_37xx_pmx_gpio_disable_free(struct udevice *dev, unsigned int s
 	return 0;
 }
 
+static int armada_37xx_pmx_get_pins_count(struct udevice *dev)
+{
+	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
+
+	return info->data->nr_pins;
+}
+
+static const char *armada_37xx_pmx_get_pin_name(struct udevice *dev, unsigned int selector)
+{
+	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
+	static char buf[sizeof("MPPx_XX")];
+
+	sprintf(buf, "MPP%c_%u", info->data->name[4], selector);
+	return buf;
+}
+
+static int armada_37xx_pmx_get_pin_muxing(struct udevice *dev, unsigned int selector,
+					  char *buf, int size)
+{
+	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
+	int n;
+
+	/*
+	 * First check if selected pin is in some extra pin group.
+	 * Function in extra pin group is active only when it is not gpio.
+	 */
+	for (n = 0; n < info->data->ngroups; n++) {
+		struct armada_37xx_pin_group *grp = &info->data->groups[n];
+
+		if (selector >= grp->extra_pin && selector < grp->extra_pin + grp->extra_npins) {
+			unsigned int reg = SELECTION;
+			unsigned int mask = grp->reg_mask;
+			int f, val;
+
+			val = (readl(info->base + reg) & mask);
+
+			for (f = 0; f < NB_FUNCS && grp->funcs[f]; f++) {
+				if (grp->val[f] == val) {
+					if (strcmp(grp->funcs[f], "gpio") != 0) {
+						strlcpy(buf, grp->funcs[f], size);
+						return 0;
+					}
+					break;
+				}
+			}
+		}
+	}
+
+	/* If pin is not active in some extra pin group then check regular groups. */
+	for (n = 0; n < info->data->ngroups; n++) {
+		struct armada_37xx_pin_group *grp = &info->data->groups[n];
+
+		if (selector >= grp->start_pin && selector < grp->start_pin + grp->npins) {
+			unsigned int reg = SELECTION;
+			unsigned int mask = grp->reg_mask;
+			int f, val;
+
+			val = (readl(info->base + reg) & mask);
+
+			for (f = 0; f < NB_FUNCS && grp->funcs[f]; f++) {
+				if (grp->val[f] == val) {
+					strlcpy(buf, grp->funcs[f], size);
+					return 0;
+				}
+			}
+
+			strlcpy(buf, "unknown", size);
+			return 0;
+		}
+	}
+
+	strlcpy(buf, "unknown", size);
+	return 0;
+}
+
 /**
  * armada_37xx_add_function() - Add a new function to the list
  * @funcs: array of function to add the new one
@@ -615,6 +690,9 @@ static int armada_37xx_gpiochip_register(struct udevice *parent,
 }
 
 static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
+	.get_pins_count = armada_37xx_pmx_get_pins_count,
+	.get_pin_name = armada_37xx_pmx_get_pin_name,
+	.get_pin_muxing = armada_37xx_pmx_get_pin_muxing,
 	.get_groups_count = armada_37xx_pmx_get_groups_count,
 	.get_group_name = armada_37xx_pmx_get_group_name,
 	.get_functions_count = armada_37xx_pmx_get_funcs_count,
-- 
2.20.1

Re: [PATCH 1/6] arm64: a37xx: pinctrl: Remove unused grp->pins fields

[Stefan Roese]

On 25.07.22 14:08, Pali Rohár wrote:
> grp->pins is just filled and never used. Remove it.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index e76ef153e604..610535fa2392 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -65,7 +65,6 @@ DECLARE_GLOBAL_DATA_PTR;
>    *		belonging to the group
>    * @npins:	Number of pins included in the second optional range
>    * @funcs:	A list of pinmux functions that can be selected for this group.
> - * @pins:	List of the pins included in the group
>    */
>   struct armada_37xx_pin_group {
>   	const char	*name;
> @@ -76,7 +75,6 @@ struct armada_37xx_pin_group {
>   	unsigned int	extra_pin;
>   	unsigned int	extra_npins;
>   	const char	*funcs[NB_FUNCS];
> -	unsigned int	*pins;
>   };
>   
>   struct armada_37xx_pin_data {
> @@ -354,19 +352,7 @@ static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info)
>   
>   	for (n = 0; n < info->ngroups; n++) {
>   		struct armada_37xx_pin_group *grp = &info->groups[n];
> -		int i, j, f;
> -
> -		grp->pins = devm_kzalloc(info->dev,
> -					 (grp->npins + grp->extra_npins) *
> -					 sizeof(*grp->pins), GFP_KERNEL);
> -		if (!grp->pins)
> -			return -ENOMEM;
> -
> -		for (i = 0; i < grp->npins; i++)
> -			grp->pins[i] = grp->start_pin + i;
> -
> -		for (j = 0; j < grp->extra_npins; j++)
> -			grp->pins[i+j] = grp->extra_pin + j;
> +		int f;
>   
>   		for (f = 0; (f < NB_FUNCS) && grp->funcs[f]; f++) {
>   			int ret;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 2/6] arm64: a37xx: pinctrl: Remove duplicate info->groups and info->ngroups fields

[Stefan Roese]

On 25.07.22 14:08, Pali Rohár wrote:
> They are available in pin_data structure.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 21 ++++++++-------------
>   1 file changed, 8 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 610535fa2392..e1cde53a0243 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -95,8 +95,6 @@ struct armada_37xx_pinctrl {
>   	const struct armada_37xx_pin_data	*data;
>   	struct udevice			*dev;
>   	struct pinctrl_dev		*pctl_dev;
> -	struct armada_37xx_pin_group	*groups;
> -	unsigned int			ngroups;
>   	struct armada_37xx_pmx_func	*funcs;
>   	unsigned int			nfuncs;
>   };
> @@ -235,7 +233,7 @@ static int armada_37xx_pmx_get_groups_count(struct udevice *dev)
>   {
>   	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
>   
> -	return info->ngroups;
> +	return info->data->ngroups;
>   }
>   
>   static const char *armada_37xx_pmx_dummy_name = "_dummy";
> @@ -245,10 +243,10 @@ static const char *armada_37xx_pmx_get_group_name(struct udevice *dev,
>   {
>   	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
>   
> -	if (!info->groups[selector].name)
> +	if (!info->data->groups[selector].name)
>   		return armada_37xx_pmx_dummy_name;
>   
> -	return info->groups[selector].name;
> +	return info->data->groups[selector].name;
>   }
>   
>   static int armada_37xx_pmx_get_funcs_count(struct udevice *dev)
> @@ -295,7 +293,7 @@ static int armada_37xx_pmx_group_set(struct udevice *dev,
>   				     unsigned func_selector)
>   {
>   	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
> -	struct armada_37xx_pin_group *grp = &info->groups[group_selector];
> +	struct armada_37xx_pin_group *grp = &info->data->groups[group_selector];
>   	const char *name = info->funcs[func_selector].name;
>   
>   	return armada_37xx_pmx_set_by_name(dev, name, grp);
> @@ -350,8 +348,8 @@ static int armada_37xx_fill_group(struct armada_37xx_pinctrl *info)
>   {
>   	int n, num = 0, funcsize = info->data->nr_pins;
>   
> -	for (n = 0; n < info->ngroups; n++) {
> -		struct armada_37xx_pin_group *grp = &info->groups[n];
> +	for (n = 0; n < info->data->ngroups; n++) {
> +		struct armada_37xx_pin_group *grp = &info->data->groups[n];
>   		int f;
>   
>   		for (f = 0; (f < NB_FUNCS) && grp->funcs[f]; f++) {
> @@ -402,8 +400,8 @@ static int armada_37xx_fill_func(struct armada_37xx_pinctrl *info)
>   
>   		groups = funcs[n].groups;
>   
> -		for (g = 0; g < info->ngroups; g++) {
> -			struct armada_37xx_pin_group *gp = &info->groups[g];
> +		for (g = 0; g < info->data->ngroups; g++) {
> +			struct armada_37xx_pin_group *gp = &info->data->groups[g];
>   			int f;
>   
>   			for (f = 0; (f < NB_FUNCS) && gp->funcs[f]; f++) {
> @@ -584,9 +582,6 @@ int armada_37xx_pinctrl_probe(struct udevice *dev)
>   		return -ENODEV;
>   	}
>   
> -	info->groups = pin_data->groups;
> -	info->ngroups = pin_data->ngroups;
> -
>   	/*
>   	 * we allocate functions for number of pins and hope there are
>   	 * fewer unique functions than pins available

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 3/6] arm64: a37xx: pinctrl: Mark all functions and structures as static

[Stefan Roese]

On 25.07.22 14:09, Pali Rohár wrote:
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index e1cde53a0243..e0445e3e2b3a 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -193,14 +193,14 @@ static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
>   		       "mii", "mii_err"),
>   };
>   
> -const struct armada_37xx_pin_data armada_37xx_pin_nb = {
> +static const struct armada_37xx_pin_data armada_37xx_pin_nb = {
>   	.nr_pins = 36,
>   	.name = "GPIO1",
>   	.groups = armada_37xx_nb_groups,
>   	.ngroups = ARRAY_SIZE(armada_37xx_nb_groups),
>   };
>   
> -const struct armada_37xx_pin_data armada_37xx_pin_sb = {
> +static const struct armada_37xx_pin_data armada_37xx_pin_sb = {
>   	.nr_pins = 30,
>   	.name = "GPIO2",
>   	.groups = armada_37xx_sb_groups,
> @@ -558,7 +558,7 @@ static int armada_37xx_gpiochip_register(struct udevice *parent,
>   	return 0;
>   }
>   
> -const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
> +static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
>   	.get_groups_count = armada_37xx_pmx_get_groups_count,
>   	.get_group_name = armada_37xx_pmx_get_group_name,
>   	.get_functions_count = armada_37xx_pmx_get_funcs_count,
> @@ -567,7 +567,7 @@ const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
>   	.set_state = pinctrl_generic_set_state,
>   };
>   
> -int armada_37xx_pinctrl_probe(struct udevice *dev)
> +static int armada_37xx_pinctrl_probe(struct udevice *dev)
>   {
>   	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
>   	const struct armada_37xx_pin_data *pin_data;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 4/6] arm64: a37xx: pinctrl: Add missing pinmuxes into the list

[Stefan Roese]

On 25.07.22 14:09, Pali Rohár wrote:
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index e0445e3e2b3a..d2abe67fe5be 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -109,6 +109,16 @@ struct armada_37xx_pinctrl {
>   		.funcs = {_func1, _func2}	\
>   	}
>   
> +#define PIN_GRP_GPIO_0(_name, _start, _nr)	\
> +	{					\
> +		.name = _name,			\
> +		.start_pin = _start,		\
> +		.npins = _nr,			\
> +		.reg_mask = 0,			\
> +		.val = {0},			\
> +		.funcs = {"gpio"}		\
> +	}
> +
>   #define PIN_GRP_GPIO(_name, _start, _nr, _mask, _func1)	\
>   	{					\
>   		.name = _name,			\
> @@ -166,6 +176,7 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
>   		       "pwm", "led"),
>   	PIN_GRP_GPIO("pmic1", 7, 1, BIT(7), "pmic"),
>   	PIN_GRP_GPIO("pmic0", 6, 1, BIT(8), "pmic"),
> +	PIN_GRP_GPIO_0("gpio1_5", 5, 1),
>   	PIN_GRP_GPIO("i2c2", 2, 2, BIT(9), "i2c"),
>   	PIN_GRP_GPIO("i2c1", 0, 2, BIT(10), "i2c"),
>   	PIN_GRP_GPIO("spi_cs1", 17, 1, BIT(12), "spi"),
> @@ -182,10 +193,13 @@ static struct armada_37xx_pin_group armada_37xx_nb_groups[] = {
>   static struct armada_37xx_pin_group armada_37xx_sb_groups[] = {
>   	PIN_GRP_GPIO("usb32_drvvbus0", 0, 1, BIT(0), "drvbus"),
>   	PIN_GRP_GPIO("usb2_drvvbus1", 1, 1, BIT(1), "drvbus"),
> +	PIN_GRP_GPIO_0("gpio2_2", 2, 1),
>   	PIN_GRP_GPIO("sdio_sb", 24, 6, BIT(2), "sdio"),
>   	PIN_GRP_GPIO("rgmii", 6, 12, BIT(3), "mii"),
>   	PIN_GRP_GPIO("smi", 18, 2, BIT(4), "smi"),
> -	PIN_GRP_GPIO("pcie1", 3, 3, BIT(5) | BIT(9) | BIT(10), "pcie"),
> +	PIN_GRP_GPIO("pcie1", 3, 1, BIT(5), "pcie"), /* this actually controls "pcie1_reset" */
> +	PIN_GRP_GPIO("pcie1_clkreq", 4, 1, BIT(9), "pcie"),
> +	PIN_GRP_GPIO("pcie1_wakeup", 5, 1, BIT(10), "pcie"),
>   	PIN_GRP_GPIO("ptp", 20, 3, BIT(11) | BIT(12) | BIT(13), "ptp"),
>   	PIN_GRP("ptp_clk", 21, 1, BIT(6), "ptp", "mii"),
>   	PIN_GRP("ptp_trig", 22, 1, BIT(7), "ptp", "mii"),

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 5/6] arm64: a37xx: pinctrl: Implement gpio_request_enable for gpio functionality

[Stefan Roese]

On 25.07.22 14:09, Pali Rohár wrote:
> To automatically enable GPIO functionality of some MPP pin, it is required
> to implement .gpio_request_enable and .gpio_disable_free callbacks in
> pinctrl driver and set .request and .rfree callbacks in GPIO driver to
> pinctrl_gpio_request / pinctrl_gpio_free functions.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 50 +++++++++++++++++++--
>   1 file changed, 47 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index d2abe67fe5be..74d915950a6e 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -280,12 +280,13 @@ static const char *armada_37xx_pmx_get_func_name(struct udevice *dev,
>   
>   static int armada_37xx_pmx_set_by_name(struct udevice *dev,
>   				       const char *name,
> -				       struct armada_37xx_pin_group *grp)
> +				       struct armada_37xx_pin_group *grp,
> +				       bool warn_on_change)
>   {
>   	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
>   	unsigned int reg = SELECTION;
>   	unsigned int mask = grp->reg_mask;
> -	int func, val;
> +	int func, val, old_func;
>   
>   	dev_dbg(info->dev, "enable function %s group %s\n",
>   		name, grp->name);
> @@ -297,6 +298,18 @@ static int armada_37xx_pmx_set_by_name(struct udevice *dev,
>   
>   	val = grp->val[func];
>   
> +	if (warn_on_change && val != (readl(info->base + reg) & mask)) {
> +		for (old_func = 0; (old_func < NB_FUNCS) && grp->funcs[old_func]; old_func++) {
> +			if (grp->val[old_func] == val)
> +				break;
> +		}
> +		dev_warn(info->dev, "Warning: Changing MPPs %u-%u function from %s to %s...\n",
> +			 grp->start_pin, grp->start_pin + grp->npins - 1,
> +			 ((old_func < NB_FUNCS && grp->funcs[old_func]) ?
> +			  grp->funcs[old_func] : "unknown"),
> +			 name);
> +	}
> +
>   	clrsetbits_le32(info->base + reg, mask, val);
>   
>   	return 0;
> @@ -310,7 +323,34 @@ static int armada_37xx_pmx_group_set(struct udevice *dev,
>   	struct armada_37xx_pin_group *grp = &info->data->groups[group_selector];
>   	const char *name = info->funcs[func_selector].name;
>   
> -	return armada_37xx_pmx_set_by_name(dev, name, grp);
> +	return armada_37xx_pmx_set_by_name(dev, name, grp, false);
> +}
> +
> +static int armada_37xx_pmx_gpio_request_enable(struct udevice *dev, unsigned int selector)
> +{
> +	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
> +	int ret = -ENOTSUPP;
> +	int n;
> +
> +	/* Find all groups where is requested selector pin and set each group to gpio function */
> +	for (n = 0; n < info->data->ngroups; n++) {
> +		struct armada_37xx_pin_group *grp = &info->data->groups[n];
> +
> +		if ((selector >= grp->start_pin && selector < grp->start_pin + grp->npins) ||
> +		    (selector >= grp->extra_pin && selector < grp->extra_pin + grp->extra_npins)) {
> +			ret = armada_37xx_pmx_set_by_name(dev, "gpio", grp, true);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +static int armada_37xx_pmx_gpio_disable_free(struct udevice *dev, unsigned int selector)
> +{
> +	/* nothing to do */
> +	return 0;
>   }
>   
>   /**
> @@ -520,6 +560,8 @@ static int armada_37xx_gpio_probe(struct udevice *dev)
>   }
>   
>   static const struct dm_gpio_ops armada_37xx_gpio_ops = {
> +	.request = pinctrl_gpio_request,
> +	.rfree = pinctrl_gpio_free,
>   	.set_value = armada_37xx_gpio_set,
>   	.get_value = armada_37xx_gpio_get,
>   	.get_function = armada_37xx_gpio_get_direction,
> @@ -578,6 +620,8 @@ static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
>   	.get_functions_count = armada_37xx_pmx_get_funcs_count,
>   	.get_function_name = armada_37xx_pmx_get_func_name,
>   	.pinmux_group_set = armada_37xx_pmx_group_set,
> +	.gpio_request_enable = armada_37xx_pmx_gpio_request_enable,
> +	.gpio_disable_free = armada_37xx_pmx_gpio_disable_free,
>   	.set_state = pinctrl_generic_set_state,
>   };
>   

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 6/6] arm64: a37xx: pinctrl: Implement get_pins_count, get_pin_name and get_pin_muxing functions

[Stefan Roese]

On 25.07.22 14:09, Pali Rohár wrote:
> These functions are required for 'pinmux status -a' command to print
> current configuration of each MPP pin.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 78 +++++++++++++++++++++
>   1 file changed, 78 insertions(+)
> 
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 74d915950a6e..bb7a76baed1f 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -353,6 +353,81 @@ static int armada_37xx_pmx_gpio_disable_free(struct udevice *dev, unsigned int s
>   	return 0;
>   }
>   
> +static int armada_37xx_pmx_get_pins_count(struct udevice *dev)
> +{
> +	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
> +
> +	return info->data->nr_pins;
> +}
> +
> +static const char *armada_37xx_pmx_get_pin_name(struct udevice *dev, unsigned int selector)
> +{
> +	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
> +	static char buf[sizeof("MPPx_XX")];
> +
> +	sprintf(buf, "MPP%c_%u", info->data->name[4], selector);
> +	return buf;
> +}
> +
> +static int armada_37xx_pmx_get_pin_muxing(struct udevice *dev, unsigned int selector,
> +					  char *buf, int size)
> +{
> +	struct armada_37xx_pinctrl *info = dev_get_priv(dev);
> +	int n;
> +
> +	/*
> +	 * First check if selected pin is in some extra pin group.
> +	 * Function in extra pin group is active only when it is not gpio.
> +	 */
> +	for (n = 0; n < info->data->ngroups; n++) {
> +		struct armada_37xx_pin_group *grp = &info->data->groups[n];
> +
> +		if (selector >= grp->extra_pin && selector < grp->extra_pin + grp->extra_npins) {
> +			unsigned int reg = SELECTION;
> +			unsigned int mask = grp->reg_mask;
> +			int f, val;
> +
> +			val = (readl(info->base + reg) & mask);
> +
> +			for (f = 0; f < NB_FUNCS && grp->funcs[f]; f++) {
> +				if (grp->val[f] == val) {
> +					if (strcmp(grp->funcs[f], "gpio") != 0) {
> +						strlcpy(buf, grp->funcs[f], size);
> +						return 0;
> +					}
> +					break;
> +				}
> +			}
> +		}
> +	}
> +
> +	/* If pin is not active in some extra pin group then check regular groups. */
> +	for (n = 0; n < info->data->ngroups; n++) {
> +		struct armada_37xx_pin_group *grp = &info->data->groups[n];
> +
> +		if (selector >= grp->start_pin && selector < grp->start_pin + grp->npins) {
> +			unsigned int reg = SELECTION;
> +			unsigned int mask = grp->reg_mask;
> +			int f, val;
> +
> +			val = (readl(info->base + reg) & mask);
> +
> +			for (f = 0; f < NB_FUNCS && grp->funcs[f]; f++) {
> +				if (grp->val[f] == val) {
> +					strlcpy(buf, grp->funcs[f], size);
> +					return 0;
> +				}
> +			}
> +
> +			strlcpy(buf, "unknown", size);
> +			return 0;
> +		}
> +	}
> +
> +	strlcpy(buf, "unknown", size);
> +	return 0;
> +}
> +
>   /**
>    * armada_37xx_add_function() - Add a new function to the list
>    * @funcs: array of function to add the new one
> @@ -615,6 +690,9 @@ static int armada_37xx_gpiochip_register(struct udevice *parent,
>   }
>   
>   static const struct pinctrl_ops armada_37xx_pinctrl_ops  = {
> +	.get_pins_count = armada_37xx_pmx_get_pins_count,
> +	.get_pin_name = armada_37xx_pmx_get_pin_name,
> +	.get_pin_muxing = armada_37xx_pmx_get_pin_muxing,
>   	.get_groups_count = armada_37xx_pmx_get_groups_count,
>   	.get_group_name = armada_37xx_pmx_get_group_name,
>   	.get_functions_count = armada_37xx_pmx_get_funcs_count,

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 0/6] arm64: a37xx: pinctrl: Fix requesting GPIOs and pinmux command

[Stefan Roese]

On 25.07.22 14:08, Pali Rohár wrote:
> This patch series cleanup pinctrl-armada-37xx.c driver, add missing pin
> muxes into the list to allow usage all MPP pins as GPIOs, implement
> gpio_request_enable callback for correctly setting MPP pins to GPIO
> mode.
> 
> For debugging purposes are implemented also get_pins_count, get_pin_name
> and get_pin_muxing functions which are required for U-Boot command:
> pinmux status -a
> 
> Pali Rohár (6):
>    arm64: a37xx: pinctrl: Remove unused grp->pins fields
>    arm64: a37xx: pinctrl: Remove duplicate info->groups and info->ngroups
>      fields
>    arm64: a37xx: pinctrl: Mark all functions and structures as static
>    arm64: a37xx: pinctrl: Add missing pinmuxes into the list
>    arm64: a37xx: pinctrl: Implement gpio_request_enable for gpio
>      functionality
>    arm64: a37xx: pinctrl: Implement get_pins_count, get_pin_name and
>      get_pin_muxing functions
> 
>   drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 189 ++++++++++++++++----
>   1 file changed, 153 insertions(+), 36 deletions(-)
> 

Applied to u-boot-marvell/master

Thanks,
Stefan