On Sat, Mar 28, 2020 at 12:37 AM Gwendal Grignou <gwendal@xxxxxxxxxxxx> wrote: > > Add buffer/hwfifo_flush. It is not part of the ABI, but it follows ST > and HID lead: Tells the sensor hub to send to the host all pending > sensor events. I see where discussion is going, but nevertheless some comments below that you will not make same mistakes in the future. ... > + int ret = 0; Useless assignment. > + bool flush; > + > + ret = strtobool(buf, &flush); kstrtobool() > + if (ret < 0) Positive error codes? I'm not sure it returns a such. So ' < 0' part is redundant. > + return ret; > + if (!flush) > + return -EINVAL; This I didn't get, you have accept only true as input? It's really strange. > + ret = cros_ec_motion_send_host_cmd(st, 0); > + if (ret != 0) Similar to above ' != 0' part is redundant. > + dev_warn(&indio_dev->dev, "Unable to flush sensor\n"); ... > +static IIO_DEVICE_ATTR(hwfifo_flush, 0644, NULL, > + cros_ec_sensors_flush, 0); IIO_DEVICE_ATTR_RW() ? -- With Best Regards, Andy Shevchenko