| Message ID | 1421581173-28416-9-git-send-email-ricardo.ribalda@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes: > Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a > counterpart for of_mm_gpiochip_add. > This patch implements the remove function of the driver making use of > it. > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Peter Korsgaard <jacmet@sunsite.dk> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > drivers/gpio/gpio-mpc8xxx.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c > index 57eb794..a6952ba3 100644 > --- a/drivers/gpio/gpio-mpc8xxx.c > +++ b/drivers/gpio/gpio-mpc8xxx.c > @@ -40,6 +40,7 @@ struct mpc8xxx_gpio_chip { > */ > u32 data; > struct irq_domain *irq; > + unsigned int irqn; > const void *of_dev_id_data; > }; > @@ -350,13 +351,14 @@ static int mpc8xxx_probe(struct platform_device *pdev) > struct of_mm_gpio_chip *mm_gc; > struct gpio_chip *gc; > const struct of_device_id *id; > - unsigned hwirq; > int ret; > mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL); > if (!mpc8xxx_gc) > return -ENOMEM; > + platform_set_drvdata(pdev, mpc8xxx_gc); > + > spin_lock_init(&mpc8xxx_gc->lock); > mm_gc = &mpc8xxx_gc->mm_gc; > @@ -377,8 +379,8 @@ static int mpc8xxx_probe(struct platform_device *pdev) > if (ret) > return ret; > - hwirq = irq_of_parse_and_map(np, 0); > - if (hwirq == NO_IRQ) > + mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0); > + if (mpc8xxx_gc->irqn == NO_IRQ) > return 0; With this return 0 converted to do of_mm_gpiochip_remove(): Acked-by: Peter Korsgaard <peter@korsgaard.com>
Hello Peter > > > - hwirq = irq_of_parse_and_map(np, 0); > > - if (hwirq == NO_IRQ) > > + mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0); > > + if (mpc8xxx_gc->irqn == NO_IRQ) > > return 0; > > > With this return 0 converted to do of_mm_gpiochip_remove(): Are you sure? The driver can still work as a normal gpio without the irq domain part and the remove function consider this option. The original code did also continue.... If you still want to abort if no irq I can of course make the change. Regards! > > Acked-by: Peter Korsgaard <peter@korsgaard.com> > > -- > Bye, Peter Korsgaard Regards
On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a > counterpart for of_mm_gpiochip_add. > > This patch implements the remove function of the driver making use of > it. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Peter Korsgaard <jacmet@sunsite.dk> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> Waiting for a v2 addressing the issues pointed out by Peter on this patch. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Linus I am waiting from the confirmation of Peter if that is what he really needs. It seems to me that the code should continue even if there is no irqdomain created. If we need to abort if there is no irqdomain I would rather create a new patch with this change, since it it is a change of the current behaviour, so if something explodes the bisect will be easier. I will attach to this mail a patch. Regards On Tue, Jan 20, 2015 at 11:17 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado > <ricardo.ribalda@gmail.com> wrote: > >> Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a >> counterpart for of_mm_gpiochip_add. >> >> This patch implements the remove function of the driver making use of >> it. >> >> Cc: Linus Walleij <linus.walleij@linaro.org> >> Cc: Alexandre Courbot <gnurou@gmail.com> >> Cc: Peter Korsgaard <jacmet@sunsite.dk> >> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > > Waiting for a v2 addressing the issues pointed out by Peter on this > patch. > > Yours, > Linus Walleij
>>>>> "Ricardo" == Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> writes: > Hello Peter >> >> > - hwirq = irq_of_parse_and_map(np, 0); >> > - if (hwirq == NO_IRQ) >> > + mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0); >> > + if (mpc8xxx_gc->irqn == NO_IRQ) >> > return 0; >> >> >> With this return 0 converted to do of_mm_gpiochip_remove(): > Are you sure? The driver can still work as a normal gpio without the > irq domain part and the remove function consider this option. The > original code did also continue.... Ahh yes, you are right. Sorry, it's been a while since I wrote that code (2008). > If you still want to abort if no irq I can of course make the change. No, it is fine.
On Sun, Jan 18, 2015 at 12:39 PM, Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> wrote: > Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a > counterpart for of_mm_gpiochip_add. > > This patch implements the remove function of the driver making use of > it. > > Cc: Linus Walleij <linus.walleij@linaro.org> > Cc: Alexandre Courbot <gnurou@gmail.com> > Cc: Peter Korsgaard <jacmet@sunsite.dk> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> Patch applied. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c index 57eb794..a6952ba3 100644 --- a/drivers/gpio/gpio-mpc8xxx.c +++ b/drivers/gpio/gpio-mpc8xxx.c @@ -40,6 +40,7 @@ struct mpc8xxx_gpio_chip { */ u32 data; struct irq_domain *irq; + unsigned int irqn; const void *of_dev_id_data; }; @@ -350,13 +351,14 @@ static int mpc8xxx_probe(struct platform_device *pdev) struct of_mm_gpio_chip *mm_gc; struct gpio_chip *gc; const struct of_device_id *id; - unsigned hwirq; int ret; mpc8xxx_gc = devm_kzalloc(&pdev->dev, sizeof(*mpc8xxx_gc), GFP_KERNEL); if (!mpc8xxx_gc) return -ENOMEM; + platform_set_drvdata(pdev, mpc8xxx_gc); + spin_lock_init(&mpc8xxx_gc->lock); mm_gc = &mpc8xxx_gc->mm_gc; @@ -377,8 +379,8 @@ static int mpc8xxx_probe(struct platform_device *pdev) if (ret) return ret; - hwirq = irq_of_parse_and_map(np, 0); - if (hwirq == NO_IRQ) + mpc8xxx_gc->irqn = irq_of_parse_and_map(np, 0); + if (mpc8xxx_gc->irqn == NO_IRQ) return 0; mpc8xxx_gc->irq = irq_domain_add_linear(np, MPC8XXX_GPIO_PINS, @@ -394,14 +396,30 @@ static int mpc8xxx_probe(struct platform_device *pdev) out_be32(mm_gc->regs + GPIO_IER, 0xffffffff); out_be32(mm_gc->regs + GPIO_IMR, 0); - irq_set_handler_data(hwirq, mpc8xxx_gc); - irq_set_chained_handler(hwirq, mpc8xxx_gpio_irq_cascade); + irq_set_handler_data(mpc8xxx_gc->irqn, mpc8xxx_gc); + irq_set_chained_handler(mpc8xxx_gc->irqn, mpc8xxx_gpio_irq_cascade); + + return 0; +} + +static int mpc8xxx_remove(struct platform_device *pdev) +{ + struct mpc8xxx_gpio_chip *mpc8xxx_gc = platform_get_drvdata(pdev); + + if (mpc8xxx_gc->irq) { + irq_set_handler_data(mpc8xxx_gc->irqn, NULL); + irq_set_chained_handler(mpc8xxx_gc->irqn, NULL); + irq_domain_remove(mpc8xxx_gc->irq); + } + + of_mm_gpiochip_remove(&mpc8xxx_gc->mm_gc); return 0; } static struct platform_driver mpc8xxx_plat_driver = { .probe = mpc8xxx_probe, + .remove = mpc8xxx_remove, .driver = { .name = "gpio-mpc8xxx", .of_match_table = mpc8xxx_gpio_ids,
Since d621e8bae5ac9c67 (Create of_mm_gpiochip_remove), there is a counterpart for of_mm_gpiochip_add. This patch implements the remove function of the driver making use of it. Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Alexandre Courbot <gnurou@gmail.com> Cc: Peter Korsgaard <jacmet@sunsite.dk> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- drivers/gpio/gpio-mpc8xxx.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-)