Messages in this thread | | | Date | Wed, 7 Mar 2018 15:18:02 +0000 | From | Lee Jones <> | Subject | Re: [PATCH 2/6] mfd: cros_ec: free IRQ automatically |
| |
On Tue, 20 Feb 2018, Enric Balletbo Serra wrote:
> Hi Vincent, > > 2018-02-20 9:00 GMT+01:00 Vincent Palatin <vpalatin@chromium.org>: > > On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra > > <enric.balletbo@collabora.com> wrote: > >> From: Vincent Palatin <vpalatin@chromium.org> > >> > >> Free the IRQ we might have requested when removing the cros_ec device, > >> so we can unload and reload the driver properly. > >> > >> Signed-off-by: Vincent Palatin <vpalatin@chromium.org> > >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > >> --- > >> drivers/mfd/cros_ec.c | 17 ++++++----------- > >> 1 file changed, 6 insertions(+), 11 deletions(-) > >> > >> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > >> index 74780f2964a1..d1a4fbee9380 100644 > >> --- a/drivers/mfd/cros_ec.c > >> +++ b/drivers/mfd/cros_ec.c > >> @@ -119,9 +119,9 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> } > >> > >> if (ec_dev->irq) { > >> - err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread, > >> - IRQF_TRIGGER_LOW | IRQF_ONESHOT, > >> - "chromeos-ec", ec_dev); > >> + err = devm_request_threaded_irq(dev, ec_dev->irq, NULL, > >> + ec_irq_thread, IRQF_TRIGGER_LOW | IRQF_ONESHOT, > >> + "chromeos-ec", ec_dev); > >> if (err) { > >> dev_err(dev, "Failed to request IRQ %d: %d", > >> ec_dev->irq, err); > >> @@ -135,7 +135,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> dev_err(dev, > >> "Failed to register Embedded Controller subdevice %d\n", > >> err); > >> - goto fail_mfd; > >> + return err; > >> } > >> > >> if (ec_dev->max_passthru) { > >> @@ -153,7 +153,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> dev_err(dev, > >> "Failed to register Power Delivery subdevice %d\n", > >> err); > >> - goto fail_mfd; > >> + return err; > >> } > >> } > >> > >> @@ -162,7 +162,7 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> if (err) { > >> mfd_remove_devices(dev); > >> dev_err(dev, "Failed to register sub-devices\n"); > >> - goto fail_mfd; > >> + return err; > >> } > >> } > >> > >> @@ -180,11 +180,6 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> cros_ec_acpi_install_gpe_handler(dev); > >> > >> return 0; > >> - > >> -fail_mfd: > >> - if (ec_dev->irq) > >> - free_irq(ec_dev->irq, ec_dev); > >> - return err; > >> } > >> EXPORT_SYMBOL(cros_ec_register); > > > > > > You need to remove the "free_irq(ec_dev->irq, ec_dev);" in cros_ec_remove() too. > > as done there: > > https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/866858/15/drivers/mfd/cros_ec.c > > > > it was missing originally but was added by: 'f58b14e6632a mfd: > > cros_ec: Free IRQ on exit' > > > > Many thanks to catch this. I'll wait a bit more for if I receive more > feedback on the other patches and send a second version of this > patchset.
Hmm... rather than mess around any further with this set, would you be kind enough to pull all of the patches back into a single set and send them out threaded with a cover-letter please?
I'm also going to un-apply 1/6 for completeness.
-- Lee Jones Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |