[lkml]   [2018]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] HID: Add support for 146b:0902 Bigben Interactive Kids' gamepad
On Tue, May 29, 2018 at 12:57 AM, Roderick Colenbrander
<> wrote:
> Hi Hanno,
> A few suggestions for your patch based on a quick review (had limited time).
> In terms of code style, I noticed a lot of C++ style comments which
> are not part of the kernel code style instead use C comments. To check
> for potential other issues as well, run your patch through
> 'scripts/'.
> I noticed you fixed up reported descriptors a bit to get axes remapped
> in a different way. This is reasonable as the default mappings are
> often not good. However, I would suggest use the mapping functions
> instead (e.g. see hid-sony.c and other drivers). It also allows you to
> properly remap buttons as well. I suspect the current button mapping
> is all over the place as well. Make sure axis and button mapping
> complies to the Linux gamepad spec (see
> Documentation/input/gamepad.rst).

Thanks a lot Roderick for the review.

In addition to those comments, please also try to follow the
submission guidelines (point 6 of
It's easier for us to share comments if your patch is inlined in the
email, so we can directly pinpoint which line is in trouble.

Also, keep point 9 in mind ("Don't get discouraged - or impatient").
For the first submissions, it's usual to have several revisions of the
same series.


> Thanks,
> Roderick
> On Wed, May 23, 2018 at 8:57 AM, Hanno Zulla <> wrote:
>> Hello,
>> this is a driver for the "BigBen Interactive Kid-friendly Wired
>> Controller PS3OFMINIPAD SONY" [1] with USB id 146b:0902. It was
>> originally sold as a PS3 accessory and serves as a very nice gamepad for
>> Retropie.
>> With the help of serialusb [2], it was possible to analyze the protocol
>> used by the gamepad and fix the missing entries for the descriptor.
>> This is my *first* driver, so please review accordingly.
>> Three things where I'm not sure and hope to hear from you about:
>> - is that the correct use of hid_validate_values() in bigben_probe()?
>> - is the error handling in bigben_probe() correct?
>> - how do I properly test possible suspend/resume scenarios?
>> Thanks,
>> Hanno
>> [1]
>> <>
>> [2] <>

 \ /
  Last update: 2018-05-29 09:13    [W:0.056 / U:0.200 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site