| Message ID | 20250506-aaeon-up-board-pinctrl-support-v5-8-3906529757d2@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v5,01/12] gpiolib: add support to register sparse pin range | expand |
On Wed, May 7, 2025 at 6:21 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, May 7, 2025 at 5:53 PM Thomas Richard > <thomas.richard@bootlin.com> wrote: > > On 5/7/25 08:29, Andy Shevchenko wrote: ... > > >> + * gpio_fwd_gpio_add - Add a GPIO in the forwarder > > > > > > forwarder > > > > Sorry I do not see the typo :) > > Your original piece of the code. Please look better. Ah, it was probably me mistakenly fixing the original text :-) It has a typo there.
On 5/7/25 17:23, Andy Shevchenko wrote: > On Wed, May 7, 2025 at 6:21 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: >> On Wed, May 7, 2025 at 5:53 PM Thomas Richard >> <thomas.richard@bootlin.com> wrote: >>> On 5/7/25 08:29, Andy Shevchenko wrote: > > ... > >>>>> + * gpio_fwd_gpio_add - Add a GPIO in the forwarder >>>> >>>> forwarder >>> >>> Sorry I do not see the typo :) >> >> Your original piece of the code. Please look better. > > Ah, it was probably me mistakenly fixing the original text :-) It has > a typo there. > Oh I get it. Yes you fixed the original text. But I checked in my code and I missed it :) Thomas
Hi Andy, On Wed, 7 May 2025 at 17:23, Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Wed, May 7, 2025 at 6:21 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Wed, May 7, 2025 at 5:53 PM Thomas Richard > > <thomas.richard@bootlin.com> wrote: > > > On 5/7/25 08:29, Andy Shevchenko wrote: > > ... > > > > >> + * gpio_fwd_gpio_add - Add a GPIO in the forwarder > > > > > > > > forwarder > > > > > > Sorry I do not see the typo :) > > > > Your original piece of the code. Please look better. > > Ah, it was probably me mistakenly fixing the original text :-) It has > a typo there. Gmail suggests correcting typos in your emails while you type them, even in quoted parts, and you may have inadvertently accepted the suggestion. I've been bitten by that, too :-( Gr{oetje,eeting}s, Geert
Hi Geert Thanks a lot for the review. On 5/9/25 11:07, Geert Uytterhoeven wrote: > Hi Thomas, > > On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: >> Export all symbols and create header file for the GPIO forwarder library. >> It will be used in the next changes. >> >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> ... >> + >> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, >> + unsigned long config); >> + >> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); > > I would expect all of these to take gpiochip_fwd pointers instead of > gpio_chip pointers. What prevents you from passing a gpio_chip pointer > that does not correspond to a gpiochip_fwd object, causing a crash? Indeed nothing prevents from passing gpio_chip pointer which does not correspond to a gpiochip_fwd object. And it is also a bit weird to pass a gpiochip_fwd pointer in some cases (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. I can keep GPIO operations as is, and create exported wrappers which take a gpiochip_fwd pointer as parameter, for example: int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, unsigned long *mask, unsigned long *bits) { struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); return gpio_fwd_get_multiple_locked(chip, mask, bits); } EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); So exported functions are gpiochip_fwd_*(). Regards, Thomas
On Mon, May 12, 2025 at 04:08:35PM +0200, Thomas Richard wrote: > On 5/9/25 11:07, Geert Uytterhoeven wrote: > > On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: ... > >> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, > >> + unsigned long config); > >> + > >> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); > > > > I would expect all of these to take gpiochip_fwd pointers instead of > > gpio_chip pointers. What prevents you from passing a gpio_chip pointer > > that does not correspond to a gpiochip_fwd object, causing a crash? > > Indeed nothing prevents from passing gpio_chip pointer which does not > correspond to a gpiochip_fwd object. > And it is also a bit weird to pass a gpiochip_fwd pointer in some cases > (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. > > I can keep GPIO operations as is, and create exported wrappers which > take a gpiochip_fwd pointer as parameter, for example: > > int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, > unsigned long *mask, > unsigned long *bits) > { > struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); > > return gpio_fwd_get_multiple_locked(chip, mask, bits); > } > EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); > > So exported functions are gpiochip_fwd_*(). Sounds good for me. Let's wait for Geert's opinoin on this.
On 5/12/25 16:11, Andy Shevchenko wrote: > On Mon, May 12, 2025 at 04:08:35PM +0200, Thomas Richard wrote: >> On 5/9/25 11:07, Geert Uytterhoeven wrote: >>> On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: > > ... > >>>> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, >>>> + unsigned long config); >>>> + >>>> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); >>> >>> I would expect all of these to take gpiochip_fwd pointers instead of >>> gpio_chip pointers. What prevents you from passing a gpio_chip pointer >>> that does not correspond to a gpiochip_fwd object, causing a crash? >> >> Indeed nothing prevents from passing gpio_chip pointer which does not >> correspond to a gpiochip_fwd object. >> And it is also a bit weird to pass a gpiochip_fwd pointer in some cases >> (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. >> >> I can keep GPIO operations as is, and create exported wrappers which >> take a gpiochip_fwd pointer as parameter, for example: >> >> int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, >> unsigned long *mask, >> unsigned long *bits) >> { >> struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); >> >> return gpio_fwd_get_multiple_locked(chip, mask, bits); >> } >> EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); >> >> So exported functions are gpiochip_fwd_*(). > > Sounds good for me. Let's wait for Geert's opinoin on this. Regarding Geert's comment for patch 9/12, an other proposal for naming: As mentioned above, exported functions gpiochip_fwd_*() take a gpiochip_fwd as parameter. But for all functions corresponding to a GPIO operation add the gpio word, and for functions to add/remove GPIO descriptor add the desc word: devm_gpiochip_fwd_alloc() gpiochip_fwd_register() gpiochip_fwd_desc_add() gpiochip_fwd_desc_free() gpiochip_fwd_get_gpiochip() gpiochip_fwd_get_data() gpiochip_fwd_gpio_request() gpiochip_fwd_gpio_get_direction() gpiochip_fwd_gpio_direction_input() gpiochip_fwd_gpio_direction_output() gpiochip_fwd_gpio_get() gpiochip_fwd_gpio_set() gpiochip_fwd_gpio_set_multiple() gpiochip_fwd_gpio_get_multiple() gpiochip_fwd_gpio_set_config() gpiochip_fwd_gpio_to_irq() Regards, Thomas
Hi Thomas, On Mon, 12 May 2025 at 16:08, Thomas Richard <thomas.richard@bootlin.com> wrote: > On 5/9/25 11:07, Geert Uytterhoeven wrote: > > On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: > >> Export all symbols and create header file for the GPIO forwarder library. > >> It will be used in the next changes. > >> > >> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > > ... > > >> + > >> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, > >> + unsigned long config); > >> + > >> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); > > > > I would expect all of these to take gpiochip_fwd pointers instead of > > gpio_chip pointers. What prevents you from passing a gpio_chip pointer > > that does not correspond to a gpiochip_fwd object, causing a crash? > > Indeed nothing prevents from passing gpio_chip pointer which does not > correspond to a gpiochip_fwd object. > And it is also a bit weird to pass a gpiochip_fwd pointer in some cases > (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. > > I can keep GPIO operations as is, and create exported wrappers which > take a gpiochip_fwd pointer as parameter, for example: > > int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, > unsigned long *mask, > unsigned long *bits) > { > struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); > > return gpio_fwd_get_multiple_locked(chip, mask, bits); > } > EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); > > So exported functions are gpiochip_fwd_*(). That sounds fine to me. BTW, do you need to use these functions as gpio_chip callbacks? If that is the case, they do no need to take struct gpio_chip pointers. Gr{oetje,eeting}s, Geert
Hi Thomas, On Mon, 12 May 2025 at 16:30, Thomas Richard <thomas.richard@bootlin.com> wrote: > On 5/12/25 16:11, Andy Shevchenko wrote: > > On Mon, May 12, 2025 at 04:08:35PM +0200, Thomas Richard wrote: > >> On 5/9/25 11:07, Geert Uytterhoeven wrote: > >>> On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: > > > > ... > > > >>>> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, > >>>> + unsigned long config); > >>>> + > >>>> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); > >>> > >>> I would expect all of these to take gpiochip_fwd pointers instead of > >>> gpio_chip pointers. What prevents you from passing a gpio_chip pointer > >>> that does not correspond to a gpiochip_fwd object, causing a crash? > >> > >> Indeed nothing prevents from passing gpio_chip pointer which does not > >> correspond to a gpiochip_fwd object. > >> And it is also a bit weird to pass a gpiochip_fwd pointer in some cases > >> (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. > >> > >> I can keep GPIO operations as is, and create exported wrappers which > >> take a gpiochip_fwd pointer as parameter, for example: > >> > >> int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, > >> unsigned long *mask, > >> unsigned long *bits) > >> { > >> struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); > >> > >> return gpio_fwd_get_multiple_locked(chip, mask, bits); > >> } > >> EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); > >> > >> So exported functions are gpiochip_fwd_*(). > > > > Sounds good for me. Let's wait for Geert's opinoin on this. > > Regarding Geert's comment for patch 9/12, an other proposal for naming: > As mentioned above, exported functions gpiochip_fwd_*() take a > gpiochip_fwd as parameter. > > But for all functions corresponding to a GPIO operation add the gpio > word, and for functions to add/remove GPIO descriptor add the desc word: > > devm_gpiochip_fwd_alloc() > gpiochip_fwd_register() > > gpiochip_fwd_desc_add() > gpiochip_fwd_desc_free() > > gpiochip_fwd_get_gpiochip() > gpiochip_fwd_get_data() > > gpiochip_fwd_gpio_request() > gpiochip_fwd_gpio_get_direction() > gpiochip_fwd_gpio_direction_input() > gpiochip_fwd_gpio_direction_output() > gpiochip_fwd_gpio_get() > gpiochip_fwd_gpio_set() > gpiochip_fwd_gpio_set_multiple() > gpiochip_fwd_gpio_get_multiple() > gpiochip_fwd_gpio_set_config() > gpiochip_fwd_gpio_to_irq() Sounds good to me (from a quick glance). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 5/12/25 16:39, Geert Uytterhoeven wrote: > Hi Thomas, > > On Mon, 12 May 2025 at 16:08, Thomas Richard <thomas.richard@bootlin.com> wrote: >> On 5/9/25 11:07, Geert Uytterhoeven wrote: >>> On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: >>>> Export all symbols and create header file for the GPIO forwarder library. >>>> It will be used in the next changes. >>>> >>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> >> >> ... >> >>>> + >>>> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, >>>> + unsigned long config); >>>> + >>>> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); >>> >>> I would expect all of these to take gpiochip_fwd pointers instead of >>> gpio_chip pointers. What prevents you from passing a gpio_chip pointer >>> that does not correspond to a gpiochip_fwd object, causing a crash? >> >> Indeed nothing prevents from passing gpio_chip pointer which does not >> correspond to a gpiochip_fwd object. >> And it is also a bit weird to pass a gpiochip_fwd pointer in some cases >> (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. >> >> I can keep GPIO operations as is, and create exported wrappers which >> take a gpiochip_fwd pointer as parameter, for example: >> >> int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, >> unsigned long *mask, >> unsigned long *bits) >> { >> struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); >> >> return gpio_fwd_get_multiple_locked(chip, mask, bits); >> } >> EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); >> >> So exported functions are gpiochip_fwd_*(). > > That sounds fine to me. > > BTW, do you need to use these functions as gpio_chip callbacks? > If that is the case, they do no need to take struct gpio_chip pointers. > I'm not sure to understand the question, or the idea behind the question. Regards, Thomas
Hi Thomas, On Mon, 12 May 2025 at 17:01, Thomas Richard <thomas.richard@bootlin.com> wrote: > On 5/12/25 16:39, Geert Uytterhoeven wrote: > > On Mon, 12 May 2025 at 16:08, Thomas Richard <thomas.richard@bootlin.com> wrote: > >> On 5/9/25 11:07, Geert Uytterhoeven wrote: > >>> On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: > >>>> Export all symbols and create header file for the GPIO forwarder library. > >>>> It will be used in the next changes. > >>>> > >>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> > >> > >> ... > >> > >>>> + > >>>> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, > >>>> + unsigned long config); > >>>> + > >>>> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); > >>> > >>> I would expect all of these to take gpiochip_fwd pointers instead of > >>> gpio_chip pointers. What prevents you from passing a gpio_chip pointer > >>> that does not correspond to a gpiochip_fwd object, causing a crash? > >> > >> Indeed nothing prevents from passing gpio_chip pointer which does not > >> correspond to a gpiochip_fwd object. > >> And it is also a bit weird to pass a gpiochip_fwd pointer in some cases > >> (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. > >> > >> I can keep GPIO operations as is, and create exported wrappers which > >> take a gpiochip_fwd pointer as parameter, for example: > >> > >> int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, > >> unsigned long *mask, > >> unsigned long *bits) > >> { > >> struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); > >> > >> return gpio_fwd_get_multiple_locked(chip, mask, bits); > >> } > >> EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); > >> > >> So exported functions are gpiochip_fwd_*(). > > > > That sounds fine to me. > > > > BTW, do you need to use these functions as gpio_chip callbacks? > > If that is the case, they do no need to take struct gpio_chip pointers. > > > I'm not sure to understand the question, or the idea behind the question. Do users of the forwarder library want to use these functions directly as callbacks in their own gpiochip? E.g. do they want to use: chip->get_multiple_rv = gpiochip_fwd_get_multiple; I hope my question is more clear now. Thanks! Gr{oetje,eeting}s, Geert
On 5/12/25 17:14, Geert Uytterhoeven wrote: > Hi Thomas, > > On Mon, 12 May 2025 at 17:01, Thomas Richard <thomas.richard@bootlin.com> wrote: >> On 5/12/25 16:39, Geert Uytterhoeven wrote: >>> On Mon, 12 May 2025 at 16:08, Thomas Richard <thomas.richard@bootlin.com> wrote: >>>> On 5/9/25 11:07, Geert Uytterhoeven wrote: >>>>> On Tue, 6 May 2025 at 17:21, Thomas Richard <thomas.richard@bootlin.com> wrote: >>>>>> Export all symbols and create header file for the GPIO forwarder library. >>>>>> It will be used in the next changes. >>>>>> >>>>>> Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> >>>> >>>> ... >>>> >>>>>> + >>>>>> +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, >>>>>> + unsigned long config); >>>>>> + >>>>>> +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); >>>>> >>>>> I would expect all of these to take gpiochip_fwd pointers instead of >>>>> gpio_chip pointers. What prevents you from passing a gpio_chip pointer >>>>> that does not correspond to a gpiochip_fwd object, causing a crash? >>>> >>>> Indeed nothing prevents from passing gpio_chip pointer which does not >>>> correspond to a gpiochip_fwd object. >>>> And it is also a bit weird to pass a gpiochip_fwd pointer in some cases >>>> (for example gpio_fwd_gpio_add()) and a gpio_chip in other cases. >>>> >>>> I can keep GPIO operations as is, and create exported wrappers which >>>> take a gpiochip_fwd pointer as parameter, for example: >>>> >>>> int gpiochip_fwd_get_multiple(struct gpiochip_fwd *fwd, >>>> unsigned long *mask, >>>> unsigned long *bits) >>>> { >>>> struct gpio_chip *gc = gpiochip_fwd_get_gpiochip(fwd); >>>> >>>> return gpio_fwd_get_multiple_locked(chip, mask, bits); >>>> } >>>> EXPORT_SYMBOL_NS_GPL(gpiochip_fwd_get_multiple, "GPIO_FORWARDER"); >>>> >>>> So exported functions are gpiochip_fwd_*(). >>> >>> That sounds fine to me. >>> >>> BTW, do you need to use these functions as gpio_chip callbacks? >>> If that is the case, they do no need to take struct gpio_chip pointers. >>> >> I'm not sure to understand the question, or the idea behind the question. > > Do users of the forwarder library want to use these functions directly > as callbacks in their own gpiochip? > E.g. do they want to use: > > chip->get_multiple_rv = gpiochip_fwd_get_multiple; > > I hope my question is more clear now. Oh ok I understand now. The answer is no (gpiochip_fwd_get_multiple() is already by default the get_multiple_rv operation of the forwarder). My use case (patch 12/12) is: I have a pinctrl driver (for a FPGA) which registers a gpiochip_fwd. The driver has to drive in tandem its configuration and SoC GPIOs (which are added in the gpiochip_fwd). During the probe, the driver will change gpiochip operation to use its own operation. gc = gpiochip_fwd_get_gpiochip(fwd) gc->direction_input = my_direction_input; This function does some custom things and them call gpiochip_fwd_gpio_direction_input(). my_direction_input() { do_something() gpiochip_fwd_gpio_direction_input() } It allows you to add custom action before/after default operation. Regards, Thomas
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c index 45d713e7a702..79fd44c2ceac 100644 --- a/drivers/gpio/gpio-aggregator.c +++ b/drivers/gpio/gpio-aggregator.c @@ -25,6 +25,7 @@ #include <linux/gpio/consumer.h> #include <linux/gpio/driver.h> +#include <linux/gpio/forwarder.h> #include <linux/gpio/machine.h> #define AGGREGATOR_MAX_GPIOS 512 @@ -275,35 +276,81 @@ struct gpiochip_fwd { #define fwd_tmp_size(ngpios) (BITS_TO_LONGS((ngpios)) + (ngpios)) -static int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset) +/** + * gpio_fwd_get_gpiochip - Get the GPIO chip for the GPIO forwarder + * @fwd: GPIO forwarder + * + * Returns: The GPIO chip for the GPIO forwarder + */ +struct gpio_chip *gpio_fwd_get_gpiochip(struct gpiochip_fwd *fwd) +{ + return &fwd->chip; +} +EXPORT_SYMBOL_NS_GPL(gpio_fwd_get_gpiochip, "GPIO_FORWARDER"); + +/** + * gpio_fwd_get_direction - Return the current direction of a GPIO forwarder line + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * + * Returns: 0 for output, 1 for input, or an error code in case of error. + */ +int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return gpiod_get_direction(fwd->descs[offset]); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_get_direction, "GPIO_FORWARDER"); -static int gpio_fwd_direction_input(struct gpio_chip *chip, unsigned int offset) +/** + * gpio_fwd_direction_input - Set a GPIO forwarder line direction to input + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_direction_input(struct gpio_chip *chip, unsigned int offset) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return gpiod_direction_input(fwd->descs[offset]); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_direction_input, "GPIO_FORWARDER"); -static int gpio_fwd_direction_output(struct gpio_chip *chip, - unsigned int offset, int value) +/** + * gpio_fwd_direction_output - Set a GPIO forwarder line direction to output + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * @value: value to set + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_direction_output(struct gpio_chip *chip, unsigned int offset, + int value) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return gpiod_direction_output(fwd->descs[offset], value); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_direction_output, "GPIO_FORWARDER"); -static int gpio_fwd_get(struct gpio_chip *chip, unsigned int offset) +/** + * gpio_fwd_get - Return a GPIO forwarder line's value + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * + * Returns: The GPIO's logical value, i.e. taking the ACTIVE_LOW status into + * account, or negative errno on failure. + */ +int gpio_fwd_get(struct gpio_chip *chip, unsigned int offset) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return chip->can_sleep ? gpiod_get_value_cansleep(fwd->descs[offset]) : gpiod_get_value(fwd->descs[offset]); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_get, "GPIO_FORWARDER"); static int gpio_fwd_get_multiple(struct gpiochip_fwd *fwd, unsigned long *mask, unsigned long *bits) @@ -331,8 +378,18 @@ static int gpio_fwd_get_multiple(struct gpiochip_fwd *fwd, unsigned long *mask, return 0; } -static int gpio_fwd_get_multiple_locked(struct gpio_chip *chip, - unsigned long *mask, unsigned long *bits) +/** + * gpio_fwd_get_multiple_locked - Get values for multiple GPIO forwarder lines + * @chip: GPIO chip in the forwarder + * @mask: bit mask array; one bit per line; BITS_PER_LONG bits per word defines + * which lines are to be read + * @bits: bit value array; one bit per line; BITS_PER_LONG bits per word will + * contains the read values for the lines specified by mask + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_get_multiple_locked(struct gpio_chip *chip, unsigned long *mask, + unsigned long *bits) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); unsigned long flags; @@ -350,6 +407,7 @@ static int gpio_fwd_get_multiple_locked(struct gpio_chip *chip, return error; } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_get_multiple_locked, "GPIO_FORWARDER"); static void gpio_fwd_delay(struct gpio_chip *chip, unsigned int offset, int value) { @@ -372,7 +430,15 @@ static void gpio_fwd_delay(struct gpio_chip *chip, unsigned int offset, int valu udelay(delay_us); } -static int gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value) +/** + * gpio_fwd_set - Assign value to a GPIO forwarder line. + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * @value: value to set + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); int ret; @@ -389,6 +455,7 @@ static int gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value) return ret; } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_set, "GPIO_FORWARDER"); static int gpio_fwd_set_multiple(struct gpiochip_fwd *fwd, unsigned long *mask, unsigned long *bits) @@ -410,8 +477,18 @@ static int gpio_fwd_set_multiple(struct gpiochip_fwd *fwd, unsigned long *mask, return ret; } -static int gpio_fwd_set_multiple_locked(struct gpio_chip *chip, - unsigned long *mask, unsigned long *bits) +/** + * gpio_fwd_set_multiple_locked - Assign values to multiple GPIO forwarder lines + * @chip: GPIO chip in the forwarder + * @mask: bit mask array; one bit per output; BITS_PER_LONG bits per word + * defines which outputs are to be changed + * @bits: bit value array; one bit per output; BITS_PER_LONG bits per word + * defines the values the outputs specified by mask are to be set to + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_set_multiple_locked(struct gpio_chip *chip, unsigned long *mask, + unsigned long *bits) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); unsigned long flags; @@ -429,21 +506,41 @@ static int gpio_fwd_set_multiple_locked(struct gpio_chip *chip, return ret; } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_set_multiple_locked, "GPIO_FORWARDER"); -static int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, - unsigned long config) +/** + * gpio_fwd_set_config - Set @config for a GPIO forwarder line + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * @config: Same packed config format as generic pinconf + * + * Returns: 0 on success, %-ENOTSUPP if the controller doesn't support setting + * the configuration. + */ +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, + unsigned long config) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return gpiod_set_config(fwd->descs[offset], config); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_set_config, "GPIO_FORWARDER"); -static int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset) +/** + * gpio_fwd_to_irq - Return the IRQ corresponding to a GPIO forwarder line + * @chip: GPIO chip in the forwarder + * @offset: the offset of the line + * + * Returns: The IRQ corresponding to the passed line, or an error code in case + * of error. + */ +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); return gpiod_to_irq(fwd->descs[offset]); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_to_irq, "GPIO_FORWARDER"); /* * The GPIO delay provides a way to configure platform specific delays @@ -454,9 +551,9 @@ static int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset) #define FWD_FEATURE_DELAY BIT(0) #ifdef CONFIG_OF_GPIO -static int gpiochip_fwd_delay_of_xlate(struct gpio_chip *chip, - const struct of_phandle_args *gpiospec, - u32 *flags) +static int gpio_fwd_delay_of_xlate(struct gpio_chip *chip, + const struct of_phandle_args *gpiospec, + u32 *flags) { struct gpiochip_fwd *fwd = gpiochip_get_data(chip); struct gpiochip_fwd_timing *timings; @@ -476,7 +573,7 @@ static int gpiochip_fwd_delay_of_xlate(struct gpio_chip *chip, return line; } -static int gpiochip_fwd_setup_delay_line(struct gpiochip_fwd *fwd) +static int gpio_fwd_setup_delay_line(struct gpiochip_fwd *fwd) { struct gpio_chip *chip = &fwd->chip; @@ -486,20 +583,27 @@ static int gpiochip_fwd_setup_delay_line(struct gpiochip_fwd *fwd) if (!fwd->delay_timings) return -ENOMEM; - chip->of_xlate = gpiochip_fwd_delay_of_xlate; + chip->of_xlate = gpio_fwd_delay_of_xlate; chip->of_gpio_n_cells = 3; return 0; } #else -static int gpiochip_fwd_setup_delay_line(struct gpiochip_fwd *fwd) +static int gpio_fwd_setup_delay_line(struct gpiochip_fwd *fwd) { return 0; } #endif /* !CONFIG_OF_GPIO */ -static struct gpiochip_fwd * -devm_gpiochip_fwd_alloc(struct device *dev, unsigned int ngpios) +/** + * devm_gpio_fwd_alloc - Allocate and initialize a new GPIO forwarder + * @dev: Parent device pointer + * @ngpios: Number of GPIOs in the forwarder + * + * Returns: An opaque object pointer, or an ERR_PTR()-encoded negative error + * code on failure. + */ +struct gpiochip_fwd *devm_gpio_fwd_alloc(struct device *dev, unsigned int ngpios) { const char *label = dev_name(dev); struct gpiochip_fwd *fwd; @@ -531,10 +635,18 @@ devm_gpiochip_fwd_alloc(struct device *dev, unsigned int ngpios) return fwd; } +EXPORT_SYMBOL_NS_GPL(devm_gpio_fwd_alloc, "GPIO_FORWARDER"); -static int gpiochip_fwd_gpio_add(struct gpiochip_fwd *fwd, - struct gpio_desc *desc, - unsigned int offset) +/** + * gpio_fwd_gpio_add - Add a GPIO in the forwader + * @fwd: GPIO forwarder + * @desc: GPIO decriptor to register + * @offset: offset for the GPIO in the forwarder + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_gpio_add(struct gpiochip_fwd *fwd, struct gpio_desc *desc, + unsigned int offset) { struct gpio_chip *parent = gpiod_to_chip(desc); struct gpio_chip *chip = &fwd->chip; @@ -561,8 +673,15 @@ static int gpiochip_fwd_gpio_add(struct gpiochip_fwd *fwd, return 0; } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_gpio_add, "GPIO_FORWARDER"); -static int gpiochip_fwd_register(struct gpiochip_fwd *fwd) +/** + * gpio_fwd_register - Register a GPIO forwarder + * @fwd: GPIO forwarder + * + * Returns: 0 on success, or negative errno on failure. + */ +int gpio_fwd_register(struct gpiochip_fwd *fwd) { struct gpio_chip *chip = &fwd->chip; @@ -573,9 +692,10 @@ static int gpiochip_fwd_register(struct gpiochip_fwd *fwd) return devm_gpiochip_add_data(chip->parent, chip, fwd); } +EXPORT_SYMBOL_NS_GPL(gpio_fwd_register, "GPIO_FORWARDER"); /** - * gpiochip_fwd_create() - Create a new GPIO forwarder + * gpio_fwd_create() - Create a new GPIO forwarder * @dev: Parent device pointer * @ngpios: Number of GPIOs in the forwarder. * @descs: Array containing the GPIO descriptors to forward to. @@ -589,32 +709,32 @@ static int gpiochip_fwd_register(struct gpiochip_fwd *fwd) * Return: An opaque object pointer, or an ERR_PTR()-encoded negative error * code on failure. */ -static struct gpiochip_fwd *gpiochip_fwd_create(struct device *dev, - unsigned int ngpios, - struct gpio_desc *descs[], - unsigned long features) +static struct gpiochip_fwd *gpio_fwd_create(struct device *dev, + unsigned int ngpios, + struct gpio_desc *descs[], + unsigned long features) { struct gpiochip_fwd *fwd; unsigned int i; int error; - fwd = devm_gpiochip_fwd_alloc(dev, ngpios); + fwd = devm_gpio_fwd_alloc(dev, ngpios); if (!fwd) return ERR_PTR(-ENOMEM); for (i = 0; i < ngpios; i++) { - error = gpiochip_fwd_gpio_add(fwd, descs[i], i); + error = gpio_fwd_gpio_add(fwd, descs[i], i); if (error) return ERR_PTR(error); } if (features & FWD_FEATURE_DELAY) { - error = gpiochip_fwd_setup_delay_line(fwd); + error = gpio_fwd_setup_delay_line(fwd); if (error) return ERR_PTR(error); } - error = gpiochip_fwd_register(fwd); + error = gpio_fwd_register(fwd); if (error) return ERR_PTR(error); @@ -649,7 +769,7 @@ static int gpio_aggregator_probe(struct platform_device *pdev) } features = (uintptr_t)device_get_match_data(dev); - fwd = gpiochip_fwd_create(dev, n, descs, features); + fwd = gpio_fwd_create(dev, n, descs, features); if (IS_ERR(fwd)) return PTR_ERR(fwd); diff --git a/include/linux/gpio/forwarder.h b/include/linux/gpio/forwarder.h new file mode 100644 index 000000000000..b17ad2c8e031 --- /dev/null +++ b/include/linux/gpio/forwarder.h @@ -0,0 +1,42 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __LINUX_GPIO_FORWARDER_H +#define __LINUX_GPIO_FORWARDER_H + +#include <linux/gpio/consumer.h> +#include <linux/gpio/driver.h> + +struct gpiochip_fwd; + +struct gpio_chip *gpio_fwd_get_gpiochip(struct gpiochip_fwd *fwd); + +int gpio_fwd_get_direction(struct gpio_chip *chip, unsigned int offset); + +int gpio_fwd_direction_input(struct gpio_chip *chip, unsigned int offset); + +int gpio_fwd_direction_output(struct gpio_chip *chip, unsigned int offset, + int value); + +int gpio_fwd_get(struct gpio_chip *chip, unsigned int offset); + +int gpio_fwd_get_multiple_locked(struct gpio_chip *chip, unsigned long *mask, + unsigned long *bits); + +int gpio_fwd_set(struct gpio_chip *chip, unsigned int offset, int value); + +int gpio_fwd_set_multiple_locked(struct gpio_chip *chip, unsigned long *mask, + unsigned long *bits); + +int gpio_fwd_set_config(struct gpio_chip *chip, unsigned int offset, + unsigned long config); + +int gpio_fwd_to_irq(struct gpio_chip *chip, unsigned int offset); + +struct gpiochip_fwd *devm_gpio_fwd_alloc(struct device *dev, + unsigned int ngpios); + +int gpio_fwd_gpio_add(struct gpiochip_fwd *fwd, + struct gpio_desc *desc, unsigned int offset); + +int gpio_fwd_register(struct gpiochip_fwd *fwd); + +#endif
Export all symbols and create header file for the GPIO forwarder library. It will be used in the next changes. Signed-off-by: Thomas Richard <thomas.richard@bootlin.com> --- drivers/gpio/gpio-aggregator.c | 190 +++++++++++++++++++++++++++++++++-------- include/linux/gpio/forwarder.h | 42 +++++++++ 2 files changed, 197 insertions(+), 35 deletions(-)