Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1730749 > unrolled thread
| Started by | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| First post | 2017-09-12 11:10 +0200 |
| Last post | 2017-09-19 14:50 +0200 |
| Articles | 13 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH 1/2] mfd: fsl-imx25: set irq handler and data in one go Martin Kaiser <martin@kaiser.cx> - 2017-09-12 11:10 +0200
[PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal Martin Kaiser <martin@kaiser.cx> - 2017-09-12 11:10 +0200
Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal Lee Jones <lee.jones@linaro.org> - 2017-09-14 11:30 +0200
Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal Martin Kaiser <martin@kaiser.cx> - 2017-09-14 23:10 +0200
Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal Lee Jones <lee.jones@linaro.org> - 2017-09-18 11:00 +0200
Re: [PATCH 1/2] mfd: fsl-imx25: set irq handler and data in one go Lee Jones <lee.jones@linaro.org> - 2017-09-14 11:30 +0200
[PATCH v2 2/2] mfd: fsl-imx25: clean up irq settings during removal Martin Kaiser <martin@kaiser.cx> - 2017-09-19 08:30 +0200
Re: [PATCH v2 2/2] mfd: fsl-imx25: clean up irq settings during removal Lucas Stach <l.stach@pengutronix.de> - 2017-09-19 10:30 +0200
[PATCH v2 1/2] mfd: fsl-imx25: set irq handler and data in one go Martin Kaiser <martin@kaiser.cx> - 2017-09-19 08:30 +0200
Re: [PATCH v2 1/2] mfd: fsl-imx25: set irq handler and data in one go Lucas Stach <l.stach@pengutronix.de> - 2017-09-19 10:20 +0200
Re: [PATCH v3 2/2] mfd: fsl-imx25: clean up irq settings during removal Lucas Stach <l.stach@pengutronix.de> - 2017-09-19 14:50 +0200
[PATCH v3 1/2] mfd: fsl-imx25: set irq handler and data in one go Martin Kaiser <martin@kaiser.cx> - 2017-09-19 14:50 +0200
[PATCH v3 2/2] mfd: fsl-imx25: clean up irq settings during removal Martin Kaiser <martin@kaiser.cx> - 2017-09-19 14:50 +0200
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-12 11:10 +0200 |
| Subject | [PATCH 1/2] mfd: fsl-imx25: set irq handler and data in one go |
| Message-ID | <uoPmG-7Mg-31@gated-at.bofh.it> |
Replace the two separate calls for setting the irq handler and data with a single irq_set_chained_handler_and_data() call. Signed-off-by: Martin Kaiser <martin@kaiser.cx> --- drivers/mfd/fsl-imx25-tsadc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c index b3767c3..14189ef 100644 --- a/drivers/mfd/fsl-imx25-tsadc.c +++ b/drivers/mfd/fsl-imx25-tsadc.c @@ -84,8 +84,7 @@ static int mx25_tsadc_setup_irq(struct platform_device *pdev, return -ENOMEM; } - irq_set_chained_handler(irq, mx25_tsadc_irq_handler); - irq_set_handler_data(irq, tsadc); + irq_set_chained_handler_and_data(irq, mx25_tsadc_irq_handler, tsadc); return 0; } -- 2.1.4
[toc] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-12 11:10 +0200 |
| Subject | [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <uoPmH-7Mg-67@gated-at.bofh.it> |
| In reply to | #1730749 |
When fsl-imx25-tsadc is compiled as a module, unloading and reloading
the module will lead to a crash.
Add a removal function which clears the irq handler and removes the irq
domain. With this cleanup in place, it's possible to unload and reload
the module.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
index 14189ef..a6423bd 100644
--- a/drivers/mfd/fsl-imx25-tsadc.c
+++ b/drivers/mfd/fsl-imx25-tsadc.c
@@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
return devm_of_platform_populate(dev);
}
+static int __exit mx25_tsadc_remove(struct platform_device *pdev)
+{
+ struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
+ int irq = platform_get_irq(pdev, 0);
+
+ if (irq) {
+ irq_set_chained_handler_and_data(irq, NULL, NULL);
+ irq_domain_remove(tsadc->domain);
+ }
+
+ return 0;
+}
+
static const struct of_device_id mx25_tsadc_ids[] = {
{ .compatible = "fsl,imx25-tsadc" },
{ /* Sentinel */ }
@@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
.of_match_table = of_match_ptr(mx25_tsadc_ids),
},
.probe = mx25_tsadc_probe,
+ .remove = __exit_p(mx25_tsadc_remove),
};
module_platform_driver(mx25_tsadc_driver);
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-14 11:30 +0200 |
| Subject | Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <upyD7-3bN-1@gated-at.bofh.it> |
| In reply to | #1730756 |
On Tue, 12 Sep 2017, Martin Kaiser wrote:
> When fsl-imx25-tsadc is compiled as a module, unloading and reloading
> the module will lead to a crash.
>
> Add a removal function which clears the irq handler and removes the irq
> domain. With this cleanup in place, it's possible to unload and reload
> the module.
More information please.
What is it specifically that causes the crash?
Why is this code not required? What's stopping this causing a leak?
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
> index 14189ef..a6423bd 100644
> --- a/drivers/mfd/fsl-imx25-tsadc.c
> +++ b/drivers/mfd/fsl-imx25-tsadc.c
> @@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int __exit mx25_tsadc_remove(struct platform_device *pdev)
> +{
> + struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
> + int irq = platform_get_irq(pdev, 0);
> +
> + if (irq) {
> + irq_set_chained_handler_and_data(irq, NULL, NULL);
> + irq_domain_remove(tsadc->domain);
> + }
> +
> + return 0;
> +}
> +
> static const struct of_device_id mx25_tsadc_ids[] = {
> { .compatible = "fsl,imx25-tsadc" },
> { /* Sentinel */ }
> @@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
> .of_match_table = of_match_ptr(mx25_tsadc_ids),
> },
> .probe = mx25_tsadc_probe,
> + .remove = __exit_p(mx25_tsadc_remove),
> };
> module_platform_driver(mx25_tsadc_driver);
>
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-14 23:10 +0200 |
| Subject | Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <upJyx-1Kz-3@gated-at.bofh.it> |
| In reply to | #1732132 |
Thus wrote Lee Jones (lee.jones@linaro.org): > On Tue, 12 Sep 2017, Martin Kaiser wrote: > > When fsl-imx25-tsadc is compiled as a module, unloading and reloading > > the module will lead to a crash. > > Add a removal function which clears the irq handler and removes the irq > > domain. With this cleanup in place, it's possible to unload and reload > > the module. > More information please. > What is it specifically that causes the crash? I tested using a touchscreen and compiled both drivers/input/touchscreen/fsl-imx25-tcq.c and drivers/mfd/fsl-imx25-tsadc.c as modules. I got the crash after running insmod fsl-imx25-tcq.ko insmod fsl-imx25-tsadc.ko rmmod fsl-imx25-tsadc.ko insmod fsl-imx25-tsadc.ko [ 133.594246] Unable to handle kernel paging request at virtual address bf005430 [ 133.601685] pgd = d3b90000 [ 133.604435] [bf005430] *pgd=93b61811, *pte=00000000, *ppte=00000000 [ 133.610902] Internal error: Oops: 7 [#1] ARM [ 133.615208] Modules linked in: fsl_imx25_tsadc(+) fsl_imx25_tcq [last unloaded: fsl_imx25_tsadc] [ 133.624078] CPU: 0 PID: 173 Comm: insmod Not tainted 4.13.0-next-20170911+ #132 [ 133.631413] Hardware name: Freescale i.MX25 (Device Tree Support) [ 133.637537] task: d3b64000 task.stack: d3a64000 [ 133.642131] PC is at irq_find_matching_fwspec+0x40/0x108 [ 133.647484] LR is at irq_find_matching_fwspec+0x30/0x108 [ 133.652826] pc : [<c004dfac>] lr : [<c004df9c>] psr: 20000013 [ 133.659116] sp : d3a65bb0 ip : d3a65bb0 fp : d3a65bd4 [ 133.664362] r10: 00000000 r9 : c03cac4c r8 : d3a65c20 [ 133.669612] r7 : 00000000 r6 : c0ab5a58 r5 : d3d700dc r4 : d3b80000 [ 133.676167] r3 : bf00542c r2 : 40000013 r1 : d3b64000 r0 : c0ab5a4c [ 133.682721] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 133.689883] Control: 0005317f Table: 93b90000 DAC: 00000051 [ 133.695655] Process insmod (pid: 173, stack limit = 0xd3a64190) ... [ 133.993772] Backtrace: [ 133.996307] [<c004df6c>] (irq_find_matching_fwspec) from [<c028d5ec>] (of_irq_get+0x58/0x74) [ 134.004798] r9:d3d737ec r8:d3948000 r7:d3948010 r6:d3b6cb10 r5:00000000 r4:d3d700dc [ 134.012607] [<c028d594>] (of_irq_get) from [<c01ff970>] (platform_get_irq+0x48/0xc8) [ 134.020375] r4:d3948000 [ 134.022990] [<c01ff928>] (platform_get_irq) from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc]) [ 134.033019] r5:00000012 r4:00000000 [ 134.036663] [<bf00e11c>] (mx25_tsadc_probe [fsl_imx25_tsadc]) from [<c01ff658>] (platform_drv_probe+0x60/0xb0) [ 134.046711] r9:00000008 r8:00000000 r7:c0b15680 r6:bf00e7b0 r5:d3948010 r4:bf00e11c [ 134.054504] [<c01ff5f8>] (platform_drv_probe) from [<c01fd5fc>] (driver_probe_device+0x204/0x470) [ 134.063414] r6:00000000 r5:bf00e7b0 r4:d3948010 r3:c01ff5f8 [ 134.069119] [<c01fd3f8>] (driver_probe_device) from [<c01fd940>] (__driver_attach+0xd8/0x118) [ 134.077691] r9:bf00e84c r8:c0af6078 r7:c0ad8ee0 r6:bf00e7b0 r5:d3948044 r4:d3948010 [ 134.085483] [<c01fd868>] (__driver_attach) from [<c01fb734>] (bus_for_each_dev+0x7c/0xa0) irq_find_matching_fwspec() loops over all registered irq domains, I guess the crash happens inside this loop. The irq domain is still registered from last time the module was loaded but the pointer to its operations is invalid after the module was unloaded. My hope was that removing the irq domain along with module removal would fix the crash in a proper way. > Why is this code not required? What's stopping this causing a leak? Not sure I got your point here. My assumption is that (at least for platform drivers), the removal function is called only when the previous probe was successful. platform_get_irq() should then get us the same irq we got in the probe function and we can remove the handler and the irq domain. Needless to say, I'm happy to update and re-test the patch if required. Best regards, Martin
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-18 11:00 +0200 |
| Subject | Re: [PATCH 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <ur04i-3Pu-15@gated-at.bofh.it> |
| In reply to | #1732574 |
On Thu, 14 Sep 2017, Martin Kaiser wrote: > Thus wrote Lee Jones (lee.jones@linaro.org): > > > On Tue, 12 Sep 2017, Martin Kaiser wrote: > > > > When fsl-imx25-tsadc is compiled as a module, unloading and reloading > > > the module will lead to a crash. > > > > Add a removal function which clears the irq handler and removes the irq > > > domain. With this cleanup in place, it's possible to unload and reload > > > the module. > > > More information please. > > > What is it specifically that causes the crash? > > I tested using a touchscreen and compiled both > drivers/input/touchscreen/fsl-imx25-tcq.c and > drivers/mfd/fsl-imx25-tsadc.c > as modules. > > I got the crash after running > insmod fsl-imx25-tcq.ko > insmod fsl-imx25-tsadc.ko > rmmod fsl-imx25-tsadc.ko > insmod fsl-imx25-tsadc.ko > > [ 133.594246] Unable to handle kernel paging request at virtual address bf005430 > [ 133.601685] pgd = d3b90000 > [ 133.604435] [bf005430] *pgd=93b61811, *pte=00000000, *ppte=00000000 > [ 133.610902] Internal error: Oops: 7 [#1] ARM > [ 133.615208] Modules linked in: fsl_imx25_tsadc(+) fsl_imx25_tcq [last unloaded: fsl_imx25_tsadc] > [ 133.624078] CPU: 0 PID: 173 Comm: insmod Not tainted 4.13.0-next-20170911+ #132 > [ 133.631413] Hardware name: Freescale i.MX25 (Device Tree Support) > [ 133.637537] task: d3b64000 task.stack: d3a64000 > [ 133.642131] PC is at irq_find_matching_fwspec+0x40/0x108 > [ 133.647484] LR is at irq_find_matching_fwspec+0x30/0x108 > [ 133.652826] pc : [<c004dfac>] lr : [<c004df9c>] psr: 20000013 > [ 133.659116] sp : d3a65bb0 ip : d3a65bb0 fp : d3a65bd4 > [ 133.664362] r10: 00000000 r9 : c03cac4c r8 : d3a65c20 > [ 133.669612] r7 : 00000000 r6 : c0ab5a58 r5 : d3d700dc r4 : d3b80000 > [ 133.676167] r3 : bf00542c r2 : 40000013 r1 : d3b64000 r0 : c0ab5a4c > [ 133.682721] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none > [ 133.689883] Control: 0005317f Table: 93b90000 DAC: 00000051 > [ 133.695655] Process insmod (pid: 173, stack limit = 0xd3a64190) > ... > [ 133.993772] Backtrace: > [ 133.996307] [<c004df6c>] (irq_find_matching_fwspec) from [<c028d5ec>] (of_irq_get+0x58/0x74) > [ 134.004798] r9:d3d737ec r8:d3948000 r7:d3948010 r6:d3b6cb10 r5:00000000 r4:d3d700dc > [ 134.012607] [<c028d594>] (of_irq_get) from [<c01ff970>] (platform_get_irq+0x48/0xc8) > [ 134.020375] r4:d3948000 > [ 134.022990] [<c01ff928>] (platform_get_irq) from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc]) > [ 134.033019] r5:00000012 r4:00000000 > [ 134.036663] [<bf00e11c>] (mx25_tsadc_probe [fsl_imx25_tsadc]) from [<c01ff658>] (platform_drv_probe+0x60/0xb0) > [ 134.046711] r9:00000008 r8:00000000 r7:c0b15680 r6:bf00e7b0 r5:d3948010 r4:bf00e11c > [ 134.054504] [<c01ff5f8>] (platform_drv_probe) from [<c01fd5fc>] (driver_probe_device+0x204/0x470) > [ 134.063414] r6:00000000 r5:bf00e7b0 r4:d3948010 r3:c01ff5f8 > [ 134.069119] [<c01fd3f8>] (driver_probe_device) from [<c01fd940>] (__driver_attach+0xd8/0x118) > [ 134.077691] r9:bf00e84c r8:c0af6078 r7:c0ad8ee0 r6:bf00e7b0 r5:d3948044 r4:d3948010 > [ 134.085483] [<c01fd868>] (__driver_attach) from [<c01fb734>] (bus_for_each_dev+0x7c/0xa0) > > > irq_find_matching_fwspec() loops over all registered irq domains, I guess the > crash happens inside this loop. The irq domain is still registered from last > time the module was loaded but the pointer to its operations is invalid after > the module was unloaded. Great. Please condense this information as much as possible and place it in the commit log. > My hope was that removing the irq domain along with module removal would fix > the crash in a proper way. > > > Why is this code not required? What's stopping this causing a leak? > > Not sure I got your point here. Never mind. This was due to an incorrect assumption I made. You can ignore this. > My assumption is that (at least for platform drivers), the removal function is > called only when the previous probe was successful. platform_get_irq() should > then get us the same irq we got in the probe function and we can remove the > handler and the irq domain. > > Needless to say, I'm happy to update and re-test the patch if required. > > Best regards, > > Martin -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Lee Jones <lee.jones@linaro.org> |
|---|---|
| Date | 2017-09-14 11:30 +0200 |
| Message-ID | <upyD7-3bN-11@gated-at.bofh.it> |
| In reply to | #1730749 |
On Tue, 12 Sep 2017, Martin Kaiser wrote: > Replace the two separate calls for setting the irq handler and data with > a single irq_set_chained_handler_and_data() call. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> > --- > drivers/mfd/fsl-imx25-tsadc.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) Applied, thanks. -- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
[toc] | [prev] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-19 08:30 +0200 |
| Subject | [PATCH v2 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <urkcF-xj-1@gated-at.bofh.it> |
| In reply to | #1730749 |
When fsl-imx25-tsadc is compiled as a module, loading, unloading and
reloading the module will lead to a crash.
Unable to handle kernel paging request at virtual address bf005430
[<c004df6c>] (irq_find_matching_fwspec)
from [<c028d5ec>] (of_irq_get+0x58/0x74)
[<c028d594>] (of_irq_get)
from [<c01ff970>] (platform_get_irq+0x48/0xc8)
[<c01ff928>] (platform_get_irq)
from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc])
irq_find_matching_fwspec() loops over all registered irq domains. The
irq domain is still registered from last time the module was loaded but
the pointer to its operations is invalid after the module was unloaded.
Add a removal function which clears the irq handler and removes the irq
domain. With this cleanup in place, it's possible to unload and reload
the module.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
changes in v2:
add more details about the crash to the commit message
drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
index 14189ef..a6423bd 100644
--- a/drivers/mfd/fsl-imx25-tsadc.c
+++ b/drivers/mfd/fsl-imx25-tsadc.c
@@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
return devm_of_platform_populate(dev);
}
+static int __exit mx25_tsadc_remove(struct platform_device *pdev)
+{
+ struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
+ int irq = platform_get_irq(pdev, 0);
+
+ if (irq) {
+ irq_set_chained_handler_and_data(irq, NULL, NULL);
+ irq_domain_remove(tsadc->domain);
+ }
+
+ return 0;
+}
+
static const struct of_device_id mx25_tsadc_ids[] = {
{ .compatible = "fsl,imx25-tsadc" },
{ /* Sentinel */ }
@@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
.of_match_table = of_match_ptr(mx25_tsadc_ids),
},
.probe = mx25_tsadc_probe,
+ .remove = __exit_p(mx25_tsadc_remove),
};
module_platform_driver(mx25_tsadc_driver);
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Lucas Stach <l.stach@pengutronix.de> |
|---|---|
| Date | 2017-09-19 10:30 +0200 |
| Subject | Re: [PATCH v2 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <urm4N-1KA-9@gated-at.bofh.it> |
| In reply to | #1734678 |
Am Dienstag, den 19.09.2017, 08:23 +0200 schrieb Martin Kaiser:
> When fsl-imx25-tsadc is compiled as a module, loading, unloading and
> reloading the module will lead to a crash.
>
> Unable to handle kernel paging request at virtual address bf005430
> [<c004df6c>] (irq_find_matching_fwspec)
> from [<c028d5ec>] (of_irq_get+0x58/0x74)
> [<c028d594>] (of_irq_get)
> from [<c01ff970>] (platform_get_irq+0x48/0xc8)
> [<c01ff928>] (platform_get_irq)
> from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc])
>
> irq_find_matching_fwspec() loops over all registered irq domains. The
> irq domain is still registered from last time the module was loaded but
> the pointer to its operations is invalid after the module was unloaded.
>
> Add a removal function which clears the irq handler and removes the irq
> domain. With this cleanup in place, it's possible to unload and reload
> the module.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> changes in v2:
> add more details about the crash to the commit message
>
> drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
> index 14189ef..a6423bd 100644
> --- a/drivers/mfd/fsl-imx25-tsadc.c
> +++ b/drivers/mfd/fsl-imx25-tsadc.c
> @@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int __exit mx25_tsadc_remove(struct platform_device *pdev)
I don't think the __exit annotation makes sense. You can also unbind the
device by using sysfs, in which case you want to do the same IRQ
cleanup.
> +{
> + struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
> + int irq = platform_get_irq(pdev, 0);
> +
> + if (irq) {
> + irq_set_chained_handler_and_data(irq, NULL, NULL);
> + irq_domain_remove(tsadc->domain);
> + }
> +
> + return 0;
> +}
> +
> static const struct of_device_id mx25_tsadc_ids[] = {
> { .compatible = "fsl,imx25-tsadc" },
> { /* Sentinel */ }
> @@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
> .of_match_table = of_match_ptr(mx25_tsadc_ids),
> },
> .probe = mx25_tsadc_probe,
> + .remove = __exit_p(mx25_tsadc_remove),
> };
> module_platform_driver(mx25_tsadc_driver);
>
[toc] | [prev] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-19 08:30 +0200 |
| Subject | [PATCH v2 1/2] mfd: fsl-imx25: set irq handler and data in one go |
| Message-ID | <urkcF-xj-3@gated-at.bofh.it> |
| In reply to | #1730749 |
Replace the two separate calls for setting the irq handler and data with a single irq_set_chained_handler_and_data() call. Signed-off-by: Martin Kaiser <martin@kaiser.cx> --- changes in v2: none drivers/mfd/fsl-imx25-tsadc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c index b3767c3..14189ef 100644 --- a/drivers/mfd/fsl-imx25-tsadc.c +++ b/drivers/mfd/fsl-imx25-tsadc.c @@ -84,8 +84,7 @@ static int mx25_tsadc_setup_irq(struct platform_device *pdev, return -ENOMEM; } - irq_set_chained_handler(irq, mx25_tsadc_irq_handler); - irq_set_handler_data(irq, tsadc); + irq_set_chained_handler_and_data(irq, mx25_tsadc_irq_handler, tsadc); return 0; } -- 2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Lucas Stach <l.stach@pengutronix.de> |
|---|---|
| Date | 2017-09-19 10:20 +0200 |
| Subject | Re: [PATCH v2 1/2] mfd: fsl-imx25: set irq handler and data in one go |
| Message-ID | <urlV7-1Hq-5@gated-at.bofh.it> |
| In reply to | #1734686 |
Am Dienstag, den 19.09.2017, 08:23 +0200 schrieb Martin Kaiser: > Replace the two separate calls for setting the irq handler and data with > a single irq_set_chained_handler_and_data() call. > > Signed-off-by: Martin Kaiser <martin@kaiser.cx> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > --- > changes in v2: none > > drivers/mfd/fsl-imx25-tsadc.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c > index b3767c3..14189ef 100644 > --- a/drivers/mfd/fsl-imx25-tsadc.c > +++ b/drivers/mfd/fsl-imx25-tsadc.c > @@ -84,8 +84,7 @@ static int mx25_tsadc_setup_irq(struct platform_device *pdev, > return -ENOMEM; > } > > - irq_set_chained_handler(irq, mx25_tsadc_irq_handler); > - irq_set_handler_data(irq, tsadc); > + irq_set_chained_handler_and_data(irq, mx25_tsadc_irq_handler, tsadc); > > return 0; > }
[toc] | [prev] | [next] | [standalone]
| From | Lucas Stach <l.stach@pengutronix.de> |
|---|---|
| Date | 2017-09-19 14:50 +0200 |
| Subject | Re: [PATCH v3 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <urq8p-5xV-13@gated-at.bofh.it> |
| In reply to | #1730749 |
Am Dienstag, den 19.09.2017, 14:38 +0200 schrieb Martin Kaiser:
> When fsl-imx25-tsadc is compiled as a module, loading, unloading and
> reloading the module will lead to a crash.
>
> Unable to handle kernel paging request at virtual address bf005430
> [<c004df6c>] (irq_find_matching_fwspec)
> from [<c028d5ec>] (of_irq_get+0x58/0x74)
> [<c028d594>] (of_irq_get)
> from [<c01ff970>] (platform_get_irq+0x48/0xc8)
> [<c01ff928>] (platform_get_irq)
> from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc])
>
> irq_find_matching_fwspec() loops over all registered irq domains. The
> irq domain is still registered from last time the module was loaded but
> the pointer to its operations is invalid after the module was unloaded.
>
> Add a removal function which clears the irq handler and removes the irq
> domain. With this cleanup in place, it's possible to unload and reload
> the module.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> ---
> changes in v3:
> delete__exit qualifier from the remove routine
>
> changes in v2:
> add more details about the crash to the commit message
>
> drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
> index 14189ef..dbb85ca 100644
> --- a/drivers/mfd/fsl-imx25-tsadc.c
> +++ b/drivers/mfd/fsl-imx25-tsadc.c
> @@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
> return devm_of_platform_populate(dev);
> }
>
> +static int mx25_tsadc_remove(struct platform_device *pdev)
> +{
> + struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
> + int irq = platform_get_irq(pdev, 0);
> +
> + if (irq) {
> + irq_set_chained_handler_and_data(irq, NULL, NULL);
> + irq_domain_remove(tsadc->domain);
> + }
> +
> + return 0;
> +}
> +
> static const struct of_device_id mx25_tsadc_ids[] = {
> { .compatible = "fsl,imx25-tsadc" },
> { /* Sentinel */ }
> @@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
> .of_match_table = of_match_ptr(mx25_tsadc_ids),
> },
> .probe = mx25_tsadc_probe,
> + .remove = mx25_tsadc_remove,
> };
> module_platform_driver(mx25_tsadc_driver);
>
[toc] | [prev] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-19 14:50 +0200 |
| Subject | [PATCH v3 1/2] mfd: fsl-imx25: set irq handler and data in one go |
| Message-ID | <urq8q-5xV-15@gated-at.bofh.it> |
| In reply to | #1730749 |
Replace the two separate calls for setting the irq handler and data with a single irq_set_chained_handler_and_data() call. Signed-off-by: Martin Kaiser <martin@kaiser.cx> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> --- changes in v3: none changes in v2: none drivers/mfd/fsl-imx25-tsadc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c index b3767c3..14189ef 100644 --- a/drivers/mfd/fsl-imx25-tsadc.c +++ b/drivers/mfd/fsl-imx25-tsadc.c @@ -84,8 +84,7 @@ static int mx25_tsadc_setup_irq(struct platform_device *pdev, return -ENOMEM; } - irq_set_chained_handler(irq, mx25_tsadc_irq_handler); - irq_set_handler_data(irq, tsadc); + irq_set_chained_handler_and_data(irq, mx25_tsadc_irq_handler, tsadc); return 0; } -- 2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Martin Kaiser <martin@kaiser.cx> |
|---|---|
| Date | 2017-09-19 14:50 +0200 |
| Subject | [PATCH v3 2/2] mfd: fsl-imx25: clean up irq settings during removal |
| Message-ID | <urq8q-5xV-17@gated-at.bofh.it> |
| In reply to | #1734884 |
When fsl-imx25-tsadc is compiled as a module, loading, unloading and
reloading the module will lead to a crash.
Unable to handle kernel paging request at virtual address bf005430
[<c004df6c>] (irq_find_matching_fwspec)
from [<c028d5ec>] (of_irq_get+0x58/0x74)
[<c028d594>] (of_irq_get)
from [<c01ff970>] (platform_get_irq+0x48/0xc8)
[<c01ff928>] (platform_get_irq)
from [<bf00e33c>] (mx25_tsadc_probe+0x220/0x2f4 [fsl_imx25_tsadc])
irq_find_matching_fwspec() loops over all registered irq domains. The
irq domain is still registered from last time the module was loaded but
the pointer to its operations is invalid after the module was unloaded.
Add a removal function which clears the irq handler and removes the irq
domain. With this cleanup in place, it's possible to unload and reload
the module.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
changes in v3:
delete__exit qualifier from the remove routine
changes in v2:
add more details about the crash to the commit message
drivers/mfd/fsl-imx25-tsadc.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/mfd/fsl-imx25-tsadc.c b/drivers/mfd/fsl-imx25-tsadc.c
index 14189ef..dbb85ca 100644
--- a/drivers/mfd/fsl-imx25-tsadc.c
+++ b/drivers/mfd/fsl-imx25-tsadc.c
@@ -179,6 +179,19 @@ static int mx25_tsadc_probe(struct platform_device *pdev)
return devm_of_platform_populate(dev);
}
+static int mx25_tsadc_remove(struct platform_device *pdev)
+{
+ struct mx25_tsadc *tsadc = platform_get_drvdata(pdev);
+ int irq = platform_get_irq(pdev, 0);
+
+ if (irq) {
+ irq_set_chained_handler_and_data(irq, NULL, NULL);
+ irq_domain_remove(tsadc->domain);
+ }
+
+ return 0;
+}
+
static const struct of_device_id mx25_tsadc_ids[] = {
{ .compatible = "fsl,imx25-tsadc" },
{ /* Sentinel */ }
@@ -191,6 +204,7 @@ static struct platform_driver mx25_tsadc_driver = {
.of_match_table = of_match_ptr(mx25_tsadc_ids),
},
.probe = mx25_tsadc_probe,
+ .remove = mx25_tsadc_remove,
};
module_platform_driver(mx25_tsadc_driver);
--
2.1.4
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web