[PATCH 1/3] pinctrl: split off consumer API into linux/pinctrl/consumer.h

[PATCH 1/3] pinctrl: split off consumer API into linux/pinctrl/consumer.h

[Ahmad Fatoum]

Follow-up patches will align the barebox pinctrl consumer API with
Linux'. So it makes sense to have the header called the same as well.
Users of the old pinctrl.h header are unaffected as it will include
the new <linux/pinctrl/consumer.h>.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/pinctrl/consumer.h | 61 ++++++++++++++++++++++++++++++++
 include/pinctrl.h                | 55 +++-------------------------
 2 files changed, 65 insertions(+), 51 deletions(-)
 create mode 100644 include/linux/pinctrl/consumer.h

diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
new file mode 100644
index 000000000000..3028f960deb4
--- /dev/null
+++ b/include/linux/pinctrl/consumer.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef LINUX_PINCTRL_CONSUMER_H
+#define LINUX_PINCTRL_CONSUMER_H
+
+#include <linux/errno.h>
+
+struct device;
+struct device_node;
+
+#ifdef CONFIG_PINCTRL
+int pinctrl_select_state(struct device *dev, const char *state);
+int pinctrl_select_state_default(struct device *dev);
+int of_pinctrl_select_state(struct device_node *np, const char *state);
+int of_pinctrl_select_state_default(struct device_node *np);
+int pinctrl_gpio_direction_input(unsigned pin);
+int pinctrl_gpio_direction_output(unsigned int pin);
+int pinctrl_gpio_get_direction(unsigned pin);
+int pinctrl_single_probe(struct device *dev);
+#else
+static inline int pinctrl_select_state(struct device *dev, const char *state)
+{
+	return -ENODEV;
+}
+
+static inline int pinctrl_select_state_default(struct device *dev)
+{
+	return -ENODEV;
+}
+
+static inline int of_pinctrl_select_state(struct device_node *np, const char *state)
+{
+	return -ENODEV;
+}
+
+static inline int of_pinctrl_select_state_default(struct device_node *np)
+{
+	return -ENODEV;
+}
+
+static inline int pinctrl_gpio_direction_input(unsigned pin)
+{
+	return -ENOTSUPP;
+}
+
+static inline int pinctrl_gpio_direction_output(unsigned int pin)
+{
+	return -ENOTSUPP;
+}
+
+static inline int pinctrl_gpio_get_direction(unsigned pin)
+{
+	return -ENOTSUPP;
+}
+
+static inline int pinctrl_single_probe(struct device *dev)
+{
+	return -ENOSYS;
+}
+#endif
+
+#endif /* LINUX_PINCTRL_CONSUMER_H */
diff --git a/include/pinctrl.h b/include/pinctrl.h
index f0f7c2cc8b3e..4ba7b489345b 100644
--- a/include/pinctrl.h
+++ b/include/pinctrl.h
@@ -2,7 +2,11 @@
 #ifndef PINCTRL_H
 #define PINCTRL_H
 
+#include <linux/types.h>
+#include <linux/pinctrl/consumer.h>
+
 struct pinctrl_device;
