linux-kernel - Re: [PATCH v5 5/9] iio: adc: mcp3911: add support for buffers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 9 Aug 2022 11:52:57 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Marcus Folkesson <marcus.folkesson@...il.com>
Cc:     Kent Gustavsson <kent@...oris.se>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        linux-iio <linux-iio@...r.kernel.org>,
        devicetree <devicetree@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 5/9] iio: adc: mcp3911: add support for buffers

On Tue, Aug 9, 2022 at 9:32 AM Marcus Folkesson
<marcus.folkesson@...il.com> wrote:

Below comments are minor and may be addressed in case you have to
issue another version of the series (depending on what Jonathan asks).

...

> Add support for buffers to make the driver fit for more usecases.

use cases

...

>  #include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/iio/trigger.h>

I would split this group...

>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/spi/spi.h>
>

...and put it here to explicitly show that the driver belongs to the
IIO subsystem.

> +#include <asm/unaligned.h>

...

> +       struct spi_transfer xfer[] = {
> +               {
> +                       .tx_buf = &adc->tx_buf,
> +                       .len = 1,
> +               }, {
> +                       .rx_buf = adc->rx_buf,
> +                       .len = sizeof(adc->rx_buf),
> +               },
> +       };

> +

Redundant blank line.

> +       int scan_index;

...

> +       for_each_set_bit(scan_index, indio_dev->active_scan_mask,
> +                       indio_dev->masklength) {

One line?

> +               const struct iio_chan_spec *scan_chan = &indio_dev->channels[scan_index];
> +
> +               adc->scan.channels[i] = get_unaligned_be24(&adc->rx_buf[scan_chan->channel * 3]);
> +               i++;
> +       }

-- 
With Best Regards,
Andy Shevchenko