From: Vincent Palatin <[email protected]>
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 <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
---
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);
--
2.16.1
On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra
<[email protected]> wrote:
> From: Vincent Palatin <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> ---
> 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'
>
> --
> 2.16.1
>
Hi Vincent,
2018-02-20 9:00 GMT+01:00 Vincent Palatin <[email protected]>:
> On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra
> <[email protected]> wrote:
>> From: Vincent Palatin <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Enric Balletbo i Serra <[email protected]>
>> ---
>> 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.
Regards,
Enric
>
>>
>> --
>> 2.16.1
>>
On Tue, 20 Feb 2018, Enric Balletbo Serra wrote:
> Hi Vincent,
>
> 2018-02-20 9:00 GMT+01:00 Vincent Palatin <[email protected]>:
> > On Mon, Feb 19, 2018 at 11:40 PM, Enric Balletbo i Serra
> > <[email protected]> wrote:
> >> From: Vincent Palatin <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> >> ---
> >> 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