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: <20250608163005.71b06e27@jic23-huawei>
Date: Sun, 8 Jun 2025 16:30:05 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>
Cc: Lothar Rubusch <l.rubusch@...il.com>, 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, 1 Jun 2025 22:26:42 +0300
Andy Shevchenko <andy.shevchenko@...il.com> wrote:

> 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).
> 

I'd just combine these two patches and patch 2 (which is also dead code
until this one is in place).

Jonathan