linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar
@ 2025-02-16 21:42 Dan Carpenter
  2025-02-17  4:50 ` Aditya Garg
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2025-02-16 21:42 UTC (permalink / raw)
  To: Aditya Garg; +Cc: linux-input

Hello Aditya Garg,

Commit 93a0fc489481 ("HID: hid-appletb-kbd: add support for automatic
brightness control while using the touchbar") from Dec 31, 2024
(linux-next), leads to the following Smatch static checker warning:

	drivers/hid/hid-appletb-kbd.c:406 appletb_kbd_probe()
	warn: passing zero to 'dev_err_probe'

drivers/hid/hid-appletb-kbd.c
    372 static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id *id)
    373 {
    374         struct appletb_kbd *kbd;
    375         struct device *dev = &hdev->dev;
    376         struct hid_field *mode_field;
    377         int ret;
    378 
    379         ret = hid_parse(hdev);
    380         if (ret)
    381                 return dev_err_probe(dev, ret, "HID parse failed\n");
    382 
    383         mode_field = hid_find_field(hdev, HID_OUTPUT_REPORT,
    384                                     HID_GD_KEYBOARD, HID_USAGE_MODE);
    385         if (!mode_field)
    386                 return -ENODEV;
    387 
    388         kbd = devm_kzalloc(dev, sizeof(*kbd), GFP_KERNEL);
    389         if (!kbd)
    390                 return -ENOMEM;
    391 
    392         kbd->mode_field = mode_field;
    393 
    394         ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
    395         if (ret)
    396                 return dev_err_probe(dev, ret, "HID hw start failed\n");
    397 
    398         ret = hid_hw_open(hdev);
    399         if (ret) {
    400                 dev_err_probe(dev, ret, "HID hw open failed\n");
    401                 goto stop_hw;
    402         }
    403 
    404         kbd->backlight_dev = backlight_device_get_by_name("appletb_backlight");
    405                 if (!kbd->backlight_dev)
--> 406                         dev_err_probe(dev, ret, "Failed to get backlight device\n");
                                                   ^^^
s/ret/-EINVAL/?

Why is this indented an extra tab?

    407                 else {
    408                         backlight_device_set_brightness(kbd->backlight_dev, 2);
    409                         timer_setup(&kbd->inactivity_timer, appletb_inactivity_timer, 0);
    410                         mod_timer(&kbd->inactivity_timer, jiffies + msecs_to_jiffies(appletb_tb_dim_timeout * 1000));
    411                 }
    412 

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar
  2025-02-16 21:42 [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar Dan Carpenter
@ 2025-02-17  4:50 ` Aditya Garg
  2025-02-17  5:26   ` Dan Carpenter
  2025-02-17  5:27   ` Aditya Garg
  0 siblings, 2 replies; 5+ messages in thread
From: Aditya Garg @ 2025-02-17  4:50 UTC (permalink / raw)
  To: Dan Carpenter, Jiri Kosina, jkosina@suse.com; +Cc: linux-input@vger.kernel.org

Hi Dan

Thanks for the report

