* Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
@ 2020-04-08 16:52 ` Markus Elfring
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
@ 2020-04-08 16:52 ` Markus Elfring
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: 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
-1 siblings, 0 replies; 10+ 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] 10+ 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
0 siblings, 0 replies; 10+ 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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 1681 bytes --]
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õe94d10e4c468357019e5c28d48499f677b284f#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õe94d10e4c468357019e5c28d48499f677b284f#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] 10+ 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
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq()call in ep93xx_keypad_probe()
@ 2020-04-10 2:52 ` Tang Bin
0 siblings, 0 replies; 10+ 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] 10+ 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:56 ` Tang Bin
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq()call in ep93xx_keypad_probe()
@ 2020-04-10 2:56 ` Tang Bin
0 siblings, 0 replies; 10+ 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] 10+ 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 5:45 ` Markus Elfring
-1 siblings, 0 replies; 10+ 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] 10+ messages in thread
* Re: Input: ep93xx_keypad: Checking for a failed platform_get_irq() call in ep93xx_keypad_probe()
@ 2020-04-10 5:45 ` Markus Elfring
0 siblings, 0 replies; 10+ 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] 10+ messages in thread
end of thread, other threads:[~2020-04-10 5:46 UTC | newest]
Thread overview: 10+ 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-08 16:52 ` Markus Elfring
2020-04-09 20:48 ` Dmitry Torokhov
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:52 ` Tang Bin
2020-04-10 2:56 ` 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
2020-04-10 5:45 ` Markus Elfring
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.