Messages in this thread | | | Date | Wed, 23 Nov 2022 19:28:50 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v3 4/9] gpio: 104-idi-48: Migrate to the regmap-irq API |
| |
On Tue, Nov 22, 2022 at 02:11:01AM -0500, William Breathitt Gray wrote: > The regmap API supports IO port accessors so we can take advantage of > regmap abstractions rather than handling access to the device registers > directly in the driver. > > For the 104-idi-48, we get an IRQ register with some status information > and basic masking, but it's broken down by banks rather than individual > GPIO. There are six banks (8 GPIO lines each) that correspond to the > lower six bits of the IRQ register (bits 0-5): > > Base Address + 7 (Read): IRQ Status Register/IRQ Clear > Bit 0-5: Respective Bank IRQ Statuses > Bit 6: IRQ Status (Active Low) > Bit 7: IRQ Enable Status > Base Address + 7 (Write): IRQ Enable/Disable > Bit 0-5: Respective Bank IRQ Enable/Disable
> Cc: Mark Brown <broonie@kernel.org>
Hint: you may use --cc parameter to `git send-email ...` to Cc additional people.
...
> +#define IDI48_REGMAP_IRQ(_id) \ > + [_id] = { \ > + .mask = BIT((_id) / 8), \ > + .type = { .types_supported = IRQ_TYPE_EDGE_BOTH, }, \ > }
Same comments as per previous patch.
...
> +static const struct regmap_irq idi48_regmap_irqs[IDI48_NGPIO] = { > + IDI48_REGMAP_IRQ(0), IDI48_REGMAP_IRQ(1), IDI48_REGMAP_IRQ(2), > + IDI48_REGMAP_IRQ(3), IDI48_REGMAP_IRQ(4), IDI48_REGMAP_IRQ(5), > + IDI48_REGMAP_IRQ(6), IDI48_REGMAP_IRQ(7), IDI48_REGMAP_IRQ(8), > + IDI48_REGMAP_IRQ(9), IDI48_REGMAP_IRQ(10), IDI48_REGMAP_IRQ(11), > + IDI48_REGMAP_IRQ(12), IDI48_REGMAP_IRQ(13), IDI48_REGMAP_IRQ(14), > + IDI48_REGMAP_IRQ(15), IDI48_REGMAP_IRQ(16), IDI48_REGMAP_IRQ(17), > + IDI48_REGMAP_IRQ(18), IDI48_REGMAP_IRQ(19), IDI48_REGMAP_IRQ(20), > + IDI48_REGMAP_IRQ(21), IDI48_REGMAP_IRQ(22), IDI48_REGMAP_IRQ(23), > + IDI48_REGMAP_IRQ(24), IDI48_REGMAP_IRQ(25), IDI48_REGMAP_IRQ(26), > + IDI48_REGMAP_IRQ(27), IDI48_REGMAP_IRQ(28), IDI48_REGMAP_IRQ(29), > + IDI48_REGMAP_IRQ(30), IDI48_REGMAP_IRQ(31), IDI48_REGMAP_IRQ(32), > + IDI48_REGMAP_IRQ(33), IDI48_REGMAP_IRQ(34), IDI48_REGMAP_IRQ(35), > + IDI48_REGMAP_IRQ(36), IDI48_REGMAP_IRQ(37), IDI48_REGMAP_IRQ(38), > + IDI48_REGMAP_IRQ(39), IDI48_REGMAP_IRQ(40), IDI48_REGMAP_IRQ(41), > + IDI48_REGMAP_IRQ(42), IDI48_REGMAP_IRQ(43), IDI48_REGMAP_IRQ(44), > + IDI48_REGMAP_IRQ(45), IDI48_REGMAP_IRQ(46), IDI48_REGMAP_IRQ(47),
Perhaps here would be nice to have it like
IDI48_REGMAP_IRQ(45), IDI48_REGMAP_IRQ(46), IDI48_REGMAP_IRQ(47), /* 45-47 */
?
> +};
...
> + err = devm_regmap_add_irq_chip(dev, map, irq[id], IRQF_SHARED, 0, chip, > + &chip_data); > + if (err) {
> + dev_err(dev, "IRQ registration failed (%d)\n", err); > + return err;
I'm wondering if you plan to switch to dev_err_probe() at some point.
> + }
-- With Best Regards, Andy Shevchenko
| |