* [PATCH 2/6] mfd: cros_ec: free IRQ automatically
@ 2018-02-19 22:40 Enric Balletbo i Serra
2018-02-20 8:00 ` Vincent Palatin
0 siblings, 1 reply; 4+ messages in thread
From: Enric Balletbo i Serra @ 2018-02-19 22:40 UTC (permalink / raw)
To: Lee Jones; +Cc: groeck, gwendal, bleung, linux-kernel, kernel, Vincent Palatin
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);
--
2.16.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/6] mfd: cros_ec: free IRQ automatically
2018-02-19 22:40 [PATCH 2/6] mfd: cros_ec: free IRQ automatically Enric Balletbo i Serra
@ 2018-02-20 8:00 ` Vincent Palatin
2018-02-20 8:52 ` Enric Balletbo Serra
0 siblings, 1 reply; 4+ messages in thread
From: Vincent Palatin @ 2018-02-20 8:00 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Lee Jones, groeck, Gwendal Grignou, Benson Leung, LKML, kernel
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'
>
> --
> 2.16.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/6] mfd: cros_ec: free IRQ automatically
2018-02-20 8:00 ` Vincent Palatin
@ 2018-02-20 8:52 ` Enric Balletbo Serra
2018-03-07 15:18 ` Lee Jones
0 siblings, 1 reply; 4+ messages in thread
From: Enric Balletbo Serra @ 2018-02-20 8:52 UTC (permalink / raw)
To: Vincent Palatin
Cc: Enric Balletbo i Serra, Lee Jones, Guenter Roeck,
Gwendal Grignou, Benson Leung, LKML, kernel
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.
Regards,
Enric
>
>>
>> --
>> 2.16.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/6] mfd: cros_ec: free IRQ automatically
2018-02-20 8:52 ` Enric Balletbo Serra
@ 2018-03-07 15:18 ` Lee Jones
0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2018-03-07 15:18 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Vincent Palatin, Enric Balletbo i Serra, Guenter Roeck,
Gwendal Grignou, Benson Leung, LKML, kernel
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-03-07 15:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-19 22:40 [PATCH 2/6] mfd: cros_ec: free IRQ automatically Enric Balletbo i Serra
2018-02-20 8:00 ` Vincent Palatin
2018-02-20 8:52 ` Enric Balletbo Serra
2018-03-07 15:18 ` Lee Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.