linux-kernel - Re: [PATCH v4 06/11] iio: accel: adxl313: add basic interrupt handling for FIFO watermark

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]

Message-ID: <CAHp75Ve6nifph44F-_sOwqSBBy_Ay5BeH9QxWMmMUNR1N7wUzQ@mail.gmail.com>
Date: Sun, 1 Jun 2025 22:26:42 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Lothar Rubusch <l.rubusch@...il.com>
Cc: jic23@...nel.org, dlechner@...libre.com, nuno.sa@...log.com, 
	andy@...nel.org, corbet@....net, lucas.p.stankus@...il.com, lars@...afoo.de, 
	Michael.Hennerich@...log.com, bagasdotme@...il.com, linux-iio@...r.kernel.org, 
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 06/11] iio: accel: adxl313: add basic interrupt
 handling for FIFO watermark

On Sun, Jun 1, 2025 at 8:22 PM Lothar Rubusch <l.rubusch@...il.com> wrote:
>
> Prepare the interrupt handler. Add register entries to evaluate the
> incoming interrupt. Add functions to clear status registers and reset the
> FIFO.
>
> Add FIFO watermark configuration and evaluation. Let a watermark to be
> configured. Evaluate the interrupt accordingly. Read out the FIFO content
> and push the values to the IIO channel.

...

> +static int adxl313_set_watermark(struct iio_dev *indio_dev, unsigned int value)
> +{
> +       struct adxl313_data *data = iio_priv(indio_dev);
> +       const unsigned int fifo_mask = 0x1f, interrupt_mask = 0x02;

GENMASK()
BIT()

> +       int ret;
> +
> +       value = min(value, ADXL313_FIFO_SIZE - 1);
> +
> +       ret = regmap_update_bits(data->regmap, ADXL313_REG_FIFO_CTL,
> +                                fifo_mask, value);
> +       if (ret)
> +               return ret;
> +
> +       data->watermark = value;
> +
> +       return regmap_update_bits(data->regmap, ADXL313_REG_INT_ENABLE,
> +                                 interrupt_mask, ADXL313_INT_WATERMARK);
> +}

...

> +static int adxl313_get_samples(struct adxl313_data *data)
> +{
> +       unsigned int regval = 0;

Useless assignment.

> +       int ret;
> +
> +       ret = regmap_read(data->regmap, ADXL313_REG_FIFO_STATUS, &regval);
> +       if (ret)
> +               return ret;
> +
> +       return FIELD_GET(ADXL313_REG_FIFO_STATUS_ENTRIES_MSK, regval);
> +}

...

> +               ret = devm_request_threaded_irq(dev, irq, NULL,
> +                                               &adxl313_irq_handler,
> +                                               IRQF_SHARED | IRQF_ONESHOT,
> +                                               indio_dev->name, indio_dev);
> +               if (ret)
> +                       return ret;

Now I see the first user of 'irq'. Logically these two patches may not
be split. Or split should be made differently, let's say IRQ type
holding variable + switch case can go in the first preparatory patch
(however it will make a little sense without real users, as it is/will
be a dead code).

-- 
With Best Regards,
Andy Shevchenko