There are GPIO controllers such as the one present in the LX2160ARDB
QIXIS CPLD which are single register fixed-direction. This cannot be
modeled using the gpio-regmap as-is since there is no way to
present the true direction of a GPIO line.
In order to make this use case possible, add a new callback to the
gpio_config structure - .get_direction() - which can be used by user
drivers to provide the fixed direction per GPIO line.
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
drivers/gpio/gpio-regmap.c | 17 ++++++++++++++++-
include/linux/gpio/regmap.h | 3 +++
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 87c4225784cf..dac2acb26655 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -32,6 +32,8 @@ struct gpio_regmap {
unsigned int reg_dir_in_base;
unsigned int reg_dir_out_base;
+ int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
+
int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
unsigned int offset, unsigned int *reg,
unsigned int *mask);
@@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
unsigned int base, val, reg, mask;
int invert, ret;
+ if (gpio->get_direction)
+ return gpio->get_direction(gpio, offset);
+
if (gpio->reg_dat_base && !gpio->reg_set_base)
return GPIO_LINE_DIRECTION_IN;
if (gpio->reg_set_base && !gpio->reg_dat_base)
@@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
{
struct gpio_regmap *gpio = gpiochip_get_data(chip);
unsigned int base, val, reg, mask;
- int invert, ret;
+ int invert, ret, dir;
+
+ if (gpio->get_direction) {
+ dir = gpio->get_direction(gpio, offset);
+ if (dir == GPIO_LINE_DIRECTION_IN && output)
+ return -EOPNOTSUPP;
+ if (dir == GPIO_LINE_DIRECTION_OUT && !output)
+ return -EOPNOTSUPP;
+ return 0;
+ }
if (gpio->reg_dir_out_base) {
base = gpio_regmap_addr(gpio->reg_dir_out_base);
@@ -247,6 +261,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
gpio->reg_clr_base = config->reg_clr_base;
gpio->reg_dir_in_base = config->reg_dir_in_base;
gpio->reg_dir_out_base = config->reg_dir_out_base;
+ gpio->get_direction = config->get_direction;
chip = &gpio->gpio_chip;
chip->parent = config->parent;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index c722c67668c6..99fd973e61fa 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -37,6 +37,8 @@ struct regmap;
* offset to a register/bitmask pair. If not
* given the default gpio_regmap_simple_xlate()
* is used.
+ * @get_direction: (Optional) Callback to the user driver to return the
+ * fixed direction of the GPIO line
* @drvdata: (Optional) Pointer to driver specific data which is
* not used by gpio-remap but is provided "as is" to the
* driver callback(s).
@@ -81,6 +83,7 @@ struct gpio_regmap_config {
int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
unsigned int offset, unsigned int *reg,
unsigned int *mask);
+ int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
void *drvdata;
};
--
2.25.1On Wed, Jul 09, 2025 at 02:26:53PM +0300, Ioana Ciornei wrote:
> There are GPIO controllers such as the one present in the LX2160ARDB
> QIXIS CPLD which are single register fixed-direction. This cannot be
> modeled using the gpio-regmap as-is since there is no way to
> present the true direction of a GPIO line.
>
> In order to make this use case possible, add a new callback to the
> gpio_config structure - .get_direction() - which can be used by user
> drivers to provide the fixed direction per GPIO line.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> drivers/gpio/gpio-regmap.c | 17 ++++++++++++++++-
> include/linux/gpio/regmap.h | 3 +++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 87c4225784cf..dac2acb26655 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -32,6 +32,8 @@ struct gpio_regmap {
> unsigned int reg_dir_in_base;
> unsigned int reg_dir_out_base;
>
> + int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> +
This is not my area, so i will deffer to the GPIO
Maintainers. However, it is not clear to me what get_direction()
should return. Is it the current direction, or the supported
directions? Maybe it should be called get_fixed_direction()?
I then wounder how it will be implemented. Since it is fixed, it is
probably just a constant bitmap, and you look at the offset bit in
this batmap? At minimum a ready made helper could be provided, or
rather than have this op, just provide the bitmap, and gpio-regmap.c
can look at the bit in the bitmap?
Andrew
On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote: > This is not my area, so i will deffer to the GPIO > Maintainers. However, it is not clear to me what get_direction() > should return. This callback should return the current direction as set up in the hardware. A major usecase is that this is called when the gpiochip is registered to read out all the current directions of the GPIO lines, so the kernel has a clear idea of the state of the hardware. Calling this should ideally result in a read of the status from a hardware register. Yours, Linus Walleij
On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote: > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > This is not my area, so i will deffer to the GPIO > > Maintainers. However, it is not clear to me what get_direction() > > should return. > > This callback should return the current direction as set up > in the hardware. > > A major usecase is that this is called when the gpiochip is > registered to read out all the current directions of the GPIO > lines, so the kernel has a clear idea of the state of the > hardware. > > Calling this should ideally result in a read of the status from > a hardware register. O.K, so completely different to what is proposed in this patch. Maybe you can suggest a better name. Andrew
On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote: > On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote: > > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > This is not my area, so i will deffer to the GPIO > > > Maintainers. However, it is not clear to me what get_direction() > > > should return. > > > > This callback should return the current direction as set up > > in the hardware. > > > > A major usecase is that this is called when the gpiochip is > > registered to read out all the current directions of the GPIO > > lines, so the kernel has a clear idea of the state of the > > hardware. > > > > Calling this should ideally result in a read of the status from > > a hardware register. > > O.K, so completely different to what is proposed in this patch. > > Maybe you can suggest a better name. If the hardware only supports one direction, then .get_direction() should return that direction. What the patch does is to read the direction from the hardware and use that in the set_direction() callback, as if all regmapped hardware in the world had fixed direction, that's wrong. I'd just add something custom in gpio-regmap if this is something reoccuring in regmapped GPIO drivers. bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset) or so? Then the core can use is_fixed_direction() together with gpio_get_direction() to check if it can do a certain set_direction(). Pseudocode: mydir = get_direction(line) if (is_fixed_direction(line) && (mydir != requested_dir) return -ERROR; Yours, Linus Walleij
On Fri Jul 11, 2025 at 8:06 PM CEST, Linus Walleij wrote: > On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote: > > On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote: > > > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > This is not my area, so i will deffer to the GPIO > > > > Maintainers. However, it is not clear to me what get_direction() > > > > should return. > > > > > > This callback should return the current direction as set up > > > in the hardware. > > > > > > A major usecase is that this is called when the gpiochip is > > > registered to read out all the current directions of the GPIO > > > lines, so the kernel has a clear idea of the state of the > > > hardware. > > > > > > Calling this should ideally result in a read of the status from > > > a hardware register. > > > > O.K, so completely different to what is proposed in this patch. > > > > Maybe you can suggest a better name. > > If the hardware only supports one direction, then .get_direction() > should return that direction. > > What the patch does is to > read the direction from the hardware and use that in the > set_direction() callback, as if all regmapped hardware in the > world had fixed direction, that's wrong. > > I'd just add something custom in gpio-regmap if this is > something reoccuring in regmapped GPIO drivers. > > bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset) > > or so? > > Then the core can use is_fixed_direction() together > with gpio_get_direction() to check if it can do > a certain set_direction(). > > Pseudocode: > > mydir = get_direction(line) > if (is_fixed_direction(line) && (mydir != requested_dir) > return -ERROR; You don't need a .is_fixed_direction(). You can deduce that if only .get_direction() is set in the gpio-regmap config. mydir = get_direction(line) if (!config->set_direction && mydir != requested_dir) return -ERROR; That or either Andrew's idea of setting a bitmap within the gpio-regmap config which already tells the gpio-regmap core and then amend gpio_regmap_get_direction() to return that fixed direction if that bitmap is not NULL. I'm fine with both. -michael
On Mon, Jul 14, 2025 at 08:36:03AM +0200, Michael Walle wrote: > On Fri Jul 11, 2025 at 8:06 PM CEST, Linus Walleij wrote: > > On Fri, Jul 11, 2025 at 7:45 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > On Fri, Jul 11, 2025 at 07:43:13PM +0200, Linus Walleij wrote: > > > > On Wed, Jul 9, 2025 at 5:09 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > > > > > > > This is not my area, so i will deffer to the GPIO > > > > > Maintainers. However, it is not clear to me what get_direction() > > > > > should return. > > > > > > > > This callback should return the current direction as set up > > > > in the hardware. > > > > > > > > A major usecase is that this is called when the gpiochip is > > > > registered to read out all the current directions of the GPIO > > > > lines, so the kernel has a clear idea of the state of the > > > > hardware. > > > > > > > > Calling this should ideally result in a read of the status from > > > > a hardware register. > > > > > > O.K, so completely different to what is proposed in this patch. > > > > > > Maybe you can suggest a better name. > > > > If the hardware only supports one direction, then .get_direction() > > should return that direction. > > > > What the patch does is to > > read the direction from the hardware and use that in the > > set_direction() callback, as if all regmapped hardware in the > > world had fixed direction, that's wrong. > > > > I'd just add something custom in gpio-regmap if this is > > something reoccuring in regmapped GPIO drivers. > > > > bool is_fixed_direction(struct gpio_regmap *gpio, unsigned int offset) > > > > or so? > > > > Then the core can use is_fixed_direction() together > > with gpio_get_direction() to check if it can do > > a certain set_direction(). > > > > Pseudocode: > > > > mydir = get_direction(line) > > if (is_fixed_direction(line) && (mydir != requested_dir) > > return -ERROR; > > You don't need a .is_fixed_direction(). You can deduce that if only > .get_direction() is set in the gpio-regmap config. > > mydir = get_direction(line) > if (!config->set_direction && mydir != requested_dir) > return -ERROR; This implies that gpio_regmap_config gets two new callbacks .get_direction() and .set_direction() and that in case .set_direction() is set in gpio-regmap config, then its used directly from gpio_regmap_set_direction(), right? > > That or either Andrew's idea of setting a bitmap within the > gpio-regmap config which already tells the gpio-regmap core and then > amend gpio_regmap_get_direction() to return that fixed direction if > that bitmap is not NULL. Even though at first glance I was under the impression that the bitmap solution is cleaner, how big should the bitmap be knows only the final gpio driver. Without this information, we cannot know the bitmap size so that we can use the DECLARE_BITMAP macro in gpio-regmap config. Ioana
Hi,
> > > Then the core can use is_fixed_direction() together
> > > with gpio_get_direction() to check if it can do
> > > a certain set_direction().
> > >
> > > Pseudocode:
> > >
> > > mydir = get_direction(line)
> > > if (is_fixed_direction(line) && (mydir != requested_dir)
> > > return -ERROR;
> >
> > You don't need a .is_fixed_direction(). You can deduce that if only
> > .get_direction() is set in the gpio-regmap config.
> >
> > mydir = get_direction(line)
> > if (!config->set_direction && mydir != requested_dir)
> > return -ERROR;
>
> This implies that gpio_regmap_config gets two new callbacks
> .get_direction() and .set_direction() and that in case .set_direction()
> is set in gpio-regmap config, then its used directly from
> gpio_regmap_set_direction(), right?
Yes. Or just .get_direction() for now and assume that .set_direction
is NULL, i.e. it just covers your use case for the fixed direction.
.. Oh I just noticed that this will really limit the use to either
all or nothing. You cannot mix set user defined directions with
fixed directions. Linus' .is_fixed_direction() will allow that.
Though I wonder if we really want to let the user override
.get_direction() and .set_direction(). I still prefer the bitmap.
> > That or either Andrew's idea of setting a bitmap within the
> > gpio-regmap config which already tells the gpio-regmap core and then
> > amend gpio_regmap_get_direction() to return that fixed direction if
> > that bitmap is not NULL.
>
> Even though at first glance I was under the impression that the bitmap
> solution is cleaner, how big should the bitmap be knows only the final
> gpio driver. Without this information, we cannot know the bitmap size so
> that we can use the DECLARE_BITMAP macro in gpio-regmap config.
Actually, I had the same thought. But there is also bitmap_alloc()
and friends, no? And the gpio-regmap config contains the ngpios.
In gpio_regmap_get_direction():
if (gpio->fixed_direction_output && test_bit(offset, gpio->fixed_direction_output))
return GPIO_LINE_DIRECTION_OUT;
Which implies that once .fixed_direction is set it will always be
checked. So if someone in the future wants to mix and match
.fixed_direction with .reg_dir_{in,out}_base we have to add a second
bitmap which tells you what pins are fixed.
You'd probably need to make sure offset is smaller than ngpio.
-michael
On Wed, Jul 09, 2025 at 05:09:17PM +0200, Andrew Lunn wrote:
> On Wed, Jul 09, 2025 at 02:26:53PM +0300, Ioana Ciornei wrote:
> > There are GPIO controllers such as the one present in the LX2160ARDB
> > QIXIS CPLD which are single register fixed-direction. This cannot be
> > modeled using the gpio-regmap as-is since there is no way to
> > present the true direction of a GPIO line.
> >
> > In order to make this use case possible, add a new callback to the
> > gpio_config structure - .get_direction() - which can be used by user
> > drivers to provide the fixed direction per GPIO line.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > drivers/gpio/gpio-regmap.c | 17 ++++++++++++++++-
> > include/linux/gpio/regmap.h | 3 +++
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 87c4225784cf..dac2acb26655 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -32,6 +32,8 @@ struct gpio_regmap {
> > unsigned int reg_dir_in_base;
> > unsigned int reg_dir_out_base;
> >
> > + int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> > +
>
> This is not my area, so i will deffer to the GPIO
> Maintainers. However, it is not clear to me what get_direction()
> should return. Is it the current direction, or the supported
> directions? Maybe it should be called get_fixed_direction()?
>
> I then wounder how it will be implemented. Since it is fixed, it is
> probably just a constant bitmap, and you look at the offset bit in
> this batmap? At minimum a ready made helper could be provided, or
> rather than have this op, just provide the bitmap, and gpio-regmap.c
> can look at the bit in the bitmap?
>
That is indeed possible and would save us from an extra callback that
doesn't do much, it just looks into the same kind of bitmap that
gpio-regmap could just have access to directly.
I will wait for feedback from the GPIO maintainers but I do like the
idea.
Thanks!
> just provide the bitmap, and gpio-regmap.c can look at the bit in > the bitmap? I like that idea. -michael
Hi Ioana,
great to see another user of gpio-regmap.
On Wed Jul 9, 2025 at 1:26 PM CEST, Ioana Ciornei wrote:
> There are GPIO controllers such as the one present in the LX2160ARDB
> QIXIS CPLD which are single register fixed-direction. This cannot be
> modeled using the gpio-regmap as-is since there is no way to
> present the true direction of a GPIO line.
You mean input and output mixed together in one register? At least
to me, that wasn't so obvious by the commit message, I had to look
at the actual driver.
> In order to make this use case possible, add a new callback to the
> gpio_config structure - .get_direction() - which can be used by user
> drivers to provide the fixed direction per GPIO line.
>
> Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> ---
> drivers/gpio/gpio-regmap.c | 17 ++++++++++++++++-
> include/linux/gpio/regmap.h | 3 +++
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 87c4225784cf..dac2acb26655 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -32,6 +32,8 @@ struct gpio_regmap {
> unsigned int reg_dir_in_base;
> unsigned int reg_dir_out_base;
>
> + int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> +
> int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> unsigned int offset, unsigned int *reg,
> unsigned int *mask);
> @@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
> unsigned int base, val, reg, mask;
> int invert, ret;
>
> + if (gpio->get_direction)
> + return gpio->get_direction(gpio, offset);
> +
> if (gpio->reg_dat_base && !gpio->reg_set_base)
> return GPIO_LINE_DIRECTION_IN;
> if (gpio->reg_set_base && !gpio->reg_dat_base)
> @@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
> {
> struct gpio_regmap *gpio = gpiochip_get_data(chip);
> unsigned int base, val, reg, mask;
> - int invert, ret;
> + int invert, ret, dir;
> +
> + if (gpio->get_direction) {
> + dir = gpio->get_direction(gpio, offset);
> + if (dir == GPIO_LINE_DIRECTION_IN && output)
> + return -EOPNOTSUPP;
> + if (dir == GPIO_LINE_DIRECTION_OUT && !output)
> + return -EOPNOTSUPP;
> + return 0;
> + }
What is the intention here? Shouldn't there be just a .set_direction
op and if there isn't one, return EOPNOTSUPP?
In any case, that is unused code for your driver as far as I see,
because you neither set .reg_dir_in_base nor .reg_dir_out_base and
thus, .direction_input nor .direction_output are set within the
gpio_chip struct (see gpio_regmap_register()).
> if (gpio->reg_dir_out_base) {
> base = gpio_regmap_addr(gpio->reg_dir_out_base);
> @@ -247,6 +261,7 @@ struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
> gpio->reg_clr_base = config->reg_clr_base;
> gpio->reg_dir_in_base = config->reg_dir_in_base;
> gpio->reg_dir_out_base = config->reg_dir_out_base;
> + gpio->get_direction = config->get_direction;
>
> chip = &gpio->gpio_chip;
> chip->parent = config->parent;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index c722c67668c6..99fd973e61fa 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -37,6 +37,8 @@ struct regmap;
> * offset to a register/bitmask pair. If not
> * given the default gpio_regmap_simple_xlate()
> * is used.
> + * @get_direction: (Optional) Callback to the user driver to return the
> + * fixed direction of the GPIO line
> * @drvdata: (Optional) Pointer to driver specific data which is
> * not used by gpio-remap but is provided "as is" to the
> * driver callback(s).
> @@ -81,6 +83,7 @@ struct gpio_regmap_config {
> int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> unsigned int offset, unsigned int *reg,
> unsigned int *mask);
> + int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
>
> void *drvdata;
> };
On Wed, Jul 09, 2025 at 05:01:38PM +0200, Michael Walle wrote:
> Hi Ioana,
>
> great to see another user of gpio-regmap.
>
> On Wed Jul 9, 2025 at 1:26 PM CEST, Ioana Ciornei wrote:
> > There are GPIO controllers such as the one present in the LX2160ARDB
> > QIXIS CPLD which are single register fixed-direction. This cannot be
> > modeled using the gpio-regmap as-is since there is no way to
> > present the true direction of a GPIO line.
>
> You mean input and output mixed together in one register? At least
> to me, that wasn't so obvious by the commit message, I had to look
> at the actual driver.
>
Yes, that is right. I will update the commit message to make it more
clear for everybody.
> > In order to make this use case possible, add a new callback to the
> > gpio_config structure - .get_direction() - which can be used by user
> > drivers to provide the fixed direction per GPIO line.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > drivers/gpio/gpio-regmap.c | 17 ++++++++++++++++-
> > include/linux/gpio/regmap.h | 3 +++
> > 2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> > index 87c4225784cf..dac2acb26655 100644
> > --- a/drivers/gpio/gpio-regmap.c
> > +++ b/drivers/gpio/gpio-regmap.c
> > @@ -32,6 +32,8 @@ struct gpio_regmap {
> > unsigned int reg_dir_in_base;
> > unsigned int reg_dir_out_base;
> >
> > + int (*get_direction)(struct gpio_regmap *gpio, unsigned int offset);
> > +
> > int (*reg_mask_xlate)(struct gpio_regmap *gpio, unsigned int base,
> > unsigned int offset, unsigned int *reg,
> > unsigned int *mask);
> > @@ -129,6 +131,9 @@ static int gpio_regmap_get_direction(struct gpio_chip *chip,
> > unsigned int base, val, reg, mask;
> > int invert, ret;
> >
> > + if (gpio->get_direction)
> > + return gpio->get_direction(gpio, offset);
> > +
> > if (gpio->reg_dat_base && !gpio->reg_set_base)
> > return GPIO_LINE_DIRECTION_IN;
> > if (gpio->reg_set_base && !gpio->reg_dat_base)
> > @@ -163,7 +168,16 @@ static int gpio_regmap_set_direction(struct gpio_chip *chip,
> > {
> > struct gpio_regmap *gpio = gpiochip_get_data(chip);
> > unsigned int base, val, reg, mask;
> > - int invert, ret;
> > + int invert, ret, dir;
> > +
> > + if (gpio->get_direction) {
> > + dir = gpio->get_direction(gpio, offset);
> > + if (dir == GPIO_LINE_DIRECTION_IN && output)
> > + return -EOPNOTSUPP;
> > + if (dir == GPIO_LINE_DIRECTION_OUT && !output)
> > + return -EOPNOTSUPP;
> > + return 0;
> > + }
>
> What is the intention here? Shouldn't there be just a .set_direction
> op and if there isn't one, return EOPNOTSUPP?
>
> In any case, that is unused code for your driver as far as I see,
> because you neither set .reg_dir_in_base nor .reg_dir_out_base and
> thus, .direction_input nor .direction_output are set within the
> gpio_chip struct (see gpio_regmap_register()).
>
Yes, you are right. I did want to return ealier an EOPNOTSUPP to make
sure that in my specific case no directions would be changed. But that
is not needed indeed since, as you said, I do not use .reg_dir_in_base
nor .reg_dir_out_base.
I will remove this in v2.
And since we are on the subject, I will try out Andrew's suggestion with
giving gpio-regmap a bitmap to use directly.
Thanks!
Ioana
© 2016 - 2025 Red Hat, Inc.