linux-kernel - Re: [PATCH v5 5/5] iio: pressure: bmp280: Add more tunable config parameters for BMP380

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:   Mon, 12 Sep 2022 02:53:06 +0200
From:   Angel Iglesias <ang.iglesiasg@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     linux-iio <linux-iio@...r.kernel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Paul Cercueil <paul@...pouillou.net>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 5/5] iio: pressure: bmp280: Add more tunable config
 parameters for BMP380

On Mon, 2022-08-08 at 11:13 +0200, Andy Shevchenko wrote:
> On Sun, Aug 7, 2022 at 2:44 PM Angel Iglesias <ang.iglesiasg@...il.com> wrote:
> > 
> > Allows sampling frequency and IIR filter coefficients configuration
> > using sysfs ABI.
> > 
> > The IIR filter coefficient is configurable using the sysfs attribute
> > "filter_low_pass_3db_frequency".
> 
> ...
> 
> > +static const int bmp380_odr_table[][2] = {
> 
> s32_fract ?

I modeled this bit and other ODR representations after the adxl355 driver. I see
that s32_fract would be a bit cleaner than having arrays inside arrays, but I'm
failing to see which additional advantages would provide.
Also, technically, these are precomputed frequencies, the first index is the
integer part and the second is the fractional part. The fractions would be
200/1, 200/2, 200/4 ... 200/131072

> > +       [BMP380_ODR_200HZ]      = {200, 0},
> > +       [BMP380_ODR_100HZ]      = {100, 0},
> > +       [BMP380_ODR_50HZ]       = {50, 0},
> > +       [BMP380_ODR_25HZ]       = {25, 0},
> > +       [BMP380_ODR_12_5HZ]     = {12, 500000},
> > +       [BMP380_ODR_6_25HZ]     = {6, 250000},
> > +       [BMP380_ODR_3_125HZ]    = {3, 125000},
> > +       [BMP380_ODR_1_5625HZ]   = {1, 562500},
> > +       [BMP380_ODR_0_78HZ]     = {0, 781250},
> > +       [BMP380_ODR_0_39HZ]     = {0, 390625},
> > +       [BMP380_ODR_0_2HZ]      = {0, 195313},
> > +       [BMP380_ODR_0_1HZ]      = {0, 97656},
> > +       [BMP380_ODR_0_05HZ]     = {0, 48828},
> > +       [BMP380_ODR_0_02HZ]     = {0, 24414},
> > +       [BMP380_ODR_0_01HZ]     = {0, 12207},
> > +       [BMP380_ODR_0_006HZ]    = {0, 6104},
> > +       [BMP380_ODR_0_003HZ]    = {0, 3052},
> > +       [BMP380_ODR_0_0015HZ]   = {0, 1526},
> > +};
> 
> ...
> 
> > +               ret = regmap_write_bits(data->regmap,
> > BMP380_REG_POWER_CONTROL,
> > +                                       BMP380_MODE_MASK,
> 
> > +                                       FIELD_PREP(BMP380_MODE_MASK,
> > +                                                  BMP380_MODE_SLEEP));
> 
> One line?
> 
> ...
> 
> > +               ret = regmap_write_bits(data->regmap,
> > BMP380_REG_POWER_CONTROL,
> > +                                       BMP380_MODE_MASK,
> 
> > +                                       FIELD_PREP(BMP380_MODE_MASK,
> > +                                                  BMP380_MODE_NORMAL));
> 
> Ditto.
> 
> ...
> 
> > +static const int bmp380_iir_filter_coeffs_avail[] = { 0, 1, 3, 7, 15, 31,
> > 63, 127 };
> 
> This seems like a power of two - 1, can it be replaced by a formula in the
> code?
>