Thread (39 messages) flat view 39 messages, 6 authors, 13d ago

Re: [PATCH v4 12/14] gpio: regmap: Add optional runtime PM support

From: Andy Shevchenko <hidden>
Date: 2026-08-24 08:05:30
Also in: linux-doc, linux-gpio, linux-iio, lkml

On Fri, Aug 21, 2026 at 04:07: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.
struct gpio_regmap {
 	unsigned int reg_clr_base;
 	unsigned int reg_dir_in_base;
 	unsigned int reg_dir_out_base;
+	struct device *pm_dev;
Please, move it out of these fields, the below is coupled with reg_dir_*
in some way.

Having this somewhere near to

	struct gpio_chip gpio_chip;

above makes more sense.
 	unsigned long *fixed_direction_mask;
 	unsigned long *fixed_direction_output;
 };
...
+static int gpio_regmap_runtime_get(struct gpio_regmap *gpio)
+{
+	if (!gpio->pm_dev)
+		return 0;
+
+	return pm_runtime_get_active(gpio->pm_dev, RPM_TRANSPARENT);
+}
+
+static void gpio_regmap_runtime_put(struct gpio_regmap *gpio)
+{
+	if (!gpio->pm_dev)
+		return;
+
+	pm_runtime_put_autosuspend(gpio->pm_dev);
+}
+DEFINE_GUARD(gpio_regmap_runtime, struct gpio_regmap *,
+	     gpio_regmap_runtime_get(_T), gpio_regmap_runtime_put(_T))
This is not used, it's for guard()() case.
+DEFINE_GUARD_COND(gpio_regmap_runtime, _try,
+		  gpio_regmap_runtime_get(_T), _RET == 0)
Yep, but also makes sense to add

#define GPIO_REGMAP_RUNTIME_ACQUIRE(_dev, _var)			\
	ACQUIRE(gpio_regmap_runtime_try, _var)(_dev)

#define GPIO_REGMAP_RUNTIME_ACQUIRE_ERR(_var_ptr)		\
	ACQUIRE_ERR(gpio_regmap_runtime, _var_ptr)

...
+	chip->can_sleep = config->pm_dev ||
+			  regmap_might_sleep(config->regmap);
Fits a single line.

...
  * @ngpio_per_reg:	(Optional) Number of GPIOs per register
  * @irq_domain:		(Optional) IRQ domain if the controller is
  *			interrupt-capable
+ * @pm_dev:		(Optional) Device to use for runtime power management.
  * @reg_mask_xlate:     (Optional) Translates base address and GPIO
  *			offset to a register/bitmask pair. If not
  *			given the default gpio_regmap_simple_xlate()
struct gpio_regmap_config {
 	int reg_stride;
 	int ngpio_per_reg;
 	struct irq_domain *irq_domain;
+	struct device *pm_dev;
 	unsigned long *fixed_direction_mask;
 	unsigned long *fixed_direction_output;
Oh, this needs synchronisation of the ordering of members and kernel-doc
descriptions. Not a problem for you or in your patch series, just let's
try to find the best fit for a new member from day 1.

Like in the above local structure I suggest to put this just after

	struct regmap *regmap;

-- 
With Best Regards,
Andy Shevchenko

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help