From: Bartosz Golaszewski <[email protected]>
Currently the pinctrl GPIO helpers all take a number from the global
GPIO numberspace - of which we're trying to get rid of as argument.
These helpers are almost universally called from GPIOLIB driver
callbacks which take a pointer to the backing gpio_chip and the
controller-relative offset as arguments.
Let's provide improved variants of these functions that match the
GPIOLIB signatures as the first step in removing the older flavor.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pinctrl/core.c | 107 ++++++++++++++++++++++---------
include/linux/pinctrl/consumer.h | 46 +++++++++++++
2 files changed, 124 insertions(+), 29 deletions(-)
diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index e2f7519bef04..679f48c5beeb 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -23,6 +23,7 @@
#include <linux/seq_file.h>
#include <linux/slab.h>
+#include <linux/gpio/driver.h>
#include <linux/pinctrl/consumer.h>
#include <linux/pinctrl/devinfo.h>
#include <linux/pinctrl/machine.h>
@@ -781,14 +782,13 @@ bool pinctrl_gpio_can_use_line(unsigned gpio)
}
EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
-/**
- * pinctrl_gpio_request() - request a single pin to be used as GPIO
- * @gpio: the GPIO pin number from the GPIO subsystem number space
- *
- * This function should *ONLY* be used from gpiolib-based GPIO drivers,
- * as part of their gpio_request() semantics, platforms and individual drivers
- * shall *NOT* request GPIO pins to be muxed in.
- */
+bool pinctrl_gpio_can_use_line_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return pinctrl_gpio_can_use_line(gc->base + offset);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line_new);
+
+/* This function is deprecated and will be removed. Don't use. */
int pinctrl_gpio_request(unsigned gpio)
{
struct pinctrl_dev *pctldev;
@@ -817,13 +817,21 @@ int pinctrl_gpio_request(unsigned gpio)
EXPORT_SYMBOL_GPL(pinctrl_gpio_request);
/**
- * pinctrl_gpio_free() - free control on a single pin, currently used as GPIO
- * @gpio: the GPIO pin number from the GPIO subsystem number space
+ * pinctrl_gpio_request_new() - request a single pin to be used as GPIO
+ * @gc: GPIO chip structure from the GPIO subsystem
+ * @offset: hardware offset of the GPIO relative to the controller
*
* This function should *ONLY* be used from gpiolib-based GPIO drivers,
- * as part of their gpio_free() semantics, platforms and individual drivers
- * shall *NOT* request GPIO pins to be muxed out.
+ * as part of their gpio_request() semantics, platforms and individual drivers
+ * shall *NOT* request GPIO pins to be muxed in.
*/
+int pinctrl_gpio_request_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return pinctrl_gpio_request(gc->base + offset);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_request_new);
+
+/* This function is deprecated and will be removed. Don't use. */
void pinctrl_gpio_free(unsigned gpio)
{
struct pinctrl_dev *pctldev;
@@ -846,6 +854,21 @@ void pinctrl_gpio_free(unsigned gpio)
}
EXPORT_SYMBOL_GPL(pinctrl_gpio_free);
+/**
+ * pinctrl_gpio_free_new() - free control on a single pin, currently used as GPIO
+ * @gc: GPIO chip structure from the GPIO subsystem
+ * @offset: hardware offset of the GPIO relative to the controller
+ *
+ * This function should *ONLY* be used from gpiolib-based GPIO drivers,
+ * as part of their gpio_request() semantics, platforms and individual drivers
+ * shall *NOT* request GPIO pins to be muxed in.
+ */
+void pinctrl_gpio_free_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return pinctrl_gpio_free(gc->base + offset);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_free_new);
+
static int pinctrl_gpio_direction(unsigned gpio, bool input)
{
struct pinctrl_dev *pctldev;
@@ -869,14 +892,7 @@ static int pinctrl_gpio_direction(unsigned gpio, bool input)
return ret;
}
-/**
- * pinctrl_gpio_direction_input() - request a GPIO pin to go into input mode
- * @gpio: the GPIO pin number from the GPIO subsystem number space
- *
- * This function should *ONLY* be used from gpiolib-based GPIO drivers,
- * as part of their gpio_direction_input() semantics, platforms and individual
- * drivers shall *NOT* touch pin control GPIO calls.
- */
+/* This function is deprecated and will be removed. Don't use. */
int pinctrl_gpio_direction_input(unsigned gpio)
{
return pinctrl_gpio_direction(gpio, true);
@@ -884,13 +900,21 @@ int pinctrl_gpio_direction_input(unsigned gpio)
EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_input);
/**
- * pinctrl_gpio_direction_output() - request a GPIO pin to go into output mode
- * @gpio: the GPIO pin number from the GPIO subsystem number space
+ * pinctrl_gpio_direction_input_new() - request a GPIO pin to go into input mode
+ * @gc: GPIO chip structure from the GPIO subsystem
+ * @offset: hardware offset of the GPIO relative to the controller
*
* This function should *ONLY* be used from gpiolib-based GPIO drivers,
- * as part of their gpio_direction_output() semantics, platforms and individual
+ * as part of their gpio_direction_input() semantics, platforms and individual
* drivers shall *NOT* touch pin control GPIO calls.
*/
+int pinctrl_gpio_direction_input_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return pinctrl_gpio_direction_input(gc->base + offset);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_input_new);
+
+/* This function is deprecated and will be removed. Don't use. */
int pinctrl_gpio_direction_output(unsigned gpio)
{
return pinctrl_gpio_direction(gpio, false);
@@ -898,14 +922,22 @@ int pinctrl_gpio_direction_output(unsigned gpio)
EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output);
/**
- * pinctrl_gpio_set_config() - Apply config to given GPIO pin
- * @gpio: the GPIO pin number from the GPIO subsystem number space
- * @config: the configuration to apply to the GPIO
+ * pinctrl_gpio_direction_output_new() - request a GPIO pin to go into output
+ * mode
+ * @gc: GPIO chip structure from the GPIO subsystem
+ * @offset: hardware offset of the GPIO relative to the controller
*
- * This function should *ONLY* be used from gpiolib-based GPIO drivers, if
- * they need to call the underlying pin controller to change GPIO config
- * (for example set debounce time).
+ * This function should *ONLY* be used from gpiolib-based GPIO drivers,
+ * as part of their gpio_direction_output() semantics, platforms and individual
+ * drivers shall *NOT* touch pin control GPIO calls.
*/
+int pinctrl_gpio_direction_output_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return pinctrl_gpio_direction_output(gc->base + offset);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_direction_output_new);
+
+/* This function is deprecated and will be removed. Don't use. */
int pinctrl_gpio_set_config(unsigned gpio, unsigned long config)
{
unsigned long configs[] = { config };
@@ -926,6 +958,23 @@ int pinctrl_gpio_set_config(unsigned gpio, unsigned long config)
}
EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config);
+/**
+ * pinctrl_gpio_set_config_new() - Apply config to 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 apply to the GPIO
+ *
+ * This function should *ONLY* be used from gpiolib-based GPIO drivers, if
+ * they need to call the underlying pin controller to change GPIO config
+ * (for example set debounce time).
+ */
+int pinctrl_gpio_set_config_new(struct gpio_chip *gc, unsigned int offset,
+ unsigned long config)
+{
+ return pinctrl_gpio_set_config(gc->base + offset, config);
+}
+EXPORT_SYMBOL_GPL(pinctrl_gpio_set_config_new);
+
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 22eef0a513ce..c95c13983376 100644
--- a/include/linux/pinctrl/consumer.h
+++ b/include/linux/pinctrl/consumer.h
@@ -17,6 +17,7 @@
#include <linux/pinctrl/pinctrl-state.h>
struct device;
+struct gpio_chip;
/* This struct is private to the core and should be regarded as a cookie */
struct pinctrl;
@@ -26,11 +27,20 @@ struct pinctrl_state;
/* External interface to pin control */
bool pinctrl_gpio_can_use_line(unsigned gpio);
+bool pinctrl_gpio_can_use_line_new(struct gpio_chip *gc, unsigned int offset);
int pinctrl_gpio_request(unsigned gpio);
+int pinctrl_gpio_request_new(struct gpio_chip *gc, unsigned int offset);
void pinctrl_gpio_free(unsigned gpio);
+void pinctrl_gpio_free_new(struct gpio_chip *gc, unsigned int offset);
int pinctrl_gpio_direction_input(unsigned gpio);
+int pinctrl_gpio_direction_input_new(struct gpio_chip *gc,
+ unsigned int offset);
int pinctrl_gpio_direction_output(unsigned gpio);
+int pinctrl_gpio_direction_output_new(struct gpio_chip *gc,
+ unsigned int offset);
int pinctrl_gpio_set_config(unsigned gpio, unsigned long config);
+int pinctrl_gpio_set_config_new(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);
@@ -68,30 +78,66 @@ static inline bool pinctrl_gpio_can_use_line(unsigned gpio)
return true;
}
+static inline bool
+pinctrl_gpio_can_use_line_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return true;
+}
+
static inline int pinctrl_gpio_request(unsigned gpio)
{
return 0;
}
+static inline int
+pinctrl_gpio_request_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return 0;
+}
+
static inline void pinctrl_gpio_free(unsigned gpio)
{
}
+static inline void
+pinctrl_gpio_free_new(struct gpio_chip *gc, unsigned int offset)
+{
+}
+
static inline int pinctrl_gpio_direction_input(unsigned gpio)
{
return 0;
}
+static inline int
+pinctrl_gpio_direction_input_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return 0;
+}
+
static inline int pinctrl_gpio_direction_output(unsigned gpio)
{
return 0;
}
+static inline int
+pinctrl_gpio_direction_output_new(struct gpio_chip *gc, unsigned int offset)
+{
+ return 0;
+}
+
static inline int pinctrl_gpio_set_config(unsigned gpio, unsigned long config)
{
return 0;
}
+static inline int
+pinctrl_gpio_set_config_new(struct gpio_chip *gc, unsigned int offset,
+ unsigned long config)
+{
+ return 0;
+}
+
static inline struct pinctrl * __must_check pinctrl_get(struct device *dev)
{
return NULL;
--
2.39.2
On Tue, Oct 3, 2023 at 5:51 PM Bartosz Golaszewski <[email protected]> wrote:
>
> From: Bartosz Golaszewski <[email protected]>
>
> Currently the pinctrl GPIO helpers all take a number from the global
> GPIO numberspace - of which we're trying to get rid of as argument.
>
> These helpers are almost universally called from GPIOLIB driver
> callbacks which take a pointer to the backing gpio_chip and the
> controller-relative offset as arguments.
>
> Let's provide improved variants of these functions that match the
> GPIOLIB signatures as the first step in removing the older flavor.
...
> +#include <linux/gpio/driver.h>
+ blank line here
> #include <linux/pinctrl/consumer.h>
> #include <linux/pinctrl/devinfo.h>
> #include <linux/pinctrl/machine.h>
...
> EXPORT_SYMBOL_GPL(pinctrl_gpio_can_use_line);
> +/* This function is deprecated and will be removed. Don't use. */
Shouldn't the same / similar comment be added to
pinctrl_gpio_can_use_line() above?
...
> + * pinctrl_gpio_request_new() - request a single pin to be used as GPIO
"new" is too broad and too odd suffix, what I would see as a better
alternative(s) is (are):
pinctrl_gpiochip_request()
pinctrl_gpio_chip_request()
pinctrl_gc_request()
(sorted by personal preference from high to low).
The similar comment to all of them.
...
> * This function should *ONLY* be used from gpiolib-based GPIO drivers,
> - * as part of their gpio_free() semantics, platforms and individual drivers
> - * shall *NOT* request GPIO pins to be muxed out.
> + * as part of their gpio_request() semantics, platforms and individual drivers
> + * shall *NOT* request GPIO pins to be muxed in.
Hmm... This looks like a fix to the previous code that can even be backported.
> */
--
With Best Regards,
Andy Shevchenko
On Tue, Oct 3, 2023 at 6:08 PM Andy Shevchenko
<[email protected]> wrote:
> On Tue, Oct 3, 2023 at 5:51 PM Bartosz Golaszewski <[email protected]> wrote:
...
> > + * pinctrl_gpio_request_new() - request a single pin to be used as GPIO
>
> "new" is too broad and too odd suffix, what I would see as a better
> alternative(s) is (are):
>
> pinctrl_gpiochip_request()
> pinctrl_gpio_chip_request()
> pinctrl_gc_request()
>
> (sorted by personal preference from high to low).
>
> The similar comment to all of them.
Or since it's about the GPIO line
pinctrl_gpio_line_request()
(and other variants near to this).
--
With Best Regards,
Andy Shevchenko