Hi Andy,
Le mar. 19 mai 2020 à 23:43, Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> a écrit :
On Sun, May 17, 2020 at 10:49 PM Artur Rojek <contact@xxxxxxxxxxxxxx>
wrote:
Add a driver for joystick devices connected to ADC controllers
supporting the Industrial I/O subsystem.
...
+static int adc_joystick_handle(const void *data, void *private)
+{
+ struct adc_joystick *joy = private;
+ enum iio_endian endianness;
+ int bytes, msb, val, i;
+ bool sign;
+
+ bytes = joy->chans[0].channel->scan_type.storagebits >> 3;
+
+ for (i = 0; i < joy->num_chans; ++i) {
+ endianness =
joy->chans[i].channel->scan_type.endianness;
+ msb = joy->chans[i].channel->scan_type.realbits - 1;
+ sign =
(tolower(joy->chans[i].channel->scan_type.sign) == 's');
Do we need tolower()?
I'll answer this one:
The sign can be uppercase to specify that the value is sign-extended in
all the storage bits.
-Paul
+
+ switch (bytes) {
+ case 1:
+ val = ((const u8 *)data)[i];
+ break;
+ case 2:
+ if (endianness == IIO_BE)
+ val = be16_to_cpu(((const u16
*)data)[i]);
Yeah, you have to provide bitwise types to satisfy sparse.
Maybe using *_to_cpup() will cure this.
+ else if (endianness == IIO_LE)
+ val = le16_to_cpu(((const u16
*)data)[i]);
+ else /* IIO_CPU */
+ val = ((const u16 *)data)[i];
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ val >>= joy->chans[i].channel->scan_type.shift;
+ if (sign)
+ val = sign_extend32(val, msb);
+ else
+ val &= GENMASK(msb, 0);
+ input_report_abs(joy->input, joy->axes[i].code,
val);
+ }
+
+ input_sync(joy->input);
+
+ return 0;
+}
...
+ /* Count how many channels we got. NULL terminated. */
+ while (joy->chans[joy->num_chans].indio_dev)
+ joy->num_chans++;
I don't see how useful this is. Why not simple do below...
+ bits = joy->chans[0].channel->scan_type.storagebits;
+ if (!bits || (bits > 16)) {
+ dev_err(dev, "Unsupported channel storage size");
+ return -EINVAL;
+ }
+ for (i = 1; i < joy->num_chans; ++i)
+ if (joy->chans[i].channel->scan_type.storagebits !=
bits) {
+ dev_err(dev, "Channels must have equal
storage size");
+ return -EINVAL;
+ }
...something like
for (i = 0; joy->chans[i].indio_dev; i++) {
bits = joy->chans[i].channel->scan_type.storagebits;
if (bits ...) {
...error handling...
}
if (bits != joy->chans[0].channel->scan_type.storagebits) {
...second level of error handling...
}
}
...
+static const struct of_device_id adc_joystick_of_match[] = {
+ { .compatible = "adc-joystick", },
+ { },
No need comma.
+};
--
With Best Regards,
Andy Shevchenko
