Re: [PATCH v3 12/14] gpio: regmap: Add optional runtime PM support
Jonathan Cameron <[email protected]>
| Newsgroups | org.kernel.vger.linux-iio,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-doc,org.kernel.vger.linux-gpio,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260816185949.4205b507@jic23-huawei> |
On Fri, 14 Aug 2026 11:34:13 +0300 Andy Shevchenko <[email protected]> wrote: > On Thu, Aug 13, 2026 at 03:57:05PM +0200, Janani Sunil wrote: > > Some gpio-regmap consumers share their regmap with a parent device that > > may be runtime suspended. GPIO register accesses must resume that device > > first. > > > > Add an optional pm_dev field and acquire it before register translation > > or access. Release it using runtime autosuspend after each operation. > > Keep the device active across the complete direction-output sequence and > > propagate failure when setting the initial output value. > > ... > > > static int gpio_regmap_get(struct gpio_chip *chip, unsigned int offset) > > { > > struct gpio_regmap *gpio = gpiochip_get_data(chip); > > > > > - ret = gpio->reg_mask_xlate(gpio, base, offset, ®, &mask); > > + ret = gpio_regmap_runtime_get(gpio); > > if (ret) > > return ret; > > > > + ret = gpio->reg_mask_xlate(gpio, base, offset, ®, &mask); > > + if (ret) > > + goto out_pm_put; > > + > > /* ensure we don't spoil any register cache with pin input values */ > > if (gpio->reg_dat_base == gpio->reg_set_base) { > > ret = regmap_read_bypassed(gpio->regmap, reg, &val); > > if (ret) > > - return ret; > > + goto out_pm_put; > > > > - return !!(val & mask); > > + ret = !!(val & mask); > > + } else { > > + ret = regmap_test_bits(gpio->regmap, reg, mask); > > } > > > > - return regmap_test_bits(gpio->regmap, reg, mask); > > +out_pm_put: > > + gpio_regmap_runtime_put(gpio); > > + return ret; > > } > > Instead of adding ugly goto:s, I would rather define the guard and acquire and > use in the same way as other PM_RUNTIME_ACQUIRE*() work. > Excellent point Andy. I was thinking exactly this when looking at v2 just now (and feeling guilty for being late to the discussion!) Would need a local GPIO specific variant but that is easy to do - just copy the way PM_RUNTIME_ACQUIRE*() works. Given how much it will reduce the code churn + result in a more elegant result for me this is a necessary change rather than a nice to have. Thanks, Jonathan