> -----Original Message----- > From: Datta, Shubhrajyoti > > > -----Original Message----- > > From: linux-iio-owner@xxxxxxxxxxxxxxx [mailto:linux-iio- > > owner@xxxxxxxxxxxxxxx] On Behalf Of Jonathan Cameron > > Sent: Friday, October 15, 2010 3:51 PM > > To: Matthias Brugger > > Cc: linux-iio@xxxxxxxxxxxxxxx; matthias > > Subject: Re: [PATCH] iio - add barometer bmp085 > > > > On 10/15/10 10:16, Matthias Brugger wrote: > > > This patch adds: > > > - digital barometer support to the iio subsytem > > > - Bosch Sensortec bmp085 driver > > > > > > I resend the patches I sent yesterday, because I merged them to one. > > > Also the comments and the define has been deleted and so, from my > point > > of view is ready to merge. > > One quick comment about patch submissions in general. When it's an > updated > > version of a previously posted patch, the title should be something like > > [PATCH V2] iio - add barometer bmp085. Makes it easy down the line to > > make > > sure you have the right patch!. > > > > To sumarise the important inline comments: > > > > As a general rule, if you know the size of the storage of a give value > > (because it is comming from hardware) it is better to use fixed sized > > types. u16 and friends. It's way too unpredictable how large a long or > > a short might be. > > > > Don't eat errors. > > > > All channel naming must have either _raw or _input on the end of > > the direct read attributes so we know whether it is already in SI units. > > > > Mostly fine, but various comments inline. > > Some of the comments are fixed in the following you may want to see > Drivers/misc/bmp085.c. Does this work for you? > > Some of the points > 1. You may solve the dependency of the pressure needing the temperature by > triggering the temperature measurement. > 2. Secondly I personally prefer the temperature reported in milli units. > However that will standardize various. Also solves the decimal point > issue. > However that will standardize various. - > drivers. > However I am OK with your take. > > 3. I appreciate your attempts to make it an IIO as I think it may make it > more futuristic. > > > > > > > > > > Thanks, > > > > Jonathan > > > > > > > > Signed-off-by: Matthias Brugger <m_brugger@xxxxxx> > > > --- > > > drivers/staging/iio/Kconfig | 1 + > > > drivers/staging/iio/Makefile | 1 + > > > drivers/staging/iio/barometer/Kconfig | 12 + > > > drivers/staging/iio/barometer/Makefile | 5 + > > > drivers/staging/iio/barometer/baro.h | 8 + > > > drivers/staging/iio/barometer/bmp085.c | 418 > > ++++++++++++++++++++++++++++++++ > > > drivers/staging/iio/barometer/bmp085.h | 73 ++++++ > > > 7 files changed, 518 insertions(+), 0 deletions(-) > > > create mode 100644 drivers/staging/iio/barometer/Kconfig > > > create mode 100644 drivers/staging/iio/barometer/Makefile > > > create mode 100644 drivers/staging/iio/barometer/baro.h > > > create mode 100644 drivers/staging/iio/barometer/bmp085.c > > > create mode 100644 drivers/staging/iio/barometer/bmp085.h > > > > > > diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig > > > index ed48815..d5ca09a 100644 > > > --- a/drivers/staging/iio/Kconfig > > > +++ b/drivers/staging/iio/Kconfig > > > @@ -46,6 +46,7 @@ source "drivers/staging/iio/gyro/Kconfig" > > > source "drivers/staging/iio/imu/Kconfig" > > > source "drivers/staging/iio/light/Kconfig" > > > source "drivers/staging/iio/magnetometer/Kconfig" > > > +source "drivers/staging/iio/barometer/Kconfig" > > > > > > source "drivers/staging/iio/trigger/Kconfig" > > > > > > diff --git a/drivers/staging/iio/Makefile > b/drivers/staging/iio/Makefile > > > index e909674..73112b2 100644 > > > --- a/drivers/staging/iio/Makefile > > > +++ b/drivers/staging/iio/Makefile > > > @@ -16,3 +16,4 @@ obj-y += imu/ > > > obj-y += light/ > > > obj-y += trigger/ > > > obj-y += magnetometer/ > > > +obj-y += barometer/ > > > diff --git a/drivers/staging/iio/barometer/Kconfig > > b/drivers/staging/iio/barometer/Kconfig > > > new file mode 100644 > > > index 0000000..d5942e9 > > > --- /dev/null > > > +++ b/drivers/staging/iio/barometer/Kconfig > > > @@ -0,0 +1,12 @@ > > > +# > > > +# IIO Digital Barometer Sensor drivers configuration > > > +# > > > +comment "Digital barometer sensors" > > > + > > > +config BMP085 > > > + > > > + tristate "BMP085 Barometer Sensor I2C driver" > > > + depends on I2C > > > + help > > > + Say yes here to build support for Bosch Sensortec BMP085, > > > + digital barometer sensor. > > > diff --git a/drivers/staging/iio/barometer/Makefile > > b/drivers/staging/iio/barometer/Makefile > > > new file mode 100644 > > > index 0000000..3234d96 > > > --- /dev/null > > > +++ b/drivers/staging/iio/barometer/Makefile > > > @@ -0,0 +1,5 @@ > > > +# > > > +# Makefile for digital barometer sensor drivers > > > +# > > > + > > > +obj-$(CONFIG_BMP085) += bmp085.o > > > diff --git a/drivers/staging/iio/barometer/baro.h > > b/drivers/staging/iio/barometer/baro.h > > > new file mode 100644 > > > index 0000000..e3b73a1 > > > --- /dev/null > > > +++ b/drivers/staging/iio/barometer/baro.h > > > @@ -0,0 +1,8 @@ > > > + > > > +#include "../sysfs.h" > > > + > > > +/* Barometer types of attribute */ > > > + > > > +#define IIO_DEV_ATTR_BARO_PRESSURE(_mode, _show, _store, _addr) \ > > > + IIO_DEVICE_ATTR(baro_pressure, _mode, _show, NULL, _addr) > > If it's processed (i.e. si units) it needs to be baro_pressure_input. > > If raw it needs to be baro_pressure_raw and conversion factors etc > > provided. > > > + > > > diff --git a/drivers/staging/iio/barometer/bmp085.c > > b/drivers/staging/iio/barometer/bmp085.c > > > new file mode 100644 > > > index 0000000..67dac39 > > > --- /dev/null > > > +++ b/drivers/staging/iio/barometer/bmp085.c > > > @@ -0,0 +1,418 @@ > > > +/** > > > + * Bosch Sensortec BMP085 Digital Pressure Sensor > > > + * > > > + * Written by: Matthias Brugger <m_brugger@xxxxxx> > > > + * > > > + * Copyright (C) 2010 Fraunhofer Institute for Integrated Circuits > > > + * > > > + * This program is free software; you can redistribute it and/or > modify > > > + * it under the terms of the GNU General Public License as published > by > > > + * the Free Software Foundation; either version 2 of the License, or > > > + * (at your option) any later version. > > > + * > > > + * This program is distributed in the hope that it will be useful, > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > > + * GNU General Public License for more details. > > > + * > > > + * You should have received a copy of the GNU General Public License > > > + * along with this program; if not, write to the > > > + * Free Software Foundation, Inc., > > > + * 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. > > > + */ > > > + > > > +#include <linux/module.h> > > > +#include <linux/init.h> > > > +#include <linux/slab.h> > > > +#include <linux/i2c.h> > > > +#include <linux/mutex.h> > > > +#include <linux/delay.h> > > > +#include <linux/time.h> > > > +#include <linux/mutex.h> > > > + > > > +#include "bmp085.h" > > > + > > > +int oss = 3; > > > +module_param(oss, int , S_IRUSR); > > > +MODULE_PARM_DESC(oss, "Oversampling setting [0-3]"); > > Not convinced this should be a module parameter. Better to use a > > combination > > of board config (for default on a given board) and appropriate sysfs > > attribute > > to allow it to be changed. Still, fine for initial merge, we can change > > this > > later. > > > + > > > > > > +/************************************************************************ > > ** > > > + * Calcualtion of temperature and pressure > > Typo Calcualtion -> Calculation. > > > + > > > ************************************************************************** > > / > > > +static short bmp085_calc_temperature(struct i2c_client *client, > > > + unsigned long ut) > > > +{ > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + long x1, x2; > > > + short temp; > > > + > > > + x1 = ((long) ut - (long) data->ac6) * (long) data->ac5 >> 15; > > > + x2 = ((long) data->mc << 11) / (x1 + data->md); > > > + data->b5 = x1 + x2; > > > + temp = ((data->b5 + 8) >> 4); > > > + > > > + return temp; > > > +} > > > + > > > +static long bmp085_calc_pressure(struct i2c_client *client, unsigned > > long up) > > > +{ > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + long b6, x1, x2, x3, b3; > > > + unsigned long b4, b7; > > > + long pressure, p; > > > + long tmp; > > > + > > > + b6 = data->b5 - 4000; > > > + x1 = (data->b2 * (b6 * b6 / (1<<12))) / (1<<11); > > > + x2 = data->ac2 * b6 / (1<<11); > > > + x3 = x1 + x2; > > > + b3 = (((data->ac1 * 4 + x3) << data->oss) + 2) / 4; > > > + > > > + x1 = data->ac3 * b6 / (1<<13); > > > + x2 = (data->b1 * ((b6 * b6) / (1<<12))) / (1<<16); > > > + x3 = ((x1 + x2) + 2) / (1<<2); > > > + b4 = data->ac4 * (unsigned long) (x3 + 32768) / (1<<15); > > > + b7 = ((unsigned long)up - b3) * (50000 >> data->oss); > > > + > > > + if (b7 < 0x80000000) > > > + p = (b7 * 2) / b4; > > > + else > > > + p = (b7 / b4) * 2; > > > + tmp = (p / (1<<8)) * (p / (1<<8)); > > > + x1 = (tmp * 3038) / (1<<16); > > > + x2 = (-7357 * p) / (1<<16); > > > + pressure = p + ((x1 + x2 + 3791) / (1<<4)); > > > + > > > + return pressure; > > > +} > > > + > > > > > > +/************************************************************************ > > ** > > > + * Digital interface to sensor > > > + > > > ************************************************************************** > > / > > > + > > This is an extremely slim function. I think it would be easier to > remove > > it and > > just ahve the i2c_block_data function inline in the code. > > > +static ssize_t bmp085_read(struct i2c_client *client, u8 reg, size_t > > count, > > > + unsigned char *buffer) > > > +{ > > > + int rc; > > > + rc = i2c_smbus_read_i2c_block_data(client, reg, count, buffer); > > > + if (rc < 0) > > No. Pass the error that came out of the i2c command on. Never eat an > > error as you just reduce the information available. > > > + return -EIO; > > > + > > > + return count; > > > +} > > > + > > > +static short bmp085_read_temp(struct i2c_client *client) > > > +{ > > > + s32 ret; > > > + short temp; > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + > > > + mutex_lock(&data->bmp085_lock); > > > + ret = i2c_smbus_write_byte_data(client, BMP085_START_CONV, > > > + BMP085_START_TEMP); > > > + mutex_unlock(&data->bmp085_lock); > > > + if (ret < 0) { > > > + dev_warn(&client->dev, "starting temperature conversation " > > > + "failed\n"); > > > + return ret; > > > + } > > > + > > probably msleep. > > > + mdelay(5); > > > + ret = bmp085_read(client, BMP085_REG_CONV, 2, data->data); > > > + if (ret < 0) { > > > + dev_warn(&client->dev, "reading ut failed, value is %#x\n" > > > + , ret); > > > + return ret; > > > + } > > > + > > > + data->ut = (data->data[0] << 8) | data->data[1]; > > > + > > > + temp = bmp085_calc_temperature(client, data->ut); > > > + > > > + return temp; > > > +} > > > + > > > +static long bmp085_read_pressure(struct i2c_client *client) > > > +{ > > > + unsigned long up = 0; > > > + int ret; > > > + u8 xlsb, ret1, ret2; > > > + long pressure; > > > + u8 reg; > > Don't worry about the todo. Overshooting on this isn't going > > to greatly change your possible read rate. > > > + /* TODO should be 4.5, 7.5, 13.5, 25.5 ms */ > > > + int time_delay[4] = {5, 8, 14, 26}; > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + > > > + if (data->oss == 0) > > > + reg = BMP085_START_PRESS_OSRS0; > > > + else if (data->oss == 1) > > > + reg = BMP085_START_PRESS_OSRS1; > > > + else if (data->oss == 2) > > > + reg = BMP085_START_PRESS_OSRS2; > > > + else if (data->oss == 3) > > > + reg = BMP085_START_PRESS_OSRS3; > > > + else { > > > + dev_warn(&client->dev, "undefined oss value, use oss = 0\n"); > > > + data->oss = 0; > > > + reg = BMP085_START_PRESS_OSRS0; > > > + } > > > + > > > + mutex_lock(&data->bmp085_lock); > > > + ret = i2c_smbus_write_byte_data(client, BMP085_START_CONV, reg); > > > + mutex_unlock(&data->bmp085_lock); > > > + if (ret < 0) > > > + return ret; > > > + > > Does this need to be that precise? If not an msleep would be a better > > bet to allow the system to get on with something else. > > > + mdelay(time_delay[data->oss]); > > > + > > > + mutex_lock(&data->bmp085_lock); > > > + ret1 = i2c_smbus_read_byte_data(client, 0xf6); > > > + ret2 = i2c_smbus_read_byte_data(client, 0xf7); > > > + xlsb = i2c_smbus_read_byte_data(client, 0xf8); > > > + mutex_unlock(&data->bmp085_lock); > > > + > > > + up = (((unsigned long) ret1 << 16) | ((unsigned long) ret2 << 8) > > > + | (unsigned long) xlsb) >> (8 - data->oss); > > > + data->up = up; > > > + > > > + pressure = bmp085_calc_pressure(client, up); > > Why cache the pressure? > > > + data->pressure = pressure; > > > + > > > + return pressure; > > > +} > > > + > > > > > > +/************************************************************************ > > ** > > > + * sysfs attributes > > > + > > > ************************************************************************** > > / > > > +static ssize_t barometer_show_temp(struct device *dev, > > > + struct device_attribute *da, char *buf) > > > +{ > > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > > + struct bmp085_data *data = indio_dev->dev_data; > > > + struct i2c_client *client = data->client; > > > + long status; > > > + > > > + status = bmp085_read_temp(client); > > > + if (status < 0) > > > + dev_warn(&client->dev, "error reading temperature: %ld\n", > > > + status); > > > + > > > + data->temp = status; > > > + > > > + return sprintf(buf, "%ld\n", data->temp); > > > +} > > > +static IIO_DEV_ATTR_TEMP_RAW(barometer_show_temp); > > > + > > > +/** > > > + * In standard mode, the temperature has to be reat every second > before > > the > > > + * pressure can be reat. We leave this semantics to the userspace, if > > later > > past tense of read is infact read not reat. > > > + * on a trigger based reading will be implemented, this should be > taken > > into > > > + * account. > > Ouch, that's an uggly requirement. > > > + */ > > > +static ssize_t barometer_show_pressure(struct device *dev, > > > + struct device_attribute *da, char *buf) > > > +{ > > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > > + struct bmp085_data *data = indio_dev->dev_data; > > > + struct i2c_client *client = data->client; > > > + long status; > > > + > > > + status = bmp085_read_pressure(client); > > > + if (status < 0) > > > + dev_warn(&client->dev, "error reading pressure: %ld\n", > > status); > > > + > > Why cache the preesure? > > > + data->pressure = status; > > > + > > > + return sprintf(buf, "%ld\n", data->pressure); > > > +} > > > +static IIO_DEV_ATTR_BARO_PRESSURE(S_IRUGO, barometer_show_pressure, > > NULL, 0); > > > + > > > +static ssize_t barometer_show_id_version(struct device *dev, > > > + struct device_attribute *da, char *buf) > > > +{ > > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > > + struct bmp085_data *data = indio_dev->dev_data; > > Doesn't really matter, but we do have a helper function for this > > (and at somepoint I'll clean all the direct reads out of the tree) > > struct bmp085_data *data = iio_dev_get_devdata(indio_dev); > > > + > > > + return sprintf(buf, "%x_%x\n", data->chip_id, data->chip_version); > > > +} > > > +static IIO_DEV_ATTR_REV(barometer_show_id_version); > > > + > > > +static ssize_t barometer_show_oss(struct device *dev, > > > + struct device_attribute *da, char *buf) > > > +{ > > > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > > > + struct bmp085_data *data = indio_dev->dev_data; > > > + > > > + return sprintf(buf, "%d\n", data->oss); > > > +} > > So is oss (which is sampling frequency) either 1Hz or 2Hz? > > (I haven't looked at datasheet but under the abi that's what this is > > giving us). > > > +static IIO_DEV_ATTR_SAMP_FREQ(S_IRUGO, barometer_show_oss, NULL); > > > + > > > +static IIO_CONST_ATTR_TEMP_SCALE("0.1"); > > No real problem with this. Personally I'd probably just do > > the trivial amount of fixed point arithmetic in the temperature > > output. > > > > sprintf(buf, "%d.%d\n", temp/10, temp%10); > > > + > > > +static struct attribute *bmp085_attributes[] = { > > > + &iio_dev_attr_temp_raw.dev_attr.attr, > > > + &iio_dev_attr_baro_pressure.dev_attr.attr, > > > + &iio_dev_attr_revision.dev_attr.attr, > > > + &iio_dev_attr_sampling_frequency.dev_attr.attr, > > > + &iio_const_attr_temp_scale.dev_attr.attr, > > > + NULL > > > +}; > > > + > > > +static const struct attribute_group bmp085_attr_group = { > > > + .attrs = bmp085_attributes, > > > +}; > > > + > > > > > > +/************************************************************************ > > ** > > > + * Calibration data processing > > > + > > > ************************************************************************** > > / > > > + > > > +static int bmp085_init_client(struct i2c_client *client) > > > +{ > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + int i; > > > + > > > + i = bmp085_read(client, BMP085_REG_CHIP_ID, 1, &data->chip_id); > > > + if (i < 0) > > > + dev_warn(&client->dev, "Chip ID not read\n"); > > > + > > > + i = bmp085_read(client, BMP085_REG_VERSION, 1, &data->chip_version); > > > + if (i < 0) > > > + dev_warn(&client->dev, "Version not read\n"); > > > + > > > + i = bmp085_read(client, BMP085_REG_PROM, BMP085_PROM_LENGTH, > > > + data->data); > > > + if (i < 0) > > > + dev_warn(&client->dev, "Unable to read %d bytes from address " > > > + "%#x\n", BMP085_PROM_LENGTH, BMP085_REG_PROM); > > > + > > > + data->ac1 = (data->data[0] << 8) | data->data[1]; > > > + data->ac2 = (data->data[2] << 8) | data->data[3]; > > > + data->ac3 = (data->data[4] << 8) | data->data[5]; > > > + data->ac4 = (data->data[6] << 8) | data->data[7]; > > > + data->ac5 = (data->data[8] << 8) | data->data[9]; > > > + data->ac6 = (data->data[10] << 8) | data->data[11]; > > > + data->b1 = (data->data[12] << 8) | data->data[13]; > > > + data->b2 = (data->data[14] << 8) | data->data[15]; > > > + data->mb = (data->data[16] << 8) | data->data[17]; > > > + data->mc = (data->data[18] << 8) | data->data[19]; > > > + data->md = (data->data[20] << 8) | data->data[21]; > > > + > > > + return 0; > > > +} > > > + > > > +static struct i2c_driver bmp085_drv; > > Why is this here? > > > > > +static int bmp085_probe(struct i2c_client *client, > > > + const struct i2c_device_id *id) > > > +{ > > > + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); > > > + struct bmp085_data *data; > > > + struct bmp085_data *data2; > > > + int status = 0; > > > + > > > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA | > > > + I2C_FUNC_SMBUS_WORD_DATA)) > > > + return -EIO; > > > + > > > + /* Allocate driver data */ > > > + data = kzalloc(sizeof(struct bmp085_data), GFP_KERNEL); > > > + if (!data) > > > + return -ENOMEM; > > > + > > > + if ((oss < 0) || (oss > 3)) { > > > + status = -EINVAL; > > > + goto err; > > > + } > > > + data->oss = oss; > > > + > > > + data->client = client; > > > + i2c_set_clientdata(client, data); > > > + data2 = i2c_get_clientdata(client); > > Guessing the above is dead debug code? > > > > > + > > > + /* Initialize the BMP085 chip */ > > > + bmp085_init_client(client); > > > + > > > + __mutex_init(&data->bmp085_lock, "bmp085_lock", NULL); > > Why the underscore form? I've not seen this before. > > > + > > > + /* Register with IIO */ > > > + data->indio_dev = iio_allocate_device(); > > > + if (data->indio_dev == NULL) { > > > + status = -ENOMEM; > > > + goto err; > > > + } > > > + > > > + data->indio_dev->dev.parent = &client->dev; > > > + data->indio_dev->attrs = &bmp085_attr_group; > > > + data->indio_dev->dev_data = (void *)(data); > > > + data->indio_dev->driver_module = THIS_MODULE; > > > + data->indio_dev->modes = INDIO_DIRECT_MODE; > > > + > > > + status = iio_device_register(data->indio_dev); > > > + if (status < 0) > > > + goto err_iio; > > > + > > Remove this as it's not needed. > > > + dev_info(&client->dev, "driver enabled\n"); > > > + > > > + return 0; > > > + > > > +err_iio: > > > + kfree(data->indio_dev); > > > +err: > > > + kfree(data); > > > + return status; > > > +} > > > + > > > +static int __devexit bmp085_remove(struct i2c_client *client) > > > +{ > > > + struct bmp085_data *data = i2c_get_clientdata(client); > > > + > > > + if (mutex_is_locked(&data->bmp085_lock)) > > > + mutex_unlock(&data->bmp085_lock); > > Something weird is occuring if you can get here without the lock > > being unlocked. > > > + > > > + iio_device_unregister(data->indio_dev); > > > + iio_free_device(data->indio_dev); > > > + > > > + kfree(data); > > > + > > > + dev_info(&client->dev, "driver removed\n"); > > > + > > > + return 0; > > > +} > > > + > > > +static const struct i2c_device_id bmp085_id[] = { > > > + { "bmp085", 0 }, > > > + {} > > > +}; > > > +MODULE_DEVICE_TABLE(i2c, bmp085_id); > > > + > > > +static struct i2c_driver bmp085_drv = { > > > + .driver = { > > > + .name = "bmp085", > > > + .owner = THIS_MODULE, > > > + }, > > > + .suspend = NULL, > > > + .resume = NULL, > > > + .probe = bmp085_probe, > > > + .remove = __devexit_p(bmp085_remove), > > > + .id_table = bmp085_id, > > > +}; > > > + > > > +/*------------------------------------------------------------------- > -- > > ----*/ > > > + > > > +static int __init bmp085_init_module(void) > > > +{ > > Get rid of tis debug. > > > + printk(KERN_INFO"bmp085 loading...\n"); > > > + > > > + return i2c_add_driver(&bmp085_drv); > > > +} > > > + > > > +static void __exit bmp085_exit_module(void) > > > +{ > > > + i2c_del_driver(&bmp085_drv); > > > +} > > > + > > > +MODULE_AUTHOR("Matthias Brugger > <matthias.brugger@xxxxxxxxxxxxxxxxx>"); > > > +MODULE_DESCRIPTION("I2c device driver for BMP085 barometererator > > sensor"); > > > +MODULE_LICENSE("GPL"); > > > + > > > +module_init(bmp085_init_module); > > > +module_exit(bmp085_exit_module); > > > diff --git a/drivers/staging/iio/barometer/bmp085.h > > b/drivers/staging/iio/barometer/bmp085.h > > > new file mode 100644 > > > index 0000000..5ed2fb1 > > > --- /dev/null > > > +++ b/drivers/staging/iio/barometer/bmp085.h > > > @@ -0,0 +1,73 @@ > > > +#ifndef BMP085_H > > > +#define BMP085_H > > > + > > > +#include "../iio.h" > > > +#include "baro.h" > > > + > > > +#define BMP085_REG_CONV 0xF6 /* Temperature/pressure value UT > > or UP */ > > > +#define BMP085_REG_CONV_XLS 0xF8 > > > + > > > +#define BMP085_REG_PROM 0xAA > > > +#define BMP085_PROM_LENGTH 22 > > > + > > > +#define BMP085_REG_AC1 0xAA > > > +#define BMP085_REG_AC2 0xAC > > > +#define BMP085_REG_AC3 0xAE > > > +#define BMP085_REG_AC4 0xB0 > > > +#define BMP085_REG_AC5 0xB2 > > > +#define BMP085_REG_AC6 0xB4 > > > +#define BMP085_REG_B1 0xB6 > > > +#define BMP085_REG_B2 0xB8 > > > +#define BMP085_REG_MB 0xBA > > > +#define BMP085_REG_MC 0xBC > > > +#define BMP085_REG_MD 0xBE > > > + > > > +#define BMP085_START_CONV 0xF4 > > > + > > > +#define BMP085_START_TEMP 0x2E /* wait 4.5 ms */ > > > + > > > +#define BMP085_START_PRESS_OSRS0 0x34 /* wait 4.5 ms */ > > > +#define BMP085_START_PRESS_OSRS1 0x74 /* wait 7.5 ms */ > > > +#define BMP085_START_PRESS_OSRS2 0xB4 /* wait 13.5 ms */ > > > +#define BMP085_START_PRESS_OSRS3 0xF4 /* wait 25.5 ms */ > > > + > > > +#define BMP085_REG_CHIP_ID 0xD0 > > > +#define BMP085_REG_VERSION 0xD1 > > > +#define BMP085_CHIP_ID 0x55 > > > + > > Ideally this structure would have some documentation (kernel doc) > > to make reviewing the driver easier. > > > > > +struct bmp085_data { > > > + struct i2c_client *client; > > > + struct iio_dev *indio_dev; > > > + > > > + struct mutex bmp085_lock; > > > + > > > + int oss; > > > + long temp; > > Why long? Looking at the code this always looks to be > > 16 bit to me, so s16 would be more appropraite. > > > > > + long pressure; > > > + > > > + short ac1; > > > + short ac2; > > > + short ac3; > > > + unsigned short ac4; > > > + unsigned short ac5; > > > + unsigned short ac6; > > > + > > > + short b1; > > > + short b2; > > > + long b5; > > > + > > > + short mb; > > > + short mc; > > > + short md; > > > + > > > + unsigned long ut; > > > + unsigned long up; > > > + > > > + u8 chip_id; > > > + u8 chip_version; > > > + > > > + unsigned char data[22]; > > > +}; > > > + > > > + > > > +#endif > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
