On 24/06/15 17:17, ludovic.tancerel@xxxxxxxxxxxxxxxxx wrote: > Hi Jonathan, Thomasz > > thank you for your feedback and recommendations. > > I am willing to do things as clean as possible in a timeframe that I can sustain myself. > I will try to give more details on what is being done, and the proposal to comply to your request. > Please read below. Thank you for reviewing and commenting. > > I use this channel to ask a few questions on upcoming drivers as well. > > Regards, > Ludovic > > Le 21 juin 2015 à 15:36, Jonathan Cameron <jic23@xxxxxxxxxx> a écrit : > >> On 17/06/15 08:08, ludovic.tancerel@xxxxxxxxxxxxxxxxx wrote: >>> Hello Tomasz, >>> >>> Thank you for notifying me and your feedback, there is a wish from >>> Measurement Specialties to have a separate driver for MS5637 and >>> MS5611. There are a few differences in HW, moreover the driver you >>> pushed does not support resolution change. >> Hmm. If I get a proposal from someone to actually do the work >> to unify the resulting drivers without any loss of functionality, I >> will be in favour. >> >> If there is any chance of pushing back on this if there is a strong >> case for a more unified driver then please do what you can. I >> appreciate the constraints that can get applied from above! > > I am willing to commonalize as well as much as possible. However, > there is different need, and request coming from Measurement > Specialties, I have to fulfill their requirements. The approach I > prefer is to have different files for different HW, and common > functions that would fall into common/meas_sensors/. That can work well. Just depends on the particular parts to whether it makes sense to do it that way or big multipart drivers. > > My main concern is about SPI vs I2C. > I did not plan to develop the drivers supporting both SPI and I2C. I developed just for I2C support. > The drivers for below HW are all ready, I just have to apply the rework for ms5637 that is applicable to them. Cool. One option to make life easier for whoever might one day implement spi support would be to use regmap for all of them. The cost is low even if you only do i2c support, but then the cost to add SPI is a matter of a few lines. One of the biggest reason regmap exists is because lots of parts support both spi and i2c (does lots of other cool stuff as well!) > >>> >>> Note additional drivers from Measurement Specialties will be added >>> soon. >> If the parts are also similar in function and could be covered by >> a single driver I might well start pushing back harder. In the long >> run, lots of duplicated code is bad for maintenance so I get grumpy >> fairly quickly and very happy when I get big patches that unify >> lots of drivers (as long as they aren’t too complex!) > > I will commonalize functions whereas it can be, but will still have separate drivers. > The upcoming drivers are for following parts: > TSYS01 (Temperature) > TSYS02D (Temperature) > HTU21D (Temperature & Humidity) > MS8607 (Temperature, Pressure & Humidity) > KMA36 (magnetic encoder) > Depending on the sensors, there is possible reuse between them. They > do not all share same protocol. That means there should be at least 3 > patches. > > Regarding Humidity, there is one thing that is typical on humidity > sensor, it is heater function. I don’t see anything similar in > existing drivers (It is marked as TODO for si7005) Is this ok to have > an attribute that enables the heater ? I presume that falls within > private-ABI area, unless a new channel shall be added to control > this. It's not really a general purpose channel so probably makes sense to have it as private ABI. Note there are a couple of humidity sensors in HWMON - it's always been an unclear question wrt to where they should go. It would be good to share ABI with them where possible (looks like it is called heater_enable in sht15 for example). > Last thing I want to ask is about kma36. I prefer asking before going further > This chipset is an angle magnetical encoder. There is no existing IIO > driver of that kind for now as far as I can see. My original intent > was to create a new directory (me ? magnetic_encoder ? mag_encoder ?) encoder would probably work best. > and reuse the IIO_ANGL channel that is originally meant for gyros. > Does that sound proper to you ? Sounds fine. We do have quite a few resolver drivers that are still in staging (for a reason!) which are probably similar is some ways. > >> >> Jonathan >>> >>> I will include your comments in corrections. >>> Thank you, >>> Ludovic >>> >>> >>> Le 16 juin 2015 à 22:42, Tomasz Duszynski <tduszyns@xxxxxxxxx> a écrit : >>> >>>> On Tue, Jun 16, 2015 at 02:33:29PM +0200, Ludovic wrote: >>>>> MS5637 spec : http://www.meas-spec.com/product/pressure/MS5637-02BA03.aspx >>>>> >>>>> This patch has already been submitted some time ago by William Markezana, it was originally developped by me. >>>>> Thank you for all the comments done. Hopefully, everything is ok now. >>>>> >>>>> Thank you in advance for reviewing again. >>>>> >>>> Hi Ludovic, >>>> >>>> Not so long ago I added support for ms5611. Comparing both chips datasheets >>>> didn't reveal much difference, so maybe two patches can somehow be combined to >>>> avoid code duplication. >>>> >>>> A few additional comments below. >>>>> Regards, >>>>> Ludovic >>>>> >>>>> Signed-off-by: Ludovic <ludovic.tancerel@xxxxxxxxxxxxxxxxx> >>>>> --- >>>>> drivers/iio/pressure/Kconfig | 10 + >>>>> drivers/iio/pressure/Makefile | 1 + >>>>> drivers/iio/pressure/ms5637.c | 475 ++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 486 insertions(+) >>>>> create mode 100644 drivers/iio/pressure/ms5637.c >>>>> >>>>> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig >>>>> index a3be537..6c7faa1 100644 >>>>> --- a/drivers/iio/pressure/Kconfig >>>>> +++ b/drivers/iio/pressure/Kconfig >>>>> @@ -52,6 +52,16 @@ config MPL3115 >>>>> To compile this driver as a module, choose M here: the module >>>>> will be called mpl3115. >>>>> >>>>> +config MS5637 >>>>> + tristate "MS5637 pressure & temperature sensor" >>>>> + depends on I2C >>>>> + help >>>>> + If you say yes here you get support for the Measurement Specialties >>>>> + MS5637 pressure and temperature sensor. >>>>> + >>>>> + This driver can also be built as a module. If so, the module will >>>>> + be called ms5637. >>>>> + >>>>> config IIO_ST_PRESS >>>>> tristate "STMicroelectronics pressure sensor Driver" >>>>> depends on (I2C || SPI_MASTER) && SYSFS >>>>> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile >>>>> index 88011f2..2455f8e 100644 >>>>> --- a/drivers/iio/pressure/Makefile >>>>> +++ b/drivers/iio/pressure/Makefile >>>>> @@ -7,6 +7,7 @@ obj-$(CONFIG_BMP280) += bmp280.o >>>>> obj-$(CONFIG_HID_SENSOR_PRESS) += hid-sensor-press.o >>>>> obj-$(CONFIG_MPL115) += mpl115.o >>>>> obj-$(CONFIG_MPL3115) += mpl3115.o >>>>> +obj-$(CONFIG_MS5637) += ms5637.o >>>>> obj-$(CONFIG_IIO_ST_PRESS) += st_pressure.o >>>>> st_pressure-y := st_pressure_core.o >>>>> st_pressure-$(CONFIG_IIO_BUFFER) += st_pressure_buffer.o >>>>> diff --git a/drivers/iio/pressure/ms5637.c b/drivers/iio/pressure/ms5637.c >>>>> new file mode 100644 >>>>> index 0000000..9c647d7 >>>>> --- /dev/null >>>>> +++ b/drivers/iio/pressure/ms5637.c >>>>> @@ -0,0 +1,475 @@ >>>>> +/* >>>>> + * ms5637.c - Support for Measurement-Specialties ms5637 >>>>> + * pressure & temperature sensor >>>>> + * >>>>> + * Copyright (c) 2014 Measurement-Specialties >>>>> + * >>>>> + * Licensed under the GPL-2. >>>>> + * >>>>> + * (7-bit I2C slave address 0x76) >>>>> + * >>>>> + */ >>>>> +#include <linux/init.h> >>>>> +#include <linux/device.h> >>>>> +#include <linux/kernel.h> >>>>> +#include <linux/stat.h> >>>>> +#include <linux/module.h> >>>>> +#include <linux/i2c.h> >>>>> +#include <linux/iio/iio.h> >>>>> +#include <linux/iio/sysfs.h> >>>>> +#include <linux/jiffies.h> >>>>> +#include <linux/delay.h> >>>>> +#include <linux/mutex.h> >>>>> + >>>>> +/* MS5637 Commands */ >>>>> +#define MS5637_RESET 0x1E >>>>> +#define MS5637_TEMPERATURE_CONVERSION_START 0x50 >>>>> +#define MS5637_PRESSURE_CONVERSION_START 0x40 >>>>> +#define MS5637_ADC_READ 0x00 >>>>> +#define MS5637_PROM_READ 0xA0 >>>>> +#define MS5637_PROM_ELEMENTS_NB 7 >>>>> + >>>>> +static const u16 ms5637_conversion_time[] = { 1000, 2000, 3000, >>>>> + 5000, 9000, 17000 }; >>>>> +static const int ms5637_samp_freq[6][2] = { {1800, 0}, {900, 0}, >>>>> + {450, 0}, {225, 0}, >>>>> + {112, 500000}, {56, 250000}, >>>>> + }; >>>>> + >>>>> +struct ms5637_dev { >>>>> + struct i2c_client *client; >>>>> + struct mutex lock; /* mutex protecting this data structure */ >>>>> + u16 calibration_coeffs[MS5637_PROM_ELEMENTS_NB]; >>>>> + bool got_calibration_words; >>>>> + unsigned long last_update; >>>>> + bool valid; >>>>> + int temperature; >>>>> + unsigned int pressure; >>>>> + u8 resolution_index; >>>>> +}; >>>>> + >>>>> +static int ms5637_get_calibration_coeffs(struct ms5637_dev *dev_data, >>>>> + unsigned char index, u16 *word) >>>>> +{ >>>>> + int ret = 0; >>>>> + >>>>> + ret = i2c_smbus_read_word_swapped(dev_data->client, >>>>> + MS5637_PROM_READ + (index << 1)); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + *word = (u16)ret & 0xFFFF; >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static bool ms5637_crc_check(u16 *n_prom, u8 crc) >>>>> +{ >>>>> + unsigned int cnt, n_bit; >>>>> + u16 n_rem, crc_read; >>>>> + >>>>> + n_rem = 0x0000; >>>>> + crc_read = n_prom[0]; >>>>> + n_prom[MS5637_PROM_ELEMENTS_NB] = 0; >>>>> + n_prom[0] &= 0x0FFF; /* Clear the CRC byte */ >>>>> + >>>>> + for (cnt = 0; cnt < (MS5637_PROM_ELEMENTS_NB + 1) * 2; cnt++) { >>>>> + if (cnt % 2 == 1) >>>>> + n_rem ^= n_prom[cnt >> 1] & 0x00FF; >>>>> + else >>>>> + n_rem ^= n_prom[cnt >> 1] >> 8; >>>>> + >>>>> + for (n_bit = 8; n_bit > 0; n_bit--) { >>>>> + if (n_rem & 0x8000) >>>>> + n_rem = (n_rem << 1) ^ 0x3000; >>>>> + else >>>>> + n_rem <<= 1; >>>>> + } >>>>> + } >>>>> + n_rem >>= 12; >>>>> + n_prom[0] = crc_read; >>>>> + return (n_rem == crc); >>>>> +} >>>>> + >>>>> +static int ms5637_fill_calibration_coeffs(struct ms5637_dev *dev_data) >>>>> +{ >>>>> + int i, ret = 0; >>>>> + >>>>> + for (i = 0; i < MS5637_PROM_ELEMENTS_NB; i++) { >>>>> + ret = ms5637_get_calibration_coeffs( >>>>> + dev_data, i, >>>>> + &dev_data->calibration_coeffs[i]); >>>>> + >>>>> + if (ret < 0) { >>>>> + dev_err(&dev_data->client->dev, >>>>> + "Unable to get calibration coefficients at address %d\n", >>>>> + i + 1); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + dev_dbg(&dev_data->client->dev, "Coeff %d : %d", i, >>>>> + dev_data->calibration_coeffs[i]); >>>>> + } >>>>> + >>>>> + ret = ms5637_crc_check( >>>>> + dev_data->calibration_coeffs, >>>>> + (dev_data->calibration_coeffs[0] & 0xF000) >> 12); >>>>> + if (ret == 0) >>>>> + dev_err(&dev_data->client->dev, >>>>> + "Calibration coefficients crc check error\n"); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ms5637_read_adc_value(struct ms5637_dev *dev_data, u32 *adc_value) >>>>> +{ >>>>> + int ret; >>>>> + u8 buf[3]; >>>>> + >>>>> + ret = >>>>> + i2c_smbus_read_i2c_block_data(dev_data->client, MS5637_ADC_READ, 3, >>>>> + buf); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + dev_dbg(&dev_data->client->dev, "ADC raw value : %x %x %x\n", buf[0], >>>>> + buf[1], buf[2]); >>>>> + *adc_value = (buf[0] << 16) + (buf[1] << 8) + buf[2]; >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ms5637_conversion_and_read_adc(struct ms5637_dev *dev_data, >>>>> + u32 *t_adc, u32 *p_adc) >>>>> +{ >>>>> + int ret; >>>>> + >>>>> + dev_dbg(&dev_data->client->dev, "ms5637_conversion_and_read_adc"); >>>>> + /* Trigger Temperature conversion */ >>>>> + ret = i2c_smbus_write_byte(dev_data->client, >>>>> + MS5637_TEMPERATURE_CONVERSION_START >>>>> + + dev_data->resolution_index * 2); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + usleep_range(ms5637_conversion_time[dev_data->resolution_index], >>>>> + ms5637_conversion_time[dev_data->resolution_index] + 3000); >>>>> + /* Retrieve ADC value */ >>>>> + ret = ms5637_read_adc_value(dev_data, t_adc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + /* Trigger Pressure conversion */ >>>>> + ret = i2c_smbus_write_byte(dev_data->client, >>>>> + MS5637_PRESSURE_CONVERSION_START >>>>> + + dev_data->resolution_index * 2); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + usleep_range(ms5637_conversion_time[dev_data->resolution_index], >>>>> + ms5637_conversion_time[dev_data->resolution_index] + 3000); >>>>> + /* Retrieve ADC value */ >>>>> + ret = ms5637_read_adc_value(dev_data, p_adc); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ms5637_read_temperature_and_pressure(struct ms5637_dev *dev_data) >>>>> +{ >>>>> + int ret = 0; >>>>> + u32 t_adc, p_adc; >>>>> + s32 dt, temp; >>>>> + s64 off, sens, t2, off2, sens2; >>>>> + >>>>> + mutex_lock(&dev_data->lock); >>>>> + >>>>> + if (time_after(jiffies, dev_data->last_update + HZ / 2) || >>>>> + !dev_data->valid) { >>>>> + if (!dev_data->got_calibration_words) { >>>>> + ret = ms5637_fill_calibration_coeffs(dev_data); >>>>> + if (ret != 0) >>>>> + goto out; >>>>> + >>>>> + dev_data->got_calibration_words = true; >>>>> + } >>>>> + >>>>> + ret = ms5637_conversion_and_read_adc(dev_data, &t_adc, &p_adc); >>>>> + if (ret < 0) { >>>>> + dev_data->got_calibration_words = false; >>>>> + dev_err(&dev_data->client->dev, >>>>> + "Unable to make sensor adc conversion\n"); >>>>> + goto out; >>>>> + } >>>>> + >>>>> + dt = (s32)t_adc - (dev_data->calibration_coeffs[5] << 8); >>>>> + >>>>> + /* Actual temperature = 2000 + dT * TEMPSENS */ >>>>> + temp = >>>>> + 2000 + (((s64)dt * dev_data->calibration_coeffs[6]) >> 23); >>>>> + >>>>> + /* Second order temperature compensation */ >>>>> + if (temp < 2000) { >>>>> + s64 tmp = (s64)temp - 2000; >>>>> + >>>>> + t2 = (3 * ((s64)dt * (s64)dt)) >> 33; >>>>> + off2 = (61 * tmp * tmp) >> 4; >>>>> + sens2 = (29 * tmp * tmp) >> 4; >>>>> + >>>>> + if (temp < -1500) { >>>>> + s64 tmp = (s64)temp + 1500; >>>>> + >>>>> + off2 += 17 * tmp * tmp; >>>>> + sens2 += 9 * tmp * tmp; >>>>> + } >>>>> + } else { >>>>> + t2 = (5 * ((s64)dt * (s64)dt)) >> 38; >>>>> + off2 = 0; >>>>> + sens2 = 0; >>>>> + } >>>>> + >>>>> + /* OFF = OFF_T1 + TCO * dT */ >>>>> + off = (((s64)dev_data->calibration_coeffs[2]) << 17) + >>>>> + ((((s64)dev_data->calibration_coeffs[4]) * (s64)dt) >> 6); >>>>> + off -= off2; >>>>> + >>>>> + /* Sensitivity at actual temperature = SENS_T1 + TCS * dT */ >>>>> + sens = (((s64)dev_data->calibration_coeffs[1]) << 16) >>>>> + + (((s64)dev_data->calibration_coeffs[3] * dt) >> 7); >>>>> + sens -= sens2; >>>>> + >>>>> + /* Temperature compensated pressure = D1 * SENS - OFF */ >>>>> + dev_data->temperature = (temp - t2) * 10; >>>>> + dev_data->pressure = (u32)(((((s64)p_adc * sens) >> 21) >>>>> + - off) >> 15); >>>>> + >>>>> + dev_data->last_update = jiffies; >>>>> + dev_data->valid = true; >>>>> + } >>>>> + out: >>>>> + mutex_unlock(&dev_data->lock); >>>>> + >>>>> + return ret >= 0 ? 0 : ret; >>>>> +} >>>>> + >>>>> +static int ms5637_get_int_plus_micros_index(const int (*vals)[2], int n, >>>>> + int val, int val2) >>>>> +{ >>>>> + while (n-- > 0) >>>>> + if (val == vals[n][0] && val2 == vals[n][1]) >>>>> + return n; >>>>> + return -EINVAL; >>>>> +} >>>>> + >>>>> +static int ms5637_get_samp_freq_index(struct ms5637_dev *dev_data, >>>>> + int val, int val2) >>>>> +{ >>>>> + return ms5637_get_int_plus_micros_index(ms5637_samp_freq, >>>>> + ARRAY_SIZE(ms5637_samp_freq), val, val2); >>>>> +} >>>>> + >>>>> +static ssize_t ms5637_show_int_plus_micros(char *buf, >>>>> + const int (*vals)[2], int n) >>>>> +{ >>>>> + size_t len = 0; >>>>> + >>>>> + while (n-- > 0) >>>>> + len += scnprintf(buf + len, PAGE_SIZE - len, >>>>> + "%d.%06d ", vals[n][0], vals[n][1]); >>>>> + >>>>> + /* replace trailing space by newline */ >>>>> + buf[len - 1] = '\n'; >>>>> + >>>>> + return len; >>>>> +} >>>>> + >>>>> +static ssize_t ms5637_show_samp_freq_avail(struct device *dev, >>>>> + struct device_attribute *attr, >>>>> + char *buf) >>>>> +{ >>>>> + return ms5637_show_int_plus_micros(buf, ms5637_samp_freq, >>>>> + ARRAY_SIZE(ms5637_samp_freq)); >>>>> +} >>>>> + >>>>> +static int ms5637_read_raw(struct iio_dev *indio_dev, >>>>> + struct iio_chan_spec const *channel, int *val, >>>>> + int *val2, long mask) >>>>> +{ >>>>> + int ret; >>>>> + struct ms5637_dev *dev_data = iio_priv(indio_dev); >>>>> + >>>>> + switch (mask) { >>>>> + case IIO_CHAN_INFO_PROCESSED: >>>>> + ret = ms5637_read_temperature_and_pressure(dev_data); >>>>> + if (ret) >>>>> + goto err; >>>>> + switch (channel->type) { >>>>> + case IIO_TEMP: /* in °C */ >>>> probably iio wants milli C >>>>> + *val = dev_data->temperature / 1000; >>>>> + *val2 = (dev_data->temperature - >>>>> + (dev_data->temperature / 1000) * 1000 >>>>> + ) * 1000; >>>> (dev->data->temperature % 1000) * 1000 has same effect but is much shorter >>>>> + break; >>>>> + case IIO_PRESSURE: /* in kPa */ >>>>> + *val = dev_data->pressure / 1000; >>>>> + *val2 = (dev_data->pressure - >>>>> + (dev_data->pressure / 1000) * 1000) * 1000; >>>>> + break; >>>>> + default: >>>>> + return -EINVAL; >>>>> + } >>>>> + break; >>>>> + case IIO_CHAN_INFO_SAMP_FREQ: >>>>> + *val = ms5637_samp_freq[dev_data->resolution_index][0]; >>>>> + *val2 = ms5637_samp_freq[dev_data->resolution_index][1]; >>>>> + break; >>>>> + default: >>>>> + return -EINVAL; >>>>> + } >>>>> + >>>>> + return IIO_VAL_INT_PLUS_MICRO; >>>>> + err: >>>>> + dev_err(&indio_dev->dev, "Device read error\n"); >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static int ms5637_write_raw(struct iio_dev *indio_dev, >>>>> + struct iio_chan_spec const *chan, >>>>> + int val, int val2, long mask) >>>>> +{ >>>>> + struct ms5637_dev *dev_data = iio_priv(indio_dev); >>>>> + int i; >>>>> + >>>>> + switch (mask) { >>>>> + case IIO_CHAN_INFO_SAMP_FREQ: >>>>> + i = ms5637_get_samp_freq_index(dev_data, val, val2); >>>>> + if (i < 0) >>>>> + return -EINVAL; >>>>> + >>>>> + dev_data->resolution_index = i; >>>>> + break; >>>>> + default: >>>>> + return -EINVAL; >>>>> + } >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static const struct iio_chan_spec ms5637_channels[] = { >>>>> + { >>>>> + .type = IIO_TEMP, >>>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), >>>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ), >>>>> + }, >>>>> + { >>>>> + .type = IIO_PRESSURE, >>>>> + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), >>>>> + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ), >>>>> + } >>>>> +}; >>>>> + >>>>> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(ms5637_show_samp_freq_avail); >>>>> + >>>>> +static struct attribute *ms5637_attributes[] = { >>>>> + &iio_dev_attr_sampling_frequency_available.dev_attr.attr, >>>>> + NULL, >>>>> +}; >>>>> + >>>>> +static const struct attribute_group ms5637_attribute_group = { >>>>> + .attrs = ms5637_attributes, >>>>> +}; >>>>> + >>>>> +static const struct iio_info ms5637_info = { >>>>> + .read_raw = ms5637_read_raw, >>>>> + .write_raw = ms5637_write_raw, >>>>> + .attrs = &ms5637_attribute_group, >>>>> + .driver_module = THIS_MODULE, >>>>> +}; >>>>> + >>>>> +static int ms5637_probe(struct i2c_client *client, >>>>> + const struct i2c_device_id *id) >>>>> +{ >>>>> + struct ms5637_dev *dev_data; >>>>> + struct iio_dev *indio_dev; >>>>> + int ret; >>>>> + >>>>> + if (!i2c_check_functionality(client->adapter, >>>>> + I2C_FUNC_SMBUS_READ_WORD_DATA)) { >>>>> + dev_err(&client->dev, >>>>> + "Adapter does not support SMBus read word transactions\n"); >>>>> + return -ENODEV; >>>>> + } >>>>> + >>>>> + if (!i2c_check_functionality(client->adapter, >>>>> + I2C_FUNC_SMBUS_WRITE_BYTE)) { >>>>> + dev_err(&client->dev, >>>>> + "Adapter does not support SMBus write byte transactions\n"); >>>>> + return -ENODEV; >>>>> + } >>>>> + >>>>> + if (!i2c_check_functionality(client->adapter, >>>>> + I2C_FUNC_SMBUS_READ_I2C_BLOCK)) { >>>>> + dev_err(&client->dev, >>>>> + "Adapter does not support SMBus read block transactions\n"); >>>>> + return -ENODEV; >>>>> + } >>>>> + >>>>> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*dev_data)); >>>>> + if (!indio_dev) >>>>> + return -ENOMEM; >>>>> + >>>>> + dev_data = iio_priv(indio_dev); >>>>> + dev_data->client = client; >>>>> + mutex_init(&dev_data->lock); >>>>> + >>>>> + indio_dev->info = &ms5637_info; >>>>> + indio_dev->name = id->name; >>>>> + indio_dev->dev.parent = &client->dev; >>>>> + indio_dev->modes = INDIO_DIRECT_MODE; >>>>> + indio_dev->channels = ms5637_channels; >>>>> + indio_dev->num_channels = ARRAY_SIZE(ms5637_channels); >>>>> + >>>>> + i2c_set_clientdata(client, indio_dev); >>>>> + ret = i2c_smbus_write_byte(client, MS5637_RESET); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + usleep_range(3000, 6000); >>>>> + >>>>> + ret = ms5637_fill_calibration_coeffs(dev_data); >>>>> + if (ret == 0) >>>>> + dev_data->got_calibration_words = true; >>>>> + else >>>>> + return ret; >>>>> + >>>>> + ret = iio_device_register(indio_dev); >>>>> + if (ret < 0) >>>>> + return ret; >>>>> + >>>>> + dev_dbg(&client->dev, "Driver initialization done"); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int ms5637_remove(struct i2c_client *client) >>>>> +{ >>>>> + struct iio_dev *indio_dev = i2c_get_clientdata(client); >>>>> + >>>>> + devm_iio_device_unregister(&client->dev, indio_dev); >>>>> + >>>>> + return 0; >>>>> +} >>>> seems that nothing special happens on removal, so maybe use >>>> devm_iio_device_register and drop *_remove(). >>>>> + >>>>> +static const struct i2c_device_id ms5637_id[] = { >>>>> + {"ms5637", 0}, >>>>> + {} >>>>> +}; >>>>> + >>>>> +static struct i2c_driver ms5637_driver = { >>>>> + .probe = ms5637_probe, >>>>> + .remove = ms5637_remove, >>>>> + .id_table = ms5637_id, >>>>> + .driver = { >>>>> + .name = "ms5637", >>>>> + .owner = THIS_MODULE, >>>>> + }, >>>>> +}; >>>>> + >>>>> +module_i2c_driver(ms5637_driver); >>>>> + >>>>> +MODULE_LICENSE("GPL"); >>>>> +MODULE_DESCRIPTION("Measurement-Specialties ms5637 temperature driver"); >>>>> +MODULE_AUTHOR("William Markezana <william.markezana@xxxxxxxxxxxxx>"); >>>>> +MODULE_AUTHOR("Ludovic Tancerel <ludovic.tancerel@xxxxxxxxxxxxxxxxx>"); >>>>> -- >>>>> 2.3.7 >>>>> >>>>> -- >>>>> 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 >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-iio" in > -- 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
