linux-kernel - Re: [PATCH v2 2/3] iio: add MEMSensing MSA311 3-axis accelerometer driver

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:   Sun, 12 Jun 2022 11:46:19 +0200
From:   Christophe JAILLET <christophe.jaillet@...adoo.fr>
To:     ddrokosov@...rdevices.ru
Cc:     andy.shevchenko@...il.com, devicetree@...r.kernel.org,
        jic23@...nel.org, kernel@...rdevices.ru, lars@...afoo.de,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org,
        robh+dt@...nel.org, shawnguo@...nel.org, stano.jakubek@...il.com,
        stephan@...hold.net
Subject: Re: [PATCH v2 2/3] iio: add MEMSensing MSA311 3-axis accelerometer
 driver

Le 25/05/2022 à 20:15, Dmitry Rokosov a écrit :
> MSA311 is a tri-axial, low-g accelerometer with I2C digital output for
> sensitivity consumer applications. It has dynamical user selectable full
> scales range of +-2g/+-4g/+-8g/+-16g and allows acceleration measurements
> with output data rates from 1Hz to 1000Hz.
> 
> Datasheet can be found at following URL:
> 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
> 
> Signed-off-by: Dmitry Rokosov <ddrokosov-i4r8oA+eLlH99rHkP+FxIw@...lic.gmane.org>
> ---
>   MAINTAINERS                |    6 +
>   drivers/iio/accel/Kconfig  |   13 +
>   drivers/iio/accel/Makefile |    2 +
>   drivers/iio/accel/msa311.c | 1525 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 1546 insertions(+)
>   create mode 100644 drivers/iio/accel/msa311.c

[...]

> +static int msa311_probe(struct i2c_client *i2c)
> +{
> +	struct msa311_priv *msa311;
> +	struct iio_dev *indio_dev;
> +	struct device *dev = &i2c->dev;
> +	int err;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*msa311));
> +	if (!indio_dev)
> +		return dev_err_probe(dev, -ENOMEM,
> +				     "iio device allocation failed\n");
> +
> +	msa311 = iio_priv(indio_dev);
> +	msa311->i2c = i2c;
> +	i2c_set_clientdata(i2c, indio_dev);
> +
> +	err = msa311_regmap_init(msa311);
> +	if (err)
> +		return err;
> +
> +	mutex_init(&msa311->lock);
> +
> +	err = devm_pm_runtime_enable(dev);
> +	if (err)
> +		return dev_err_probe(dev, err,
> +				     "cannot enable runtime PM (%d)\n", err);
> +

Nit: dev_err_probe() already print the 'err' (in a human readable 
maner), so unless the code itself is of any interest, it can be removed:

i.e.:
+		return dev_err_probe(dev, err,
+				     "cannot enable runtime PM");

This pattern is used in many places.

just my 2c.

CJ