Re: [PATCH 4/9] gpio: regmap: add the .get_direction() callback

["Michael Walle" <mwalle@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2430 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]