linux-kernel - Re: [PATCH v2 2/7] iio: inv_mpu6050: Initial regcache support

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: <e0cee595-043b-3642-9e40-2a23975e855a@axentia.se>
Date:	Fri, 20 May 2016 08:39:05 +0200
From:	Peter Rosin <peda@...ntia.se>
To:	Matt Ranostay <mranostay@...il.com>,
	Crestez Dan Leonard <leonard.crestez@...el.com>
CC:	Jonathan Cameron <jic23@...nel.org>,
	"linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Hartmut Knaack <knaack.h@....de>,
	Lars-Peter Clausen <lars@...afoo.de>,
	Peter Meerwald-Stadler <pmeerw@...erw.net>,
	Daniel Baluta <daniel.baluta@...el.com>,
	Ge Gao <ggao@...ensense.com>, <linux-i2c@...r.kernel.org>,
	Wolfram Sang <wsa@...-dreams.de>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>
Subject: Re: [PATCH v2 2/7] iio: inv_mpu6050: Initial regcache support

Hi!

On 2016-05-20 04:34, Matt Ranostay wrote:
> On Wed, May 18, 2016 at 8:00 AM, Crestez Dan Leonard
> <leonard.crestez@...el.com> wrote:
>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@...el.com>
>> ---
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_core.c | 47 ++++++++++++++++++++++++++++++
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_i2c.c  |  5 ----
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_iio.h  |  1 +
>>  drivers/iio/imu/inv_mpu6050/inv_mpu_spi.c  |  5 ----
>>  4 files changed, 48 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> index b269b37..5918c23 100644
>> --- a/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> +++ b/drivers/iio/imu/inv_mpu6050/inv_mpu_core.c
>> @@ -116,6 +116,53 @@ static const struct inv_mpu6050_hw hw_info[] = {
>>         },
>>  };
>>
>> +static bool inv_mpu6050_volatile_reg(struct device *dev, unsigned int reg)
>> +{
>> +       if (reg >= INV_MPU6050_REG_RAW_ACCEL && reg < INV_MPU6050_REG_RAW_ACCEL + 6)
>> +               return true;
>> +       if (reg >= INV_MPU6050_REG_RAW_GYRO && reg < INV_MPU6050_REG_RAW_GYRO + 6)
>> +               return true;
> 
> I think you want to put parenthesis around the addition operations...

Maybe.

> the condition check probably don't evaluate to what you are expecting.

Looks sane to me since + has highest precedence, then < and >=, and && comes
in last...

>> +       switch (reg) {
>> +       case INV_MPU6050_REG_TEMPERATURE:
>> +       case INV_MPU6050_REG_TEMPERATURE + 1:
>> +       case INV_MPU6050_REG_USER_CTRL:
>> +       case INV_MPU6050_REG_PWR_MGMT_1:
>> +       case INV_MPU6050_REG_FIFO_COUNT_H:
>> +       case INV_MPU6050_REG_FIFO_COUNT_H + 1:
>> +       case INV_MPU6050_REG_FIFO_R_W:
>> +               return true;
>> +       default:
>> +               return false;
>> +       }
>> +}

...but even so, I think I would use an ellipsis in the switch statement
instead, like so:

static bool inv_mpu6050_volatile_reg(struct device *dev, unsigned int reg)
{
	switch (reg) {
	case INV_MPU6050_REG_RAW_ACCEL ... INV_MPU6050_REG_RAW_ACCEL + 5:
	case INV_MPU6050_REG_RAW_GYRO ... INV_MPU6050_REG_RAW_GYRO  + 5:
	case INV_MPU6050_REG_TEMPERATURE:
	case INV_MPU6050_REG_TEMPERATURE + 1:
	case INV_MPU6050_REG_USER_CTRL:
	case INV_MPU6050_REG_PWR_MGMT_1:
	case INV_MPU6050_REG_FIFO_COUNT_H:
	case INV_MPU6050_REG_FIFO_COUNT_H + 1:
	case INV_MPU6050_REG_FIFO_R_W:
		return true;
	default:
		return false;
	}
}

Cheers,
Peter