[RFC 3/7] pinctrl: introduce pinctrl_gpio_get_config()

Dan Carpenter posted 7 patches 9 months, 1 week ago
There is a newer version of this series
[RFC 3/7] pinctrl: introduce pinctrl_gpio_get_config()
Posted by Dan Carpenter 9 months, 1 week ago
From: AKASHI Takahiro <takahiro.akashi@linaro.org>

This is a counterpart of pinctrl_gpio_set_config(), which will initially
be used to implement gpio_get interface in SCMI pinctrl based GPIO driver.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/pinctrl/core.c           | 35 ++++++++++++++++++++++++++++++++
 include/linux/pinctrl/consumer.h |  9 ++++++++
 2 files changed, 44 insertions(+)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index 4bdbf6bb26e2..4310f9e2118b 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -30,6 +30,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/pinctrl/devinfo.h>
 #include <linux/pinctrl/machine.h>
+#include <linux/pinctrl/pinconf.h>
 #include <linux/pinctrl/pinctrl.h>
 
 #include "core.h"
@@ -937,6 +938,40 @@ int pinctrl_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 }
 EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config);
 
+/**
+ * pinctrl_gpio_get_config() - Get the config for a given GPIO pin
+ * @gc: GPIO chip structure from the GPIO subsystem
+ * @offset: hardware offset of the GPIO relative to the controller
+  * @config: the configuration to query.  On success it holds the result
+ */
+int pinctrl_gpio_get_config(struct gpio_chip *gc, unsigned int offset, unsigned long *config)
+{
+	struct pinctrl_gpio_range *range;
+	const struct pinconf_ops *ops;
+	struct pinctrl_dev *pctldev;
+	int ret, pin;
+
+	ret = pinctrl_get_device_gpio_range(gc, offset, &pctldev, &range);
+	if (ret)
+		return ret;
+
+	ops = pctldev->desc->confops;
+	if (!ops || !ops->pin_config_get)
+		return -EINVAL;
+
+	mutex_lock(&pctldev->mutex);
+	pin = gpio_to_pin(range, gc, offset);
+	ret = ops->pin_config_get(pctldev, pin, config);
+	mutex_unlock(&pctldev->mutex);
+
+	if (ret)
+		return ret;
+
+	*config = pinconf_to_config_argument(*config);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_get_config);
+
 static struct pinctrl_state *find_state(struct pinctrl *p,
 					const char *name)
 {
diff --git a/include/linux/pinctrl/consumer.h b/include/linux/pinctrl/consumer.h
index 73de70362b98..e5815b3382dc 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -35,6 +35,8 @@ int pinctrl_gpio_direction_output(struct gpio_chip *gc,
 				  unsigned int offset);
 int pinctrl_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 				unsigned long config);
+int pinctrl_gpio_get_config(struct gpio_chip *gc, unsigned int offset,
+			    unsigned long *config);
 
 struct pinctrl * __must_check pinctrl_get(struct device *dev);
 void pinctrl_put(struct pinctrl *p);
@@ -96,6 +98,13 @@ pinctrl_gpio_direction_output(struct gpio_chip *gc, unsigned int offset)
 	return 0;
 }
 
+static inline int
+pinctrl_gpio_get_config(struct gpio_chip *gc, unsigned int offset,
+			unsigned long *config)
+{
+	return 0;
+}
+
 static inline int
 pinctrl_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
 			    unsigned long config)
-- 
2.47.2
Re: [RFC 3/7] pinctrl: introduce pinctrl_gpio_get_config()
Posted by ALOK TIWARI 9 months, 1 week ago
Hi Dan,


Thanks for your patch.

On 05-05-2025 17:07, Dan Carpenter wrote:
> From: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> This is a counterpart of pinctrl_gpio_set_config(), which will initially
> be used to implement gpio_get interface in SCMI pinctrl based GPIO driver.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
>   drivers/pinctrl/core.c           | 35 ++++++++++++++++++++++++++++++++
>   include/linux/pinctrl/consumer.h |  9 ++++++++
>   2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
> index 4bdbf6bb26e2..4310f9e2118b 100644
> --- a/drivers/pinctrl/core.c
> +++ b/drivers/pinctrl/core.c
> @@ -30,6 +30,7 @@
>   #include <linux/pinctrl/consumer.h>
>   #include <linux/pinctrl/devinfo.h>
>   #include <linux/pinctrl/machine.h>
> +#include <linux/pinctrl/pinconf.h>
>   #include <linux/pinctrl/pinctrl.h>
>   
>   #include "core.h"
> @@ -937,6 +938,40 @@ int pinctrl_gpio_set_config(struct gpio_chip *gc, unsigned int offset,
>   }
>   EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config);
>   
> +/**
> + * pinctrl_gpio_get_config() - Get the config for a given GPIO pin
> + * @gc: GPIO chip structure from the GPIO subsystem
> + * @offset: hardware offset of the GPIO relative to the controller
> +  * @config: the configuration to query.  On success it holds the result

remove extra ' ' before * @config and On.

> + */
> +int pinctrl_gpio_get_config(struct gpio_chip *gc, unsigned int offset, unsigned long *config)
> +{
> +	struct pinctrl_gpio_range *range;
> +	const struct pinconf_ops *ops;
> +	struct pinctrl_dev *pctldev;
> +	int ret, pin;
> +
> +	ret = pinctrl_get_device_gpio_range(gc, offset, &pctldev, &range);
> +	if (ret)
> +		return ret;
> +
> +	ops = pctldev->desc->confops;
> +	if (!ops || !ops->pin_config_get)
> +		return -EINVAL;
> +
> +	mutex_lock(&pctldev->mutex);
> +	pin = gpio_to_pin(range, gc, offset);
> +	ret = ops->pin_config_get(pctldev, pin, config);

can we add reason here, as now we are not calling pin_config_get_for_pin()

https://lore.kernel.org/all/20231002021602.260100-3-takahiro.akashi@linaro.org/

> +	mutex_unlock(&pctldev->mutex);
> +
> +	if (ret)
> +		return ret;
> +
> +	*config = pinconf_to_config_argument(*config);

a '\n' before return.

> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(pinctrl_gpio_get_config);
> +
>   static struct pinctrl_state *find_state(struct pinctrl *p,
>   					const char *name)
>   {


Thanks,
Alok
Re: [RFC 3/7] pinctrl: introduce pinctrl_gpio_get_config()
Posted by Dan Carpenter 9 months, 1 week ago
On Mon, May 05, 2025 at 10:00:35PM +0530, ALOK TIWARI wrote:
> > +int pinctrl_gpio_get_config(struct gpio_chip *gc, unsigned int offset, unsigned long *config)
> > +{
> > +	struct pinctrl_gpio_range *range;
> > +	const struct pinconf_ops *ops;
> > +	struct pinctrl_dev *pctldev;
> > +	int ret, pin;
> > +
> > +	ret = pinctrl_get_device_gpio_range(gc, offset, &pctldev, &range);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ops = pctldev->desc->confops;
> > +	if (!ops || !ops->pin_config_get)
> > +		return -EINVAL;
> > +
> > +	mutex_lock(&pctldev->mutex);
> > +	pin = gpio_to_pin(range, gc, offset);
> > +	ret = ops->pin_config_get(pctldev, pin, config);
> 
> can we add reason here, as now we are not calling pin_config_get_for_pin()
> 
> https://lore.kernel.org/all/20231002021602.260100-3-takahiro.akashi@linaro.org/
> 

I don't even know why I changed that.  Using pin_config_get_for_pin()
works and it's cleaner.

regards,
dan carpenter