The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks
@ 2026-07-31 17:49 Alex Tran
  2026-07-31 17:49 ` [PATCH 1/2] pinctrl: scmi: Replace pinmux ops get function info with generics Alex Tran
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Alex Tran @ 2026-07-31 17:49 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Linus Walleij
  Cc: arm-scmi, linux-arm-kernel, linux-gpio, linux-kernel, Alex Tran

This series refactors the SCMI pinctrl driver to eliminate metadata
getter callbacks to be replaced with pinctrl and pinmux generics
provided by the pinctrl core.

Previously, the driver implemented its own get_functions_count,
get_function_name, get_function_groups, get_groups_count,
get_group_name, and get_group_pins callbacks, which required
maintaining driver metadata like the functions and count.

The pinctrl core already has support to store and manage this
topology in the pinctrl_dev via its own internal radix trees.
By registering functions and groups at probe time using
pinmux_generic_add_function and pinctrl_generic_add_group, the
driver can delegate group and function metadata management to
the core.

Signed-off-by: Alex Tran <alex.tran@oss.qualcomm.com>
---
Alex Tran (2):
      pinctrl: scmi: Replace pinmux ops get function info with generics
      pinctrl: scmi: Replace pinctrl ops get group info with generics

 drivers/pinctrl/pinctrl-scmi.c | 223 ++++++++++++++++++-----------------------
 1 file changed, 97 insertions(+), 126 deletions(-)
---
base-commit: 48a5a7ab8d6ab7090564339e039c421f315de912
change-id: 20260724-scmi-pinctrl-generics-746484c98b66

Best regards,
--  
Alex Tran <alex.tran@oss.qualcomm.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] pinctrl: scmi: Replace pinmux ops get function info with generics
  2026-07-31 17:49 [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Alex Tran
@ 2026-07-31 17:49 ` Alex Tran
  2026-07-31 17:49 ` [PATCH 2/2] pinctrl: scmi: Replace pinctrl ops get group " Alex Tran
  2026-08-14  8:26 ` [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Cristian Marussi
  2 siblings, 0 replies; 4+ messages in thread
From: Alex Tran @ 2026-07-31 17:49 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Linus Walleij
  Cc: arm-scmi, linux-arm-kernel, linux-gpio, linux-kernel, Alex Tran

During probe, populate the pinctrl device with function info
so that the generic callbacks can be used to fetch function
count, name, and groups. Remove locally stored functions
since caching them in this driver is no longer needed.

Signed-off-by: Alex Tran <alex.tran@oss.qualcomm.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 145 ++++++++++++++++-------------------------
 1 file changed, 55 insertions(+), 90 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 1bb36ca477b7..3461c5712613 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -24,6 +24,7 @@
 #include "pinctrl-utils.h"
 #include "core.h"
 #include "pinconf.h"
+#include "pinmux.h"
 
 #define DRV_NAME "scmi-pinctrl"
 
@@ -37,8 +38,6 @@ struct scmi_pinctrl {
 	struct scmi_protocol_handle *ph;
 	struct pinctrl_dev *pctldev;
 	struct pinctrl_desc pctl_desc;
-	struct pinfunction *functions;
-	unsigned int nr_functions;
 };
 
 static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
@@ -84,86 +83,6 @@ static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
 #endif
 };
 
