On Tue, 20 Feb 2018, Enric Balletbo Serra wrote:
> Hi Vincent,
>
> 2018-02-20 9:00 GMT+01:00 Vincent Palatin <vpala...@chromium.org>:
> > On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra
> > <enric.balle...@collabora.com> wrote:
> >> From: Vincent Palatin <vpala...@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 <vpala...@chromium.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balle...@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
