[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VeRZO9DnTRZWavfp2FPjg_kqmpNr0VZnXv1U3G4Pr0iFQ@mail.gmail.com>
Date: Sun, 29 Mar 2020 14:22:11 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Gwendal Grignou <gwendal@...omium.org>
Cc: Benson Leung <bleung@...omium.org>,
Enric Balletbo i Serra <enric.balletbo@...labora.com>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
linux-iio <linux-iio@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v7 12/12] iio: cros_ec: flush as hwfifo attribute
On Sat, Mar 28, 2020 at 12:37 AM Gwendal Grignou <gwendal@...omium.org> 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
Powered by blists - more mailing lists