> On 17 Feb 2025, at 3:12 AM, Dan Carpenter <dan.carpenter@linaro.org> wrote:
> 
> Hello Aditya Garg,
> 
> Commit 93a0fc489481 ("HID: hid-appletb-kbd: add support for automatic
> brightness control while using the touchbar") from Dec 31, 2024
> (linux-next), leads to the following Smatch static checker warning:
> 
> drivers/hid/hid-appletb-kbd.c:406 appletb_kbd_probe()
> warn: passing zero to 'dev_err_probe'
> 
> drivers/hid/hid-appletb-kbd.c
>    372 static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id *id)
>    373 {
>    374         struct appletb_kbd *kbd;
>    375         struct device *dev = &hdev->dev;
>    376         struct hid_field *mode_field;
>    377         int ret;
>    378 
>    379         ret = hid_parse(hdev);
>    380         if (ret)
>    381                 return dev_err_probe(dev, ret, "HID parse failed\n");
>    382 
>    383         mode_field = hid_find_field(hdev, HID_OUTPUT_REPORT,
>    384                                     HID_GD_KEYBOARD, HID_USAGE_MODE);
>    385         if (!mode_field)
>    386                 return -ENODEV;
>    387 
>    388         kbd = devm_kzalloc(dev, sizeof(*kbd), GFP_KERNEL);
>    389         if (!kbd)
>    390                 return -ENOMEM;
>    391 
>    392         kbd->mode_field = mode_field;
>    393 
>    394         ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
>    395         if (ret)
>    396                 return dev_err_probe(dev, ret, "HID hw start failed\n");
>    397 
>    398         ret = hid_hw_open(hdev);
>    399         if (ret) {
>    400                 dev_err_probe(dev, ret, "HID hw open failed\n");
>    401                 goto stop_hw;
>    402         }
>    403 
>    404         kbd->backlight_dev = backlight_device_get_by_name("appletb_backlight");
>    405                 if (!kbd->backlight_dev)
> --> 406                         dev_err_probe(dev, ret, "Failed to get backlight device\n");
>                                                   ^^^
> s/ret/-EINVAL/?

Should be -ENODEV

> 
> Why is this indented an extra tab?

Thanks for pointing this out.

Patch series with some issues I myself found out, along with the one you flagged are sent here:

https://lore.kernel.org/linux-input/8365C1B3-3A38-4F6E-955B-D6BBABA6B00A@live.com/

Cheers!
Aditya


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar
  2025-02-17  4:50 ` Aditya Garg
@ 2025-02-17  5:26   ` Dan Carpenter
  2025-02-17  5:27   ` Aditya Garg
  1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-02-17  5:26 UTC (permalink / raw)
  To: Aditya Garg; +Cc: Jiri Kosina, jkosina@suse.com, linux-input@vger.kernel.org

On Mon, Feb 17, 2025 at 04:50:19AM +0000, Aditya Garg wrote:
> 
> Patch series with some issues I myself found out, along with the one you flagged are sent here:
> 
> https://lore.kernel.org/linux-input/8365C1B3-3A38-4F6E-955B-D6BBABA6B00A@live.com/
> 

Thanks!

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar
  2025-02-17  4:50 ` Aditya Garg
  2025-02-17  5:26   ` Dan Carpenter
@ 2025-02-17  5:27   ` Aditya Garg
  2025-02-17  7:00     ` Dan Carpenter
  1 sibling, 1 reply; 5+ messages in thread
From: Aditya Garg @ 2025-02-17  5:27 UTC (permalink / raw)
  To: Dan Carpenter, Jiri Kosina, jkosina@suse.com; +Cc: linux-input@vger.kernel.org



> On 17 Feb 2025, at 10:20 AM, Aditya Garg <gargaditya08@live.com> wrote:
> 
> Hi Dan
> 
> Thanks for the report
> 
>> On 17 Feb 2025, at 3:12 AM, Dan Carpenter <dan.carpenter@linaro.org> wrote:
>> 
>> Hello Aditya Garg,
>> 
>> Commit 93a0fc489481 ("HID: hid-appletb-kbd: add support for automatic
>> brightness control while using the touchbar") from Dec 31, 2024
>> (linux-next), leads to the following Smatch static checker warning:
>> 
>> drivers/hid/hid-appletb-kbd.c:406 appletb_kbd_probe()
>> warn: passing zero to 'dev_err_probe'
>> 
>> drivers/hid/hid-appletb-kbd.c
>>   372 static int appletb_kbd_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>   373 {
>>   374         struct appletb_kbd *kbd;
>>   375         struct device *dev = &hdev->dev;
>>   376         struct hid_field *mode_field;
>>   377         int ret;
>>   378 
>>   379         ret = hid_parse(hdev);
>>   380         if (ret)
>>   381                 return dev_err_probe(dev, ret, "HID parse failed\n");
>>   382 
>>   383         mode_field = hid_find_field(hdev, HID_OUTPUT_REPORT,
>>   384                                     HID_GD_KEYBOARD, HID_USAGE_MODE);
>>   385         if (!mode_field)
>>   386                 return -ENODEV;
>>   387 
>>   388         kbd = devm_kzalloc(dev, sizeof(*kbd), GFP_KERNEL);
>>   389         if (!kbd)
>>   390                 return -ENOMEM;
>>   391 
>>   392         kbd->mode_field = mode_field;
>>   393 
>>   394         ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
>>   395         if (ret)
>>   396                 return dev_err_probe(dev, ret, "HID hw start failed\n");
>>   397 
>>   398         ret = hid_hw_open(hdev);
>>   399         if (ret) {
>>   400                 dev_err_probe(dev, ret, "HID hw open failed\n");
>>   401                 goto stop_hw;
>>   402         }
>>   403 
>>   404         kbd->backlight_dev = backlight_device_get_by_name("appletb_backlight");
>>   405                 if (!kbd->backlight_dev)
>> --> 406                         dev_err_probe(dev, ret, "Failed to get backlight device\n");
>>                                                  ^^^
>> s/ret/-EINVAL/?
> 
> Should be -ENODEV
> 
>> 
>> Why is this indented an extra tab?
> 
> Thanks for pointing this out.
> 
> Patch series with some issues I myself found out, along with the one you flagged are sent here:


Btw, I wonder what’s wrong with checkpatch. I didn't get any errors/warnings regarding these.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar
  2025-02-17  5:27   ` Aditya Garg
@ 2025-02-17  7:00     ` Dan Carpenter
  0 siblings, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2025-02-17  7:00 UTC (permalink / raw)
  To: Aditya Garg; +Cc: Jiri Kosina, jkosina@suse.com, linux-input@vger.kernel.org

On Mon, Feb 17, 2025 at 05:27:42AM +0000, Aditya Garg wrote:
> >>   397 
> >>   398         ret = hid_hw_open(hdev);
> >>   399         if (ret) {
> >>   400                 dev_err_probe(dev, ret, "HID hw open failed\n");
> >>   401                 goto stop_hw;
> >>   402         }
> >>   403 
> >>   404         kbd->backlight_dev = backlight_device_get_by_name("appletb_backlight");
> >>   405                 if (!kbd->backlight_dev)
> >> --> 406                         dev_err_probe(dev, ret, "Failed to get backlight device\n");
> >>                                                  ^^^
> >> s/ret/-EINVAL/?
> > 
> > Should be -ENODEV
> > 
> >> 
> >> Why is this indented an extra tab?
> > 
> > Thanks for pointing this out.
> > 
> > Patch series with some issues I myself found out, along with the one you flagged are sent here:
> 
> 
> Btw, I wonder what’s wrong with checkpatch. I didn't get any errors/warnings regarding these.


These are Smatch warnings:
https://github.com/error27/smatch/blob/master/Documentation/smatch.rst

~/smatch_dir/smatch_scripts/kchecker drivers/hid/hid-appletb-kbd.c

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-02-17  7:00 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-16 21:42 [bug report] HID: hid-appletb-kbd: add support for automatic brightness control while using the touchbar Dan Carpenter
2025-02-17  4:50 ` Aditya Garg
2025-02-17  5:26   ` Dan Carpenter
2025-02-17  5:27   ` Aditya Garg
2025-02-17  7:00     ` Dan Carpenter

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).