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
