lkml.org 
[lkml]   [2022]   [Nov]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2022-11-23 18:30    [W:0.119 / U:0.696 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site