On Sun, Jun 10, 2018 at 06:56:21PM +1000, NeilBrown wrote:
> On Sat, Jun 09 2018, Sergio Paracuellos wrote:
>
> > When implementing custom irqchips it is important to also
> > implement .irq_request_resources() and .irq_release_resources()
> > and make sure these call gpiochip_[un]lock_as_irq().
> > Add those two for this driver. Also store struct device pointer
> > in global state structure to be able to use 'dev_err' with the
> > device from 'mediatek_request_resources' function.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> > ---
> > drivers/staging/mt7621-gpio/gpio-mt7621.c | 42 +++++++++++++++++++++++++++----
> > 1 file changed, 37 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > index 654c9eb..2a1c506 100644
> > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > @@ -40,6 +40,7 @@ struct mtk_gc {
> > };
> >
> > struct mtk_data {
> > + struct device *dev;
> > void __iomem *gpio_membase;
> > int gpio_irq;
> > struct irq_domain *gpio_irq_domain;
> > @@ -229,12 +230,42 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type)
> > return 0;
> > }
> >
> > +static int mediatek_irq_reqres(struct irq_data *d)
> > +{
> > + struct mtk_data *gpio_data = irq_data_get_irq_chip_data(d);
>
> By the end of the patch set, irq_data_get_irq_chip_data returns a
> 'struct gpio_chip' here.
> You can use container_of to get the 'rg', and you don't need 'bank' at
> all.
Function 'container_of' was a little confused and only
was used in the deleted 'gpio_to_irq' so I prefer to delete it also.
Is a wrong approach to use bank instead?
>
> The same applies in several places.
> It might not be this patch that introduces the problem, but this was the
> easiest place to point to it.
>
> Thanks,
> NeilBrown
Best regards,
Sergio Paracuellos
>
>
> > + int bank = irqd_to_hwirq(d) / MTK_BANK_WIDTH;
> > + struct mtk_gc *rg = &gpio_data->gc_map[bank];
> > + struct gpio_chip *gc = &rg->chip;
> > + int ret;
> > +
> > + ret = gpiochip_lock_as_irq(gc, irqd_to_hwirq(d));
> > + if (ret) {
> > + dev_err(gpio_data->dev, "unable to lock HW IRQ %lu for IRQ\n",
> > + irqd_to_hwirq(d));
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void mediatek_irq_relres(struct irq_data *d)
> > +{
> > + struct mtk_data *gpio_data = irq_data_get_irq_chip_data(d);
> > + int bank = irqd_to_hwirq(d) / MTK_BANK_WIDTH;
> > + struct mtk_gc *rg = &gpio_data->gc_map[bank];
> > + struct gpio_chip *gc = &rg->chip;
> > +
> > + gpiochip_unlock_as_irq(gc, irqd_to_hwirq(d));
> > +}
> > +
> > static struct irq_chip mediatek_gpio_irq_chip = {
> > - .name = "GPIO",
> > - .irq_unmask = mediatek_gpio_irq_unmask,
> > - .irq_mask = mediatek_gpio_irq_mask,
> > - .irq_mask_ack = mediatek_gpio_irq_mask,
> > - .irq_set_type = mediatek_gpio_irq_type,
> > + .name = "GPIO",
> > + .irq_unmask = mediatek_gpio_irq_unmask,
> > + .irq_mask = mediatek_gpio_irq_mask,
> > + .irq_mask_ack = mediatek_gpio_irq_mask,
> > + .irq_set_type = mediatek_gpio_irq_type,
> > + .irq_request_resources = mediatek_irq_reqres,
> > + .irq_release_resources = mediatek_irq_relres,
> > };
> >
> > static int
> > @@ -282,6 +313,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
> > dev_err(&pdev->dev, "irq_domain_add_linear failed\n");
> > }
> >
> > + gpio_data->dev = &pdev->dev;
> > platform_set_drvdata(pdev, gpio_data);
> >
> > for_each_child_of_node(np, bank)
> > --
> > 2.7.4
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
