lkml.org 
[lkml]   [2022]   [Aug]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 6/7] iio: add MEMSensing MSA311 3-axis accelerometer driver
On Fri, 12 Aug 2022 16:52:29 +0000
Dmitry Rokosov <DDRokosov@sberdevices.ru> wrote:

> MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
> sensitivity consumer applications. It has dynamic user-selectable full
> scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
> with output data rates from 1Hz to 1000Hz.
>
> Spec: https://cdn-shop.adafruit.com/product-files/5309/MSA311-V1.1-ENG.pdf
>
> This driver supports following MSA311 features:
> - IIO interface
> - Different power modes: NORMAL and SUSPEND (using pm_runtime)
> - ODR (Output Data Rate) selection
> - Scale and samp_freq selection
> - IIO triggered buffer, IIO reg access
> - NEW_DATA interrupt + trigger
>
> Below features to be done:
> - Motion Events: ACTIVE, TAP, ORIENT, FREEFALL
> - Low Power mode
>
> Signed-off-by: Dmitry Rokosov <ddrokosov@sberdevices.ru>
Hi Dmitry, a few minor comments from another read through.

> + */
> +static inline int msa311_set_odr(struct msa311_priv *msa311, unsigned int odr)
> +{
> + struct device *dev = msa311->dev;
> + unsigned int pwr_mode;
> + bool good_odr = false;
> + int err;
> +
> + err = regmap_field_read(msa311->fields[F_PWR_MODE], &pwr_mode);
> + if (err)
> + return err;
> +
> + /* Filter bad ODR values */
> + if (pwr_mode == MSA311_PWR_MODE_NORMAL)
> + good_odr = (odr > MSA311_ODR_1_95_HZ);
> +
> + if (!good_odr) {
> + dev_err(dev,
> + "cannot set odr %u.%luHz, not available in %s mode\n",

Don't you need to zero pad the fractional part?
or use iio_format_values.

> + msa311_odr_table[odr].val,
> + msa311_odr_table[odr].val2 / MILLIHZ_PER_HZ,
> + msa311_pwr_modes[pwr_mode]);
> + return -EINVAL;
> + }
> +
> + return regmap_field_write(msa311->fields[F_ODR], odr);
> +}


> +
> +static int msa311_setup_interrupts(struct msa311_priv *msa311)
> +{
> + struct device *dev = msa311->dev;
> + struct i2c_client *i2c = to_i2c_client(dev);
> + struct iio_dev *indio_dev = i2c_get_clientdata(i2c);
> + struct iio_trigger *trig;
> + int err;
> +
> + /* Keep going without interrupts if no initialized I2C irq */
> + if (i2c->irq <= 0)
> + return 0;
> +
> + err = devm_request_threaded_irq(&i2c->dev, i2c->irq,
> + NULL, msa311_irq_thread,
> + IRQF_ONESHOT,
> + msa311->chip_name,
> + indio_dev);

Could wrap this a little less given you are respinning anyway. It's way under
80 chars currently.

...
"cannot map new data interrupt\n");
> +
> + return 0;
> +}
> +

> + msa311->vdd = devm_regulator_get_optional(dev, "vdd");
> + if (IS_ERR(msa311->vdd)) {
> + err = PTR_ERR(msa311->vdd);
> + if (err == -ENODEV)
> + msa311->vdd = NULL;
> + else
> + return dev_err_probe(dev, PTR_ERR(msa311->vdd),

use err instead of PTR_ERR(msa311->vdd) ?

> + "cannot get vdd supply\n");
> + }
> +
> + if (msa311->vdd) {
> + err = regulator_enable(msa311->vdd);
> + if (err)
> + return dev_err_probe(dev, err,
> + "cannot enable vdd supply\n");
> +
> + err = devm_add_action_or_reset(dev, msa311_vdd_disable,
> + msa311->vdd);
> + if (err) {
> + return dev_err_probe(dev, err,
> + "cannot add vdd disable action\n");
> + }
> + }
> +
> + err = msa311_check_partid(msa311);
> + if (err)
> + return err;
> +
> + err = msa311_soft_reset(msa311);
> + if (err)
> + return err;
> +

\
 
 \ /
  Last update: 2022-08-14 18:45    [W:0.095 / U:0.608 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site