On Fri, Aug 16, 2013 at 05:38:37PM +0200, Linus Walleij wrote:
> On Wed, Aug 14, 2013 at 4:59 PM, Florian Lobmaier

> > +       ret = as3722_reg_read(as3722, AS3722_GPIO_SIGNAL_IN_REG, &val);

> These accessors is looking a lot like regmap being reimplemented.

> Have you considered switching the core driver to use regmap?

> This is definately to be fixed since it's using a new core MFD
> driver.

It's common to have inline wrappers for the regmap ops which take the
device specific struct and fish the regmap pointer out of it.

> > +static int as3722_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> > +{
> > +       struct as3722_gpio *as3722_gpio = to_as3722_gpio(chip);
> > +       struct as3722 *as3722 = as3722_gpio->as3722;
> > +
> > +       return regmap_irq_get_virq(as3722->irq_data, AS3722_IRQ_GPIO1 + 
> > offset);
> > +}

> And *here* yoy're using regmap, this looks messy.

> Please use an irqdomain with the handling in the MFD core
> instead so the driver get a translated IRQ base per AS3722 IRQ passed,
> and get rid of this clunkiness.

> For example see how we're handling this in ab8500-core.c
> and abx500-pinctrl.c

This is actually doing the same thing as the ab8500 doing there - all
regmap_irq_get_virq() is doing is calling irq_create_mapping() on the
irqdomain to translate a hwirq into a virq.  It's an API function mostly
because it was created when regmap-irq didn't always use irqdomains,
plus it means drivers don't need to look at the domain.  There's also
regmap_irq_get_domain() which allows the translation to be done directly
in the driver if desired.

The remapping that the MFD core does unfortunately only works
transparently for single IRQs :(

Attachment: signature.asc
Description: Digital signature

Reply via email to