Hi,
Thanks for the patchset. Some questions and comments on the driver:
1. In the rumble patch, there are some functions that are only used if
CONFIG_JOYCON_FF is enabled. Otherwise, the compiler complains about
unused functions. Could you guard these with #ifdef?
2. Inside hid-ids, most of the defines use tabs, but the current patch
uses spaces. Could you change to use tabs?
3. Currently, the driver quits and prints an error if it is not able to
read the calibration data. That means, if the device is being simulated
using /dev/uhid, it is necessary to also properly respond to these
low-level requests for flash memory reading. This is doable, but is a
lot of work. Any way to allow the driver to continue to function even if
calibration data is missing? Maybe just print a warning that the sticks
won't function well?
4. The word "synchronous" is mispelled
5. Currently, in this driver, the DPAD presses generate key events.
However, most of the other drivers for today's popular controllers, like
DS4 and xbox, produce axis events for these. Is it possible to use axis
in this driver, in order to make it easier for applications to handle
these?
6. There is currently a compiler error in joycon_rumble_amplitudes.
The last (max) amplitude is not a compile-time constant. Suggest to
revise as follows:
static const u16 joycon_max_rumble_amp = 1003;
...
static const struct joycon_rumble_amp_data joycon_rumble_amplitudes[] = {
...
{ 0xc8, 0x0072, joycon_max_rumble_amp }
};
7. In hid-joycon.c, there are currently a lot of defines for different
commands, such as JC_INPUT_BUTTON_EVENT. My personal preference is to
use const u8 for these for type safety.
