Re: [PATCH v3 3/7] gpio: regmap: Add gpio_regmap_operation and write-enable support

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Thu, Jul 16, 2026 at 12:55:37PM +0200, Michael Walle wrote:
> On Thu Jul 16, 2026 at 11:40 AM CEST, Andy Shevchenko wrote:
> > On Thu, Jul 16, 2026 at 11:08:55AM +0200, Michael Walle wrote:
> >> On Thu Jul 16, 2026 at 10:27 AM CEST, Andy Shevchenko wrote:
> >> > On Thu, Jul 16, 2026 at 02:26:14PM +0800, Yu-Chun Lin wrote:

...

> >> > From the above list I tend to the approach 2, but this might require to have
> >> > GPIO regmap level of locking. I'm a bit lost in the context, though. I assume
> >> > we need a fresh start, id est issue a v6 with approach 2 or 3 in place and
> >> > summarize the choices in the cover letter, so we can understand what has been
> >> > considered.
> >> 
> >> I don't really like approach 3. You'd need to check if the regs of
> >> both xlate calls are the same. With the sample code above, you
> >> silently drop the first xlate'd reg.
> >
> > If I rank the proposals, the worst is #1, the best is #2.
> >
> >> And honestly, it really seems like a one-off. What controllers, are
> >> there that need a write enable bit. The real problem seems to be
> >> the assumption that we operate on just one bit. IOW we either set
> >> mask or don't set mask in gpio_regmap_set().
> >
> > Yes, we should KISS.
> 
> But IMHO #2 and #3 are not KISS. Approach 2 is just a way of adding
> some kind of pre op to a gpio set. Just tying it to a write enable
> feature. That kinda bothers me. It might also be useful for other
> things, too. So don't tie it to just write enable. And who is doing
> a write disable if it's not self clearing for example. Probably Some
> kind of post op :)
> 
> Approach 3 is a way to change the value of the written value - in
> a restricted way, as is is just doing a OR with both values.
> 
> Also approach 2 might not even work if the hardware requires the
> write enable bit set in the *same* write as the gpio set bit. Thus,
> we might need both anyway in the future.
> 
> >> For a more generic solution, we should be able to control the
> >> written value. We could add another .value_xlate().
> >
> > Maybe not now?
> 
> But if not now, then when? I wouldn't add the write enable feature
> and later a more generic solution which also covers the write enable
> feature.

Taking into account how it's done in HW, I think the whole approach can
be folded to just a boolean flag  (or a simply bit shift) in the config.
Wouldn't it work?

> > As per IPs, Synopsys IPs (not exactly GPIO) likes to
> > have that kind of "protection". So, from HW perspective it's kinda
> > pattern, and it might be possible to see more IPs (including GPIO)
> > that follow it in some cases.

-- 
With Best Regards,
Andy Shevchenko