staging: nvec: kbd: use -EINVAL instead of -1

[PATCH] staging: nvec: kbd: use -EINVAL instead of -1

Posted by Soham Kute 1 month, 2 weeks ago

Return proper error code -EINVAL instead of -1 when
the event type or code is not supported.

Signed-off-by: Soham Kute <officialsohamkute@gmail.com>
---
 drivers/staging/nvec/nvec_kbd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/nvec/nvec_kbd.c b/drivers/staging/nvec/nvec_kbd.c
index d2b91318f151..5f742c8b32cc 100644
--- a/drivers/staging/nvec/nvec_kbd.c
+++ b/drivers/staging/nvec/nvec_kbd.c
@@ -92,10 +92,10 @@ static int nvec_kbd_event(struct input_dev *dev, unsigned int type,
 		return 0;
 
 	if (type != EV_LED)
-		return -1;
+		return -EINVAL;
 
 	if (code != LED_CAPSL)
-		return -1;
+		return -EINVAL;
 
 	buf[2] = !!value;
 	nvec_write_async(nvec, buf, sizeof(buf));
-- 
2.34.1

Re: [PATCH] staging: nvec: kbd: use -EINVAL instead of -1

Posted by Dan Carpenter 1 month, 2 weeks ago

On Sun, Mar 01, 2026 at 01:50:08AM +0530, Soham Kute wrote:
> Return proper error code -EINVAL instead of -1 when
> the event type or code is not supported.
> 
> Signed-off-by: Soham Kute <officialsohamkute@gmail.com>
> ---

Could you do some analysis to see if this affects runtime?  You'll need
to review the callers to see what they do with error codes.  In this
case they ignore them.

One thing which could help would be to build the Smatch cross function
DB.  But it takes a long time so do it overnight.
https://github.com/error27/smatch
https://github.com/error27/smatch/blob/master/Documentation/smatch.rst
cd ~/path/to/kernel_dir ~/path/to/smatch_dir/smatch_scripts/build_kernel_data.sh

$ ~/progs/smatch/devel/smatch_data/db/smdb.py nvec_kbd_event | grep INTER
drivers/input/input.c |  input_event_dispose | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
drivers/input/input.c |     input_dev_toggle | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
drivers/input/input.c |     input_dev_toggle | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
drivers/input/input.c |     input_dev_toggle | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
drivers/input/input.c |     input_dev_toggle | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
drivers/hid/hid-holtek-kbd.c | holtek_kbd_input_event | (struct input_dev)->event |           INTERNAL | -1 |                 | int(*)(struct input_dev*, uint, uint, int)
$

regards,
dan carpenter

Re: [PATCH] staging: nvec: kbd: use -EINVAL instead of -1

Posted by Soham Kute 1 month, 1 week ago

Hi Dan,

Thanks for pointing me to smatch. I built the DB and ran the queries.

From what I can see, dev->event() is called in input_handle_event()
but the return value is not propagated back up. The smatch caller_info
table shows type INTERNAL for that call which I understand means the
return value stays internal and never reaches the caller of
input_event().

So yes, this patch has no runtime effect. The reason I sent it was
that pcspkr.c uses -EINVAL in the same type of event callback, so I
thought nvec_kbd should be consistent with that.

Also noticed atkbd.c has the same -1 in its event callback. Should I
send a fix for that too?

Best regards,
Soham

Re: [PATCH] staging: nvec: kbd: use -EINVAL instead of -1

Posted by Dan Carpenter 1 month, 1 week ago

On Wed, Mar 04, 2026 at 01:13:05PM +0530, Soham Kute wrote:
> Hi Dan,
> 
> Thanks for pointing me to smatch. I built the DB and ran the queries.
> 
> >From what I can see, dev->event() is called in input_handle_event()
> but the return value is not propagated back up. The smatch caller_info
> table shows type INTERNAL for that call which I understand means the
> return value stays internal and never reaches the caller of
> input_event().
> 
> So yes, this patch has no runtime effect. The reason I sent it was
> that pcspkr.c uses -EINVAL in the same type of event callback, so I
> thought nvec_kbd should be consistent with that.

Put that kind of thing in the commit message.

"This patch has no effect on runtime but returning proper error codes
is the correct thing and it makes it more consistent with other functions
that implement it such whatever_function() in pcspkr.c"

> 
> Also noticed atkbd.c has the same -1 in its event callback. Should I
> send a fix for that too?

If you want to do that, then sure, absolutely.

regards,
dan carpenter