diff mbox series

[RFC,1/6] pinctrl: Add pinctrl_gpio_as_pin()

Message ID 20210723075858.376378-2-andrew@aj.id.au (mailing list archive)
State New, archived
Headers show
Series leds: Fix pca955x GPIO pin mappings | expand

Commit Message

Andrew Jeffery July 23, 2021, 7:58 a.m. UTC
Allow gpiochips to map the GPIO numberspace onto a pin numberspace when
the register layout for GPIO control is implemented in terms of the
pin numberspace.

This requirement sounds kind of strange, but the patch is driven by
trying to resolve a bug in the leds-pca955x driver where this mapping is
not correctly performed.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 drivers/pinctrl/core.c          | 19 +++++++++++++++++++
 include/linux/pinctrl/pinctrl.h |  3 +++
 2 files changed, 22 insertions(+)

Comments

Linus Walleij Aug. 10, 2021, 1:34 p.m. UTC | #1
On Fri, Jul 23, 2021 at 9:59 AM Andrew Jeffery <andrew@aj.id.au> wrote:

> Allow gpiochips to map the GPIO numberspace onto a pin numberspace when
> the register layout for GPIO control is implemented in terms of the
> pin numberspace.
>
> This requirement sounds kind of strange, but the patch is driven by
> trying to resolve a bug in the leds-pca955x driver where this mapping is
> not correctly performed.
>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

(...)

Hm  this looks a bit strange...

> +int pinctrl_gpio_as_pin(struct pinctrl_dev *pctldev, unsigned int gpio)

This is not a good name for this function. Try to come up with
a name that says exactly what the function does.

E.g. "apple pear as apple slice" isn't very helpful, the use case for
this is really hard to understand.

> +EXPORT_SYMBOL_GPL(pinctrl_find_gpio_range_from_pin);

This looks completely wrong.

Yours,
Linus Walleij
Andrew Jeffery Aug. 11, 2021, 12:24 a.m. UTC | #2
On Tue, 10 Aug 2021, at 23:04, Linus Walleij wrote:
> On Fri, Jul 23, 2021 at 9:59 AM Andrew Jeffery <andrew@aj.id.au> wrote:
> 
> > Allow gpiochips to map the GPIO numberspace onto a pin numberspace when
> > the register layout for GPIO control is implemented in terms of the
> > pin numberspace.
> >
> > This requirement sounds kind of strange, but the patch is driven by
> > trying to resolve a bug in the leds-pca955x driver where this mapping is
> > not correctly performed.
> >
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> 
> (...)
> 
> Hm  this looks a bit strange...
> 
> > +int pinctrl_gpio_as_pin(struct pinctrl_dev *pctldev, unsigned int gpio)
> 
> This is not a good name for this function. Try to come up with
> a name that says exactly what the function does.
> 
> E.g. "apple pear as apple slice" isn't very helpful, the use case for
> this is really hard to understand.

That's probably because I shouldn't be trying to do what I'm doing :)

I'll stop doing that (i.e. rework patch 4/6) and this will go away entirely.

> 
> > +EXPORT_SYMBOL_GPL(pinctrl_find_gpio_range_from_pin);
> 
> This looks completely wrong.

Yeah, whoops. That was an oversight while iterating on the patch.

Andrew
diff mbox series

Patch

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
index a4ac87c8b4f8..9c788f0e2844 100644
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -738,6 +738,25 @@  int pinctrl_get_group_selector(struct pinctrl_dev *pctldev,
 	return -EINVAL;
 }
 
+int pinctrl_gpio_as_pin(struct pinctrl_dev *pctldev, unsigned int gpio)
+{
+	struct pinctrl_gpio_range *range;
+	int pin;
+
+	range = pinctrl_match_gpio_range(pctldev, gpio);
+	if (!range)
+		return -ENODEV;
+
+	mutex_lock(&pctldev->mutex);
+
+	pin = gpio_to_pin(range, gpio);
+
+	mutex_unlock(&pctldev->mutex);
+
+	return pin;
+}
+EXPORT_SYMBOL_GPL(pinctrl_find_gpio_range_from_pin);
+
 bool pinctrl_gpio_can_use_line(unsigned gpio)
 {
 	struct pinctrl_dev *pctldev;
diff --git a/include/linux/pinctrl/pinctrl.h b/include/linux/pinctrl/pinctrl.h
index 70b45d28e7a9..1ceebc499cc4 100644
--- a/include/linux/pinctrl/pinctrl.h
+++ b/include/linux/pinctrl/pinctrl.h
@@ -182,6 +182,9 @@  extern struct pinctrl_dev *pinctrl_find_and_add_gpio_range(const char *devname,
 extern struct pinctrl_gpio_range *
 pinctrl_find_gpio_range_from_pin(struct pinctrl_dev *pctldev,
 				 unsigned int pin);
+
+extern int pinctrl_gpio_as_pin(struct pinctrl_dev *pctldev, unsigned int gpio);
+
 extern int pinctrl_get_group_pins(struct pinctrl_dev *pctldev,
 				const char *pin_group, const unsigned **pins,
 				unsigned *num_pins);