-static int pinctrl_scmi_get_functions_count(struct pinctrl_dev *pctldev)
-{
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	return pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
-}
-
-static const char *pinctrl_scmi_get_function_name(struct pinctrl_dev *pctldev,
-						  unsigned int selector)
-{
-	int ret;
-	const char *name;
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	ret = pinctrl_ops->name_get(pmx->ph, selector, FUNCTION_TYPE, &name);
-	if (ret) {
-		dev_err(pmx->dev, "get name failed with err %d", ret);
-		return NULL;
-	}
-
-	return name;
-}
-
-static int pinctrl_scmi_get_function_groups(struct pinctrl_dev *pctldev,
-					    unsigned int selector,
-					    const char * const **p_groups,
-					    unsigned int * const p_num_groups)
-{
-	struct pinfunction *func;
-	const unsigned int *group_ids;
-	unsigned int num_groups;
-	const char **groups;
-	int ret, i;
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	if (!p_groups || !p_num_groups)
-		return -EINVAL;
-
-	if (selector >= pmx->nr_functions)
-		return -EINVAL;
-
-	func = &pmx->functions[selector];
-	if (func->ngroups)
-		goto done;
-
-	ret = pinctrl_ops->function_groups_get(pmx->ph, selector, &num_groups,
-					       &group_ids);
-	if (ret) {
-		dev_err(pmx->dev, "Unable to get function groups, err %d", ret);
-		return ret;
-	}
-	if (!num_groups)
-		return -EINVAL;
-
-	groups = kcalloc(num_groups, sizeof(*groups), GFP_KERNEL);
-	if (!groups)
-		return -ENOMEM;
-
-	for (i = 0; i < num_groups; i++) {
-		groups[i] = pinctrl_scmi_get_group_name(pctldev, group_ids[i]);
-		if (!groups[i]) {
-			ret = -EINVAL;
-			goto err_free;
-		}
-	}
-
-	func->ngroups = num_groups;
-	func->groups = groups;
-done:
-	*p_groups = func->groups;
-	*p_num_groups = func->ngroups;
-
-	return 0;
-
-err_free:
-	kfree(groups);
-
-	return ret;
-}
-
 static int pinctrl_scmi_func_set_mux(struct pinctrl_dev *pctldev,
 				     unsigned int selector, unsigned int group)
 {
@@ -190,9 +109,9 @@ static int pinctrl_scmi_free(struct pinctrl_dev *pctldev, unsigned int offset)
 static const struct pinmux_ops pinctrl_scmi_pinmux_ops = {
 	.request = pinctrl_scmi_request,
 	.free = pinctrl_scmi_free,
-	.get_functions_count = pinctrl_scmi_get_functions_count,
-	.get_function_name = pinctrl_scmi_get_function_name,
-	.get_function_groups = pinctrl_scmi_get_function_groups,
+	.get_functions_count = pinmux_generic_get_function_count,
+	.get_function_name = pinmux_generic_get_function_name,
+	.get_function_groups = pinmux_generic_get_function_groups,
 	.set_mux = pinctrl_scmi_func_set_mux,
 };
 
@@ -487,6 +406,54 @@ static const struct pinconf_ops pinctrl_scmi_pinconf_ops = {
 	.pin_config_config_dbg_show = pinconf_generic_dump_config,
 };
 
+static int pinctrl_scmi_get_functions(struct scmi_pinctrl *pmx,
+				      struct pinctrl_dev *pctldev)
+{
+	const unsigned int *group_ids;
+	unsigned int nr_funcs, nr_groups;
+	const char **gnames;
+	const char *fname;
+	unsigned int i, j;
+	int ret;
+
+	nr_funcs = pinctrl_ops->count_get(pmx->ph, FUNCTION_TYPE);
+
+	for (i = 0; i < nr_funcs; i++) {
+		ret = pinctrl_ops->name_get(pmx->ph, i, FUNCTION_TYPE,
+					    &fname);
+		if (ret)
+			return ret;
+
+		ret = pinctrl_ops->function_groups_get(pmx->ph, i,
+						       &nr_groups,
+						       &group_ids);
+		if (ret)
+			return ret;
+
+		if (!nr_groups)
+			return -EINVAL;
+
+		gnames = devm_kmalloc_array(pmx->dev, nr_groups,
+					    sizeof(*gnames), GFP_KERNEL);
+		if (!gnames)
+			return -ENOMEM;
+
+		for (j = 0; j < nr_groups; j++) {
+			ret = pinctrl_ops->name_get(pmx->ph, group_ids[j],
+						    GROUP_TYPE, &gnames[j]);
+			if (ret)
+				return ret;
+		}
+
+		ret = pinmux_generic_add_function(pctldev, fname, gnames,
+						  nr_groups, NULL);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 				 struct pinctrl_desc *desc)
 {
@@ -571,11 +538,9 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
 
-	pmx->nr_functions = pinctrl_scmi_get_functions_count(pmx->pctldev);
-	pmx->functions = devm_kcalloc(dev, pmx->nr_functions,
-				      sizeof(*pmx->functions), GFP_KERNEL);
-	if (!pmx->functions)
-		return -ENOMEM;
+	ret = pinctrl_scmi_get_functions(pmx, pmx->pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register functions\n");
 
 	return pinctrl_enable(pmx->pctldev);
 }

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] pinctrl: scmi: Replace pinctrl ops get group info with generics
  2026-07-31 17:49 [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Alex Tran
  2026-07-31 17:49 ` [PATCH 1/2] pinctrl: scmi: Replace pinmux ops get function info with generics Alex Tran
@ 2026-07-31 17:49 ` Alex Tran
  2026-08-14  8:26 ` [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Cristian Marussi
  2 siblings, 0 replies; 4+ messages in thread
From: Alex Tran @ 2026-07-31 17:49 UTC (permalink / raw)
  To: Sudeep Holla, Cristian Marussi, Linus Walleij
  Cc: arm-scmi, linux-arm-kernel, linux-gpio, linux-kernel, Alex Tran

During probe, populate the pinctrl device with group info
so that the generic callbacks can be used to fetch group
count, name, and pins.

Signed-off-by: Alex Tran <alex.tran@oss.qualcomm.com>
---
 drivers/pinctrl/pinctrl-scmi.c | 78 +++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 36 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-scmi.c b/drivers/pinctrl/pinctrl-scmi.c
index 3461c5712613..032b23fa17e0 100644
--- a/drivers/pinctrl/pinctrl-scmi.c
+++ b/drivers/pinctrl/pinctrl-scmi.c
@@ -40,43 +40,10 @@ struct scmi_pinctrl {
 	struct pinctrl_desc pctl_desc;
 };
 
-static int pinctrl_scmi_get_groups_count(struct pinctrl_dev *pctldev)
-{
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	return pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
-}
-
-static const char *pinctrl_scmi_get_group_name(struct pinctrl_dev *pctldev,
-					       unsigned int selector)
-{
-	int ret;
-	const char *name;
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	ret = pinctrl_ops->name_get(pmx->ph, selector, GROUP_TYPE, &name);
-	if (ret) {
-		dev_err(pmx->dev, "get name failed with err %d", ret);
-		return NULL;
-	}
-
-	return name;
-}
-
-static int pinctrl_scmi_get_group_pins(struct pinctrl_dev *pctldev,
-				       unsigned int selector,
-				       const unsigned int **pins,
-				       unsigned int *num_pins)
-{
-	struct scmi_pinctrl *pmx = pinctrl_dev_get_drvdata(pctldev);
-
-	return pinctrl_ops->group_pins_get(pmx->ph, selector, pins, num_pins);
-}
-
 static const struct pinctrl_ops pinctrl_scmi_pinctrl_ops = {
-	.get_groups_count = pinctrl_scmi_get_groups_count,
-	.get_group_name = pinctrl_scmi_get_group_name,
-	.get_group_pins = pinctrl_scmi_get_group_pins,
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
 #ifdef CONFIG_OF
 	.dt_node_to_map = pinconf_generic_dt_node_to_map_all,
 	.dt_free_map = pinconf_generic_dt_free_map,
@@ -454,6 +421,41 @@ static int pinctrl_scmi_get_functions(struct scmi_pinctrl *pmx,
 	return 0;
 }
 
+static int pinctrl_scmi_get_groups(struct scmi_pinctrl *pmx,
+				   struct pinctrl_dev *pctldev)
+{
+	unsigned int nr_groups, nr_pins, i;
+	const unsigned int *pins;
+	unsigned int *pins_cpy;
+	const char *gname;
+	int ret;
+
+	nr_groups = pinctrl_ops->count_get(pmx->ph, GROUP_TYPE);
+
+	for (i = 0; i < nr_groups; i++) {
+		ret = pinctrl_ops->name_get(pmx->ph, i, GROUP_TYPE, &gname);
+		if (ret)
+			return ret;
+
+		ret = pinctrl_ops->group_pins_get(pmx->ph, i, &pins,
+						  &nr_pins);
+		if (ret)
+			return ret;
+
+		pins_cpy = devm_kmemdup(pmx->dev, pins,
+						 nr_pins * sizeof(*pins), GFP_KERNEL);
+		if (!pins_cpy)
+			return -ENOMEM;
+
+		ret = pinctrl_generic_add_group(pctldev, gname, pins_cpy,
+						nr_pins, NULL);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 static int pinctrl_scmi_get_pins(struct scmi_pinctrl *pmx,
 				 struct pinctrl_desc *desc)
 {
@@ -538,6 +540,10 @@ static int scmi_pinctrl_probe(struct scmi_device *sdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to register pinctrl\n");
 
+	ret = pinctrl_scmi_get_groups(pmx, pmx->pctldev);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to register groups\n");
+
 	ret = pinctrl_scmi_get_functions(pmx, pmx->pctldev);
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to register functions\n");

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks
  2026-07-31 17:49 [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Alex Tran
  2026-07-31 17:49 ` [PATCH 1/2] pinctrl: scmi: Replace pinmux ops get function info with generics Alex Tran
  2026-07-31 17:49 ` [PATCH 2/2] pinctrl: scmi: Replace pinctrl ops get group " Alex Tran
@ 2026-08-14  8:26 ` Cristian Marussi
  2 siblings, 0 replies; 4+ messages in thread
From: Cristian Marussi @ 2026-08-14  8:26 UTC (permalink / raw)
  To: Alex Tran
  Cc: Sudeep Holla, Cristian Marussi, Linus Walleij, arm-scmi,
	linux-arm-kernel, linux-gpio, linux-kernel

On Fri, Jul 31, 2026 at 10:49:31AM -0700, Alex Tran wrote:
> This series refactors the SCMI pinctrl driver to eliminate metadata
> getter callbacks to be replaced with pinctrl and pinmux generics
> provided by the pinctrl core.

Hi Alex,

I gave it a go at this series on an emulated setup and with these
changes I get:

root@deb-guest:~# modprobe pinctrl-scmi
[   15.329932] scmi-pinctrl scmi_dev.4: error -EINVAL: Failed to register functions
[   15.330259] scmi-pinctrl scmi_dev.4: probe with driver scmi-pinctrl failed with error -22

...not debugged any further.

Thanks,
Cristian

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-14  8:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-31 17:49 [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Alex Tran
2026-07-31 17:49 ` [PATCH 1/2] pinctrl: scmi: Replace pinmux ops get function info with generics Alex Tran
2026-07-31 17:49 ` [PATCH 2/2] pinctrl: scmi: Replace pinctrl ops get group " Alex Tran
2026-08-14  8:26 ` [PATCH 0/2] pinctrl: scmi: Replace get function and group info with generic callbacks Cristian Marussi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox