* Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
@ 2020-04-08 16:52 Markus Elfring
2020-04-09 20:48 ` Dmitry Torokhov
0 siblings, 1 reply; 5+ messages in thread
From: Markus Elfring @ 2020-04-08 16:52 UTC (permalink / raw)
To: linux-input, Allison Randal, Arnd Bergmann, Dmitry Torokhov,
H Hartley Sweeten, Olof Johansson, Thomas Gleixner
Cc: LKML, kernel-janitors, Tang Bin
Hello,
I have taken another look at the implementation of the function “ep93xx_keypad_probe”.
A software analysis approach points the following source code out for
further development considerations.
https://elixir.bootlin.com/linux/v5.6.3/source/drivers/input/keyboard/ep93xx_keypad.c#L252
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/keyboard/ep93xx_keypad.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n252
keypad->irq = platform_get_irq(pdev, 0);
if (!keypad->irq) {
err = -ENXIO;
goto failed_free;
}
The software documentation is providing the following information
for the used programming interface.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n221
https://elixir.bootlin.com/linux/v5.6.3/source/drivers/base/platform.c#L202
“…
* Return: IRQ number on success, negative error number on failure.
…”
Would you like to reconsider the shown condition check?
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
2020-04-08 16:52 Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe() Markus Elfring
@ 2020-04-09 20:48 ` Dmitry Torokhov
2020-04-10 2:52 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq()call " Tang Bin
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Dmitry Torokhov @ 2020-04-09 20:48 UTC (permalink / raw)
To: Markus Elfring
Cc: linux-input, Allison Randal, Arnd Bergmann, H Hartley Sweeten,
Olof Johansson, Thomas Gleixner, LKML, kernel-janitors, Tang Bin
Hi Markus,
On Wed, Apr 08, 2020 at 06:52:31PM +0200, Markus Elfring wrote:
> Hello,
>
> I have taken another look at the implementation of the function “ep93xx_keypad_probe”.
> A software analysis approach points the following source code out for
> further development considerations.
> https://elixir.bootlin.com/linux/v5.6.3/source/drivers/input/keyboard/ep93xx_keypad.c#L252
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/input/keyboard/ep93xx_keypad.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n252
>
> keypad->irq = platform_get_irq(pdev, 0);
> if (!keypad->irq) {
> err = -ENXIO;
> goto failed_free;
> }
>
>
> The software documentation is providing the following information
> for the used programming interface.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/base/platform.c?id=f5e94d10e4c468357019e5c28d48499f677b284f#n221
> https://elixir.bootlin.com/linux/v5.6.3/source/drivers/base/platform.c#L202
>
> “…
> * Return: IRQ number on success, negative error number on failure.
> …”
>
> Would you like to reconsider the shown condition check?
Platform code historically allowed creating IRQ resources with IRQ
number 0 to indicate "no interrupt assigned", so this driver tries to
filter out such conditions. The negative IRQs (errors) will be rejected
by request_irq() but I guess we can lose -EPROBE_DEFER. We could do
if (keypad->irq <= 0) {
err = keypad->irq ?: -ENXIO : keypad->irq;
goto failed_free;
}
or, maybe we should take a look at platform_get_irq() and see if we can
stop it returning 0 interrupt numbers and clean up the drivers.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq()call in ep93xx_keypad_probe()
2020-04-09 20:48 ` Dmitry Torokhov
@ 2020-04-10 2:52 ` Tang Bin
2020-04-10 2:56 ` Tang Bin
2020-04-10 5:45 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq() call " Markus Elfring
2 siblings, 0 replies; 5+ messages in thread
From: Tang Bin @ 2020-04-10 2:52 UTC (permalink / raw)
To: Dmitry Torokhov, Markus Elfring
Cc: linux-input, Allison Randal, Arnd Bergmann, H Hartley Sweeten,
Olof Johansson, Thomas Gleixner, LKML, kernel-janitors
Hi Dmitry:
On 2020/4/10 4:48, Dmitry Torokhov wrote:
> Platform code historically allowed creating IRQ resources with IRQ
> number 0 to indicate "no interrupt assigned", so this driver tries to
> filter out such conditions. The negative IRQs (errors) will be rejected
> by request_irq() but I guess we can lose -EPROBE_DEFER. We could do
>
> if (keypad->irq <= 0) {
> err = keypad->irq ?: -ENXIO : keypad->irq;
> goto failed_free;
> }
>
>
I have been aware of this problem for a few days, and by doing
experiments on the hardware, I have found the following ways that maybe
suitable:
if (keypad->irq <= 0) {
err = keypad->irq ? : -ENXIO;
goto failed_free;
}
or
if (keypad->irq <= 0) {
err = keypad->irq < 0 ? keypad->irq : -ENXIO;
goto failed_free;
}
If you think it's usefull, I will send this patch to fix it.
Thanks
Tang Bin
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq()call in ep93xx_keypad_probe()
2020-04-09 20:48 ` Dmitry Torokhov
2020-04-10 2:52 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq()call " Tang Bin
@ 2020-04-10 2:56 ` Tang Bin
2020-04-10 5:45 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq() call " Markus Elfring
2 siblings, 0 replies; 5+ messages in thread
From: Tang Bin @ 2020-04-10 2:56 UTC (permalink / raw)
To: Dmitry Torokhov, Markus Elfring
Cc: linux-input, Allison Randal, Arnd Bergmann, H Hartley Sweeten,
Olof Johansson, Thomas Gleixner, LKML, kernel-janitors
Hi Dmitry:
On 2020/4/10 4:48, Dmitry Torokhov wrote:
> Platform code historically allowed creating IRQ resources with IRQ
> number 0 to indicate "no interrupt assigned", so this driver tries to
> filter out such conditions. The negative IRQs (errors) will be rejected
> by request_irq() but I guess we can lose -EPROBE_DEFER. We could do
>
> if (keypad->irq <= 0) {
> err = keypad->irq ?: -ENXIO : keypad->irq;
> goto failed_free;
> }
I have been aware of this problem for several days, and by doing
experiments on the hardware, I have found the following ways that maybe
suitable:
if (keypad->irq <= 0) {
err = keypad->irq ? : -ENXIO;
goto failed_free;
}
or
if (keypad->irq <= 0) {
err = keypad->irq < 0 ? keypad->irq : -ENXIO;
goto failed_free;
}
If you think it's usefull, I will send this patch to fix this problem.
Thanks
Tang Bin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
2020-04-09 20:48 ` Dmitry Torokhov
2020-04-10 2:52 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq()call " Tang Bin
2020-04-10 2:56 ` Tang Bin
@ 2020-04-10 5:45 ` Markus Elfring
2 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2020-04-10 5:45 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Allison Randal, Arnd Bergmann, H Hartley Sweeten, Olof Johansson,
Thomas Gleixner, Tang Bin, LKML, kernel-janitors, linux-doc
> Platform code historically allowed creating IRQ resources with IRQ
> number 0 to indicate "no interrupt assigned", so this driver tries to
> filter out such conditions. The negative IRQs (errors) will be rejected
> by request_irq() but I guess we can lose -EPROBE_DEFER.
Thanks for this explanation.
* Should such information be better represented in the description for
these programming interfaces?
* Can the software documentation become clearer anyhow?
> or, maybe we should take a look at platform_get_irq() and see if we can
> stop it returning 0 interrupt numbers and clean up the drivers.
Will further collateral evolution become interesting?
Regards,
Markus
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-04-10 5:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08 16:52 Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe() Markus Elfring
2020-04-09 20:48 ` Dmitry Torokhov
2020-04-10 2:52 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq()call " Tang Bin
2020-04-10 2:56 ` Tang Bin
2020-04-10 5:45 ` Input: ep93xx_keypad: Checking for a failed platform_get_irq() call " Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).