+struct device_node;
 
 struct pinctrl_ops {
 	int (*set_state)(struct pinctrl_device *, struct device_node *);
@@ -21,55 +25,4 @@ struct pinctrl_device {
 int pinctrl_register(struct pinctrl_device *pdev);
 void pinctrl_unregister(struct pinctrl_device *pdev);
 
-#ifdef CONFIG_PINCTRL
-int pinctrl_select_state(struct device *dev, const char *state);
-int pinctrl_select_state_default(struct device *dev);
-int of_pinctrl_select_state(struct device_node *np, const char *state);
-int of_pinctrl_select_state_default(struct device_node *np);
-int pinctrl_gpio_direction_input(unsigned pin);
-int pinctrl_gpio_direction_output(unsigned int pin);
-int pinctrl_gpio_get_direction(unsigned pin);
-int pinctrl_single_probe(struct device *dev);
-#else
-static inline int pinctrl_select_state(struct device *dev, const char *state)
-{
-	return -ENODEV;
-}
-
-static inline int pinctrl_select_state_default(struct device *dev)
-{
-	return -ENODEV;
-}
-
-static inline int of_pinctrl_select_state(struct device_node *np, const char *state)
-{
-	return -ENODEV;
-}
-
-static inline int of_pinctrl_select_state_default(struct device_node *np)
-{
-	return -ENODEV;
-}
-
-static inline int pinctrl_gpio_direction_input(unsigned pin)
-{
-	return -ENOTSUPP;
-}
-
-static inline int pinctrl_gpio_direction_output(unsigned int pin)
-{
-	return -ENOTSUPP;
-}
-
-static inline int pinctrl_gpio_get_direction(unsigned pin)
-{
-	return -ENOTSUPP;
-}
-
-static inline int pinctrl_single_probe(struct device *dev)
-{
-	return -ENOSYS;
-}
-#endif
-
 #endif /* PINCTRL_H */
-- 
2.39.2

[PATCH 2/3] pinctrl: rename barebox pinctrl_select_state to pinctrl_get_select

[Ahmad Fatoum]

Linux pinctrl_select_state() operates on struct pinctrl_state. The
helper that selects a state by name is instead called
pinctrl_get_select.

There's only a single user in-tree for that function, so let's rename
it and adjust the prototype.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/i2c/busses/i2c-imx.c     | 24 ++++++++++++++----------
 drivers/pinctrl/pinctrl.c        | 13 +++++++++----
 include/linux/pinctrl/consumer.h | 10 +++++++---
 3 files changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index f6a67ec067df..981db015ea34 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -509,21 +509,25 @@ static int i2c_fsl_xfer(struct i2c_adapter *adapter,
 	return (result < 0) ? result : num;
 }
 
+static void i2c_fsl_pinctrl_select_state(struct i2c_adapter *adapter,
+					 const char *name)
+{
+	struct pinctrl *pinctrl;
+
+	pinctrl = pinctrl_get_select(adapter->dev.parent, name);
+	if (pinctrl)
+		pinctrl_put(pinctrl);
+	else
+		dev_err(adapter->dev.parent, "pinctrl failed: %pe\n", pinctrl);
+}
+
 static void i2c_fsl_prepare_recovery(struct i2c_adapter *adapter)
 {
-	int ret;
-
-	ret = pinctrl_select_state(adapter->dev.parent, "gpio");
-	if (ret)
-		dev_err(adapter->dev.parent, "pinctrl failed: %s\n", strerror(-ret));
+	i2c_fsl_pinctrl_select_state(adapter, "gpio");
 }
 static void i2c_fsl_unprepare_recovery(struct i2c_adapter *adapter)
 {
-	int ret;
-
-	ret = pinctrl_select_state(adapter->dev.parent, "default");
-	if (ret)
-		dev_err(adapter->dev.parent, "pinctrl failed: %s\n", strerror(-ret));
+	i2c_fsl_pinctrl_select_state(adapter, "default");
 }
 
 static void i2c_fsl_init_recovery(struct fsl_i2c_struct *i2c_fsl,
diff --git a/drivers/pinctrl/pinctrl.c b/drivers/pinctrl/pinctrl.c
index 95e7b0ea96e1..2d331211f71c 100644
--- a/drivers/pinctrl/pinctrl.c
+++ b/drivers/pinctrl/pinctrl.c
@@ -161,20 +161,25 @@ int of_pinctrl_select_state_default(struct device_node *np)
 	return of_pinctrl_select_state(np, "default");
 }
 
-int pinctrl_select_state(struct device *dev, const char *name)
+struct pinctrl *pinctrl_get_select(struct device *dev, const char *name)
 {
 	struct device_node *np;
+	int ret;
 
 	np = dev->of_node;
 	if (!np)
-		return 0;
+		return ERR_PTR(-ENODEV);
 
-	return of_pinctrl_select_state(np, name);
+	ret = of_pinctrl_select_state(np, name);
+	if (ret)
+		return ERR_PTR(ret);
+
+	return (struct pinctrl *)np;
 }
 
 int pinctrl_select_state_default(struct device *dev)
 {
-	return pinctrl_select_state(dev, "default");
+	return PTR_ERR_OR_ZERO(pinctrl_get_select(dev, "default"));
 }
 
 int pinctrl_register(struct pinctrl_device *pdev)
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 3028f960deb4..33d5c1b19a89 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -7,8 +7,10 @@
 struct device;
 struct device_node;
 
+struct pinctrl;
+
 #ifdef CONFIG_PINCTRL
-int pinctrl_select_state(struct device *dev, const char *state);
+struct pinctrl *pinctrl_get_select(struct device *dev, const char *state);
 int pinctrl_select_state_default(struct device *dev);
 int of_pinctrl_select_state(struct device_node *np, const char *state);
 int of_pinctrl_select_state_default(struct device_node *np);
@@ -17,9 +19,9 @@ int pinctrl_gpio_direction_output(unsigned int pin);
 int pinctrl_gpio_get_direction(unsigned pin);
 int pinctrl_single_probe(struct device *dev);
 #else
-static inline int pinctrl_select_state(struct device *dev, const char *state)
+static inline struct pinctrl *pinctrl_get_select(struct device *dev, const char *state)
 {
-	return -ENODEV;
+	return ERR_PTR(-ENODEV);
 }
 
 static inline int pinctrl_select_state_default(struct device *dev)
@@ -58,4 +60,6 @@ static inline int pinctrl_single_probe(struct device *dev)
 }
 #endif
 
+static inline void pinctrl_put(struct pinctrl *pinctrl) {}
+
 #endif /* LINUX_PINCTRL_CONSUMER_H */
-- 
2.39.2

[PATCH 3/3] pinctrl: implement pinctrl_lookup_state/select_state

[Ahmad Fatoum]

Besides pinctrl_get_select, the Linux API provides separate functions to
lookup a state and verify its existence and activating a state.

To make porting code easier that calls the former during probe and the
latter at runtime, define these functions and implement
of_pinctrl_select_state on top of them.

No functional change intended.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/pinctrl/pinctrl.c        | 98 ++++++++++++++++++++++----------
 include/linux/pinctrl/consumer.h | 23 ++++++++
 2 files changed, 90 insertions(+), 31 deletions(-)

diff --git a/drivers/pinctrl/pinctrl.c b/drivers/pinctrl/pinctrl.c
index 2d331211f71c..dd0ed156ec9e 100644
--- a/drivers/pinctrl/pinctrl.c
+++ b/drivers/pinctrl/pinctrl.c
@@ -10,6 +10,14 @@
 #include <errno.h>
 #include <of.h>
 
+struct pinctrl {
+	struct device_node consumer_np;
+};
+
+struct pinctrl_state {
+	struct property prop;
+};
+
 static LIST_HEAD(pinctrl_list);
 
 static struct pinctrl_device *pin_to_pinctrl(unsigned int pin)
@@ -90,30 +98,28 @@ static int pinctrl_config_one(struct device_node *for_node, struct device_node *
 		return -ENODEV;
 }
 
-int of_pinctrl_select_state(struct device_node *np, const char *name)
+static inline struct pinctrl_state *
+of_property_pinctrl_get_state(struct property *prop)
 {
+	return container_of(prop, struct pinctrl_state, prop);
+}
+
+struct pinctrl_state *pinctrl_lookup_state(struct pinctrl *pinctrl,
+					   const char *name)
+{
+	struct device_node *np = &pinctrl->consumer_np;
 	int state, ret;
 	char propname[sizeof("pinctrl-4294967295")];
-	const __be32 *list;
-	int size, config;
-	phandle phandle;
-	struct device_node *np_config;
+	struct property *prop;
 	const char *statename;
 
-	if (!of_find_property(np, "pinctrl-0", NULL))
-		return 0;
-
 	/* For each defined state ID */
 	for (state = 0; ; state++) {
 		/* Retrieve the pinctrl-* property */
 		sprintf(propname, "pinctrl-%d", state);
-		list = of_get_property(np, propname, &size);
-		if (!list) {
-			ret = -ENODEV;
-			break;
-		}
-
-		size /= sizeof(*list);
+		prop = of_find_property(np, propname, NULL);
+		if (!prop)
+			return ERR_PTR(-ENODEV);
 
 		/* Determine whether pinctrl-names property names the state */
 		ret = of_property_read_string_index(np, "pinctrl-names",
@@ -131,31 +137,61 @@ int of_pinctrl_select_state(struct device_node *np, const char *name)
 		if (strcmp(name, statename))
 			continue;
 
-		/* For every referenced pin configuration node in it */
-		for (config = 0; config < size; config++) {
-			phandle = be32_to_cpup(list++);
+		return of_property_pinctrl_get_state(prop);
+	}
 
-			/* Look up the pin configuration node */
-			np_config = of_find_node_by_phandle(phandle);
-			if (!np_config) {
-				pr_err("prop %pOF %s index %i invalid phandle\n",
-					np, propname, config);
-				ret = -EINVAL;
-				goto err;
-			}
+	return ERR_PTR(ret);
+}
 
-			/* Parse the node */
-			ret = pinctrl_config_one(np, np_config);
-			if (ret < 0)
-				goto err;
+int pinctrl_select_state(struct pinctrl *pinctrl, struct pinctrl_state *state)
+{
+	int ret = -ENODEV;
+	const __be32 *list;
+	int size, config;
+	phandle phandle;
+	struct device_node *np = &pinctrl->consumer_np, *np_config;
+	struct property *prop = &state->prop;
+
+	list = of_property_get_value(prop);
+	size = prop->length / sizeof(*list);
+
+	/* For every referenced pin configuration node in it */
+	for (config = 0; config < size; config++) {
+		phandle = be32_to_cpup(list++);
+
+		/* Look up the pin configuration node */
+		np_config = of_find_node_by_phandle(phandle);
+		if (!np_config) {
+			pr_err("prop %pOF %s index %i invalid phandle\n",
+				np, prop->name, config);
+			ret = -EINVAL;
+			goto err;
 		}
 
-		return 0;
+		/* Parse the node */
+		ret = pinctrl_config_one(np, np_config);
+		if (ret < 0)
+			goto err;
 	}
 err:
 	return ret;
 }
 
+int of_pinctrl_select_state(struct device_node *np, const char *name)
+{
+	struct pinctrl *pinctrl = of_pinctrl_get(np);
+	struct pinctrl_state *state;
+
+	if (!of_find_property(np, "pinctrl-0", NULL))
+		return 0;
+
+	state = pinctrl_lookup_state(pinctrl, name);
+	if (IS_ERR(state))
+		return PTR_ERR(state);
+
+	return pinctrl_select_state(pinctrl, state);
+}
+
 int of_pinctrl_select_state_default(struct device_node *np)
 {
 	return of_pinctrl_select_state(np, "default");
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 33d5c1b19a89..faa2c0e15724 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -3,6 +3,7 @@
 #define LINUX_PINCTRL_CONSUMER_H
 
 #include <linux/errno.h>
+#include <device.h>
 
 struct device;
 struct device_node;
@@ -10,6 +11,9 @@ struct device_node;
 struct pinctrl;
 
 #ifdef CONFIG_PINCTRL
+struct pinctrl_state *pinctrl_lookup_state(struct pinctrl *pinctrl,
+					   const char *name);
+int pinctrl_select_state(struct pinctrl *pinctrl, struct pinctrl_state *state);
 struct pinctrl *pinctrl_get_select(struct device *dev, const char *state);
 int pinctrl_select_state_default(struct device *dev);
 int of_pinctrl_select_state(struct device_node *np, const char *state);
@@ -19,6 +23,15 @@ int pinctrl_gpio_direction_output(unsigned int pin);
 int pinctrl_gpio_get_direction(unsigned pin);
 int pinctrl_single_probe(struct device *dev);
 #else
+static inline struct pinctrl_state *pinctrl_lookup_state(struct pinctrl *pinctrl,
+							 const char *name);
+{
+        return NULL;
+}
+static inline int pinctrl_select_state(struct pinctrl *pinctrl, struct pinctrl_state *state)
+{
+	return 0;
+}
 static inline struct pinctrl *pinctrl_get_select(struct device *dev, const char *state)
 {
 	return ERR_PTR(-ENODEV);
@@ -62,4 +75,14 @@ static inline int pinctrl_single_probe(struct device *dev)
 
 static inline void pinctrl_put(struct pinctrl *pinctrl) {}
 
+static inline struct pinctrl *of_pinctrl_get(struct device_node *np)
+{
+	return (struct pinctrl *)np;
+}
+
+static inline struct pinctrl *pinctrl_get(struct device *dev)
+{
+	return of_pinctrl_get(dev_of_node(dev));
+}
+
 #endif /* LINUX_PINCTRL_CONSUMER_H */
-- 
2.39.2

Re: [PATCH 1/3] pinctrl: split off consumer API into linux/pinctrl/consumer.h

[Sascha Hauer]

On Fri, 17 May 2024 07:56:10 +0200, Ahmad Fatoum wrote:
> Follow-up patches will align the barebox pinctrl consumer API with
> Linux'. So it makes sense to have the header called the same as well.
> Users of the old pinctrl.h header are unaffected as it will include
> the new <linux/pinctrl/consumer.h>.
> 
> 

Applied, thanks!

[1/3] pinctrl: split off consumer API into linux/pinctrl/consumer.h
      https://git.pengutronix.de/cgit/barebox/commit/?id=2cbfef3c0964 (link may not be stable)
[2/3] pinctrl: rename barebox pinctrl_select_state to pinctrl_get_select
      https://git.pengutronix.de/cgit/barebox/commit/?id=1a34f8cc686a (link may not be stable)
[3/3] pinctrl: implement pinctrl_lookup_state/select_state
      https://git.pengutronix.de/cgit/barebox/commit/?id=e13c73ff4b17 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>