regmap-irq: support chips with separate mask bits for irq edges

From: Bartosz Golaszewski <[email protected]>

I'm working on an MFD driver (and its accompanying drivers in various subsystems) for a simple low-power PMIC which exposes a single GPIO line. It has a couple of interrupts all bunched together in two registers and all of them but the one for GPIO are controlled by single mask bits. The GPIO interrupt is configured with two separate bits - one for rising edge and one for falling edge interrupts.

Since the device is relatively simple I would really like to avoid having to write the entire irq_chip boiler code if regmap_irq_chip would be perfect in this case.

We already have the type mask fields in struct regmap_irq. This patch proposes a simple change that reuses them. If the mask_base and type_base offsets are the same, the enable callback will use the bits written to the type buffer by the set_type callback to only enable the requested edge interrupt.

Bartosz Golaszewski (1):
regmap: irq: handle HW using separate mask bits for edges

 drivers/base/regmap/regmap-irq.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

2.19.1

From: Bartosz Golaszewski <[email protected]>

Some interrupt controllers use separate bits for controlling rising and falling edge interrupts in the mask register.

Let's reuse the existing type fields in struct regmap_irq to make regmap_irq_chip available to such HW.

If the type_base and mask_base offsets are the same - assume there are separate bits for falling and rising edge interrupts and use the value previously written to the type buffer by the set_type() callback instead of the entire mask specified for this interrupt so that we only enable the requested edge interrupts.

---
 drivers/base/regmap/regmap-irq.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

index 429ca8ed7e51..109ae353c526 100644
@@ -194,8 +194,24 @@ static void regmap_irq_enable(struct irq_data *data)
 	struct regmap_irq_chip_data *d = irq_data_get_irq_chip_data(data);
 	struct regmap *map = d->map;
 	const struct regmap_irq *irq_data = irq_to_regmap_irq(d, data->hwirq);
+	unsigned int mask;
 
-	d->mask_buf[irq_data->reg_offset / map->reg_stride] &= ~irq_data->mask;
+	/*
+	 * If the type_base and mask_base addresses are the same, then
+	 * the underlying hardware uses separate mask bits for rising and
+	 * falling edge interrupts, but we want to make them into a single
+	 * virtual interrupt with configurable edge.
+	 *
+	 * Instead of using the regular mask bits for this interrupt, use
+	 * the value previously written to the type buffer at the
+	 * corresponding offset in regmap_irq_set_type().
+	 */
+	if (unlikely(d->chip->type_base == d->chip->mask_base))
+		mask = d->type_buf[irq_data->reg_offset / map->reg_stride];
+	else
+		mask = irq_data->mask;
+
+	d->mask_buf[irq_data->reg_offset / map->reg_stride] &= ~mask;
 }
 
 static void regmap_irq_disable(struct irq_data *data)

2.19.1

I'm not sure this is ideal, it makes the interface less clear for users especially since there's nothing in the comments in the header that users will look at which mentions the feature.

This feels like it's very strongly tied to a specific implementation of the feature and TBH I'm somewhat unclear on what this ends up concretely meaning. It sounds like this hardware represents the two edges as separate interrupts but you want to combine them into one but I can't see exactly how the interrupt number gets mapped with your change.

śr., 5 gru 2018 o 16:35 Mark Brown <[email protected]> napisał(a):

Yes, it's two edges represented as separate interrupts. I will post a patch with a different (hopefully clearer) approach together with an example snippet from the code actually using it.

Bart

Please don't send cover letters for single patches, if there is anything that needs saying put it in the changelog of the patch or after the --- if it's administrative stuff. This reduces mail volume and ensures that any important information is recorded in the changelog rather than being lost.