regmap-irq: Minor adjustments to .handle_mask_sync()
authorAidan MacDonald <aidanmacdonald.0x0@gmail.com>
Thu, 11 May 2023 09:13:41 +0000 (10:13 +0100)
committerMark Brown <broonie@kernel.org>
Fri, 12 May 2023 01:28:02 +0000 (10:28 +0900)
If a .handle_mask_sync() callback is provided it supersedes all
inbuilt handling of mask registers, and judging by the commit
69af4bcaa08d ("regmap-irq: Add handle_mask_sync() callback") it
is intended to completely replace all default IRQ masking logic.

The implementation has two minor inconsistencies, which can be
fixed without breaking compatibility:

(1) mask_base must be set to enable .handle_mask_sync(), even
    though mask_base is otherwise unused. This is easily fixed
    because mask_base is already optional.

(2) Unmask registers aren't accounted for -- they are part of
    the default IRQ masking logic and are just a bit-inverted
    version of mask registers. It would be a bad idea to allow
    them to be used at the same time as .handle_mask_sync(),
    as the result would be confusing and unmaintainable, so
    make sure this can't happen.

Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com
Link: https://lore.kernel.org/r/20230511091342.26604-4-aidanmacdonald.0x0@gmail.com
Signed-off-by: Mark Brown <broonie@kernel.org
drivers/base/regmap/regmap-irq.c

index 7cb457af2332096ca73421fc86dd579df80790e0..95bf457204bf26df414202e4df98067ba210be17 100644 (file)
@@ -113,23 +113,21 @@ static void regmap_irq_sync_unlock(struct irq_data *data)
         * suppress pointless writes.
         */
        for (i = 0; i < d->chip->num_regs; i++) {
-               if (d->mask_base) {
-                       if (d->chip->handle_mask_sync)
-                               d->chip->handle_mask_sync(i, d->mask_buf_def[i],
-                                                         d->mask_buf[i],
-                                                         d->chip->irq_drv_data);
-                       else {
-                               reg = d->get_irq_reg(d, d->mask_base, i);
-                               ret = regmap_update_bits(d->map, reg,
-                                               d->mask_buf_def[i],
-                                               d->mask_buf[i]);
-                               if (ret)
-                                       dev_err(d->map->dev, "Failed to sync masks in %x\n",
-                                               reg);
-                       }
+               if (d->chip->handle_mask_sync)
+                       d->chip->handle_mask_sync(i, d->mask_buf_def[i],
+                                                 d->mask_buf[i],
+                                                 d->chip->irq_drv_data);
+
+               if (d->mask_base && !d->chip->handle_mask_sync) {
+                       reg = d->get_irq_reg(d, d->mask_base, i);
+                       ret = regmap_update_bits(d->map, reg,
+                                                d->mask_buf_def[i],
+                                                d->mask_buf[i]);
+                       if (ret)
+                               dev_err(d->map->dev, "Failed to sync masks in %x\n", reg);
                }
 
-               if (d->unmask_base) {
+               if (d->unmask_base && !d->chip->handle_mask_sync) {
                        reg = d->get_irq_reg(d, d->unmask_base, i);
                        ret = regmap_update_bits(d->map, reg,
                                        d->mask_buf_def[i], ~d->mask_buf[i]);
@@ -785,28 +783,27 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
        for (i = 0; i < chip->num_regs; i++) {
                d->mask_buf[i] = d->mask_buf_def[i];
 
-               if (d->mask_base) {
-                       if (chip->handle_mask_sync) {
-                               ret = chip->handle_mask_sync(i,
-                                                            d->mask_buf_def[i],
-                                                            d->mask_buf[i],
-                                                            chip->irq_drv_data);
-                               if (ret)
-                                       goto err_alloc;
-                       } else {
-                               reg = d->get_irq_reg(d, d->mask_base, i);
-                               ret = regmap_update_bits(d->map, reg,
-                                               d->mask_buf_def[i],
-                                               d->mask_buf[i]);
-                               if (ret) {
-                                       dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
-                                               reg, ret);
-                                       goto err_alloc;
-                               }
+               if (chip->handle_mask_sync) {
+                       ret = chip->handle_mask_sync(i, d->mask_buf_def[i],
+                                                    d->mask_buf[i],
+                                                    chip->irq_drv_data);
+                       if (ret)
+                               goto err_alloc;
+               }
+
+               if (d->mask_base && !chip->handle_mask_sync) {
+                       reg = d->get_irq_reg(d, d->mask_base, i);
+                       ret = regmap_update_bits(d->map, reg,
+                                                d->mask_buf_def[i],
+                                                d->mask_buf[i]);
+                       if (ret) {
+                               dev_err(map->dev, "Failed to set masks in 0x%x: %d\n",
+                                       reg, ret);
+                               goto err_alloc;
                        }
                }
 
-               if (d->unmask_base) {
+               if (d->unmask_base && !chip->handle_mask_sync) {
                        reg = d->get_irq_reg(d, d->unmask_base, i);
                        ret = regmap_update_bits(d->map, reg,
                                        d->mask_buf_def[i], ~d->mask_buf[i]);