| Message ID | 20221115073718.2377311-3-gerald.loacker@wolfvision.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | add ti tmag5273 driver | expand |
On Tue, Nov 15, 2022 at 08:37:18AM +0100, Gerald Loacker wrote: > Add support for TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor. > Additionally to temperature and magnetic X, Y and Z-axes the angle and > magnitude are reported. > The sensor is operating in continuous measurement mode and changes to sleep > mode if not used for 5 seconds. > > Datasheet: https://www.ti.com/lit/gpn/tmag5273 > Drop this blank line to make above to be a tag. > Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net> ... > +#define TMAG5273_MANUFACTURER_ID 0x5449 Can you add ASCII comment on that magic value? ... > +#define TMAG5273_AUTOSLEEP_DELAY 5000 Units? > +struct tmag5273_data { > + struct device *dev; > + unsigned int devid; > + unsigned int version; > + char name[16]; > + int conv_avg; > + int max_avg; > + int range; > + u32 angle_en; > + struct regmap *map; > + struct regulator *vcc; + Blank line > + /* Locks the sensor for exclusive use during a measurement (which > + * involves several register transactions so the regmap lock is not > + * enough) so that measurements get serialized in a first-come-first- > + * serve manner. > + */ /* * Wrong multi-line * comment style. Fix * it accordingly. */ > + struct mutex lock; > +}; ... > +static const struct { > + int avg; > + u8 reg_val; > +} tmag5273_avg_table[] = { > + { 1, 0x00 }, { 2, 0x01 }, { 4, 0x02 }, > + { 8, 0x03 }, { 16, 0x04 }, { 32, 0x05 }, Isn't the second one is just an index? > +}; ... > +/* > + * magnetic range in mT for different TMAG5273 versions Magnetic > + * only version 1 and 2 are valid, version 0 and 3 are reserved > + */ > +static const struct { > + int range; > + u8 reg_val; > +} tmag5273_range_table[4][2] = { I believe you can drop 4. > + { { 0, 0 }, { 0, 3 } }, > + { { 40, 0 }, { 80, 3 } }, > + { { 133, 0 }, { 266, 3 } }, > + { { 0, 0 }, { 0, 3 } }, > +}; ... > +/* Shouldn't it be a kernel doc? Ditto for the rest. > + * tmag5273_measure() - Make a measure from the hardware > + * @tmag5273: The device state > + * @t: the processed temperature measurement > + * @x: the raw x axis measurement > + * @y: the raw x axis measurement > + * @z: the raw x axis measurement > + * @angle: the calculated angle > + * @magnitude: the calculated magnitude > + * @return: 0 on success or error code > + */ ... > + /* > + * convert device specific value to millicelsius Convert > + * use multiply by 16639 and divide by 10000 to achieve divide by 60.1 > + * and convert to millicelsius One space is enough and missing period. > + */ > +} ... > +static ssize_t tmag5273_show_conv_avg(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct tmag5273_data *data = iio_priv(indio_dev); > + > + return sprintf(buf, "%d\n", data->conv_avg); Must be sysfs_emit(). > +} ... > + for (i = 0; i < ARRAY_SIZE(tmag5273_avg_table); i++) { > + if (tmag5273_avg_table[i].avg == val) > + break; > + } > + Redundant blank line. > + if (i == ARRAY_SIZE(tmag5273_avg_table)) > + return -EINVAL; ... > +static IIO_DEVICE_ATTR(conv_avg, 0644, tmag5273_show_conv_avg, > + tmag5273_store_conv_avg, 0); IIO_DEVICE_ATTR_RW() ? ... > + for (i = 0; i < ARRAY_SIZE(tmag5273_avg_table); i++) { > + if (tmag5273_avg_table[i].avg > data->max_avg) > + break; > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", > + tmag5273_avg_table[i].avg); sysfs_emit_at() > + } > + /* replace last space with a newline */ > + if (len > 0) > + buf[len - 1] = '\n'; ... > +static IIO_DEVICE_ATTR(conv_avg_available, 0444, tmag5273_conv_avg_available, > + NULL, 0); IIO_DEVICE_ATTR_RO() ... > + return sprintf(buf, "%d\n", data->range); sysfs_emit(). ... > + for (i = 0; i < ARRAY_SIZE(tmag5273_range_table[0]); i++) { > + if (tmag5273_range_table[data->version][i].range == val) > + break; > + } > + Redundant blank line. > + if (i == ARRAY_SIZE(tmag5273_range_table[0])) > + return -EINVAL; ... > +static ssize_t tmag5273_store_range(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t len) > +{ > + struct iio_dev *indio_dev = dev_to_iio_dev(dev); > + struct tmag5273_data *data = iio_priv(indio_dev); > + int range, ret; > + > + ret = kstrtoint(buf, 0, &range); Here and in the other ->store() do you really have negative values possible? Can you revisit all those kstrtoX() calls and check the arguments, so you can narrow down the range of the input where it's appropriate? > + if (ret) > + return ret; > + > + ret = tmag5273_write_range(data, range); > + if (ret < 0) > + return ret; > + > + return len; > +} > + > +static IIO_DEVICE_ATTR(range, 0644, tmag5273_show_range, > + tmag5273_store_range, 0); IIO_DEVICE_ATTR_RW() > + for (i = 0; i < ARRAY_SIZE(tmag5273_range_table[0]); i++) { > + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", > + tmag5273_range_table[data->version][i].range); > + } sysfs_emit(); > + /* replace last space with a newline */ > + if (len > 0) > + buf[len - 1] = '\n'; > + > + return len; > +} ... > +static IIO_DEVICE_ATTR(range_available, 0444, tmag5273_range_available, NULL, > + 0); One line. ... > +static struct attribute *tmag5273_attributes[] = { > + &iio_dev_attr_conv_avg.dev_attr.attr, > + &iio_dev_attr_conv_avg_available.dev_attr.attr, > + &iio_dev_attr_range.dev_attr.attr, > + &iio_dev_attr_range_available.dev_attr.attr, > + NULL, No comma in terminator entry. > +}; > +static const struct attribute_group tmag5273_attrs_group = { > + .attrs = tmag5273_attributes, > +}; ... > +static bool tmag5273_volatile_reg(struct device *dev, unsigned int reg) > +{ > + return (reg >= TMAG5273_T_MSB_RESULT && > + reg <= TMAG5273_MAGNITUDE_RESULT); Drop parentheses and make it one line. > +} ... > +static int tmag5273_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) Why ->probe_new() can't be utilized? ... > + struct device_node *node = dev->of_node; What for? Use device property API (see below). ... > + data->map = devm_regmap_init_i2c(i2c, &tmag5273_regmap_config); > + if (IS_ERR(data->map)) { > + ret = PTR_ERR(data->map); > + dev_err_probe(dev, ret, "failed to allocate register map\n"); ret = dev_err_probe(); But don't you have an ordering issue here? > + goto out_disable_vcc; > + } ... > + strncpy(data->name, "TMAG5273", sizeof(data->name) - 2); > + switch (data->version) { > + case 1: > + strncat(data->name, "x1", 2); > + break; > + case 2: > + strncat(data->name, "x2", 2); > + break; > + default: > + break; > + } This all can be replaced with fewer lines: if (data->version < 1 || data->version > 2) snprintf(data->name, "TMAG5273", sizeof(data->name)); else snprintf(data->name, "TMAG5273x%.1u", sizeof(data->name), data->version); ... > + dev_info(dev, "%s", data->name); Useless? ... > + of_property_read_u32(node, "tmag5273,angle-enable", &data->angle_en); Missing header for that, but the question is why? What's wrong with device property API instead? ... > +static const struct i2c_device_id tmag5273_id[] = { > + { > + "tmag5273", > + }, Can be one line. > + { /* sentinel */ }, No comma for terminator entry. > +}; ... > + { /* sentinel */ }, Ditto.
On Tue, 15 Nov 2022 08:37:18 +0100 Gerald Loacker <gerald.loacker@wolfvision.net> wrote: > Add support for TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor. > Additionally to temperature and magnetic X, Y and Z-axes the angle and > magnitude are reported. > The sensor is operating in continuous measurement mode and changes to sleep > mode if not used for 5 seconds. > > Datasheet: https://www.ti.com/lit/gpn/tmag5273 > > Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net> Hi Gerald, Some comments inline. Jonathan > diff --git a/drivers/iio/magnetometer/tmag5273.c b/drivers/iio/magnetometer/tmag5273.c > new file mode 100644 > index 000000000000..549ed1ddba61 > --- /dev/null > +++ b/drivers/iio/magnetometer/tmag5273.c > @@ -0,0 +1,809 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Driver for the TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor > + * > + * Copyright (C) 2022 WolfVision GmbH > + * > + * Author: Gerald Loacker <gerald.loacker@wolfvision.net> > + */ > + > +#include <linux/delay.h> > +#include <linux/module.h> > +#include <linux/i2c.h> > +#include <linux/regmap.h> > +#include <linux/pm_runtime.h> > +#include <linux/iio/iio.h> > +#include <linux/iio/sysfs.h> > + > +#include <asm/unaligned.h> > + > +#define TMAG5273_DEVICE_CONFIG_1 0x00 > +#define TMAG5273_DEVICE_CONFIG_2 0x01 > +#define TMAG5273_SENSOR_CONFIG_1 0x02 > +#define TMAG5273_SENSOR_CONFIG_2 0x03 > +#define TMAG5273_X_THR_CONFIG 0x04 > +#define TMAG5273_Y_THR_CONFIG 0x05 > +#define TMAG5273_Z_THR_CONFIG 0x06 > +#define TMAG5273_T_CONFIG 0x07 > +#define TMAG5273_INT_CONFIG_1 0x08 > +#define TMAG5273_MAG_GAIN_CONFIG 0x09 > +#define TMAG5273_MAG_OFFSET_CONFIG_1 0x0A > +#define TMAG5273_MAG_OFFSET_CONFIG_2 0x0B > +#define TMAG5273_I2C_ADDRESS 0x0C > +#define TMAG5273_DEVICE_ID 0x0D > +#define TMAG5273_MANUFACTURER_ID_LSB 0x0E > +#define TMAG5273_MANUFACTURER_ID_MSB 0x0F > +#define TMAG5273_T_MSB_RESULT 0x10 > +#define TMAG5273_T_LSB_RESULT 0x11 > +#define TMAG5273_X_MSB_RESULT 0x12 > +#define TMAG5273_X_LSB_RESULT 0x13 > +#define TMAG5273_Y_MSB_RESULT 0x14 > +#define TMAG5273_Y_LSB_RESULT 0x15 > +#define TMAG5273_Z_MSB_RESULT 0x16 > +#define TMAG5273_Z_LSB_RESULT 0x17 > +#define TMAG5273_CONV_STATUS 0x18 > +#define TMAG5273_ANGLE_RESULT_MSB 0x19 > +#define TMAG5273_ANGLE_RESULT_LSB 0x1A > +#define TMAG5273_MAGNITUDE_RESULT 0x1B > +#define TMAG5273_DEVICE_STATUS 0x1C > + > +#define TMAG5273_MANUFACTURER_ID 0x5449 > + > +#define TMAG5273_AUTOSLEEP_DELAY 5000 > + > +/* Bits in the TMAG5273_DEVICE_CONFIG_1 register */ > +#define TMAG5273_AVG_MODE_MASK GENMASK(4, 2) > +#define TMAG5273_AVG_1_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 0) > +#define TMAG5273_AVG_2_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 1) > +#define TMAG5273_AVG_4_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 2) > +#define TMAG5273_AVG_8_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 3) > +#define TMAG5273_AVG_16_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 4) > +#define TMAG5273_AVG_32_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 5) > + > +/* bits in the TMAG5273_DEVICE_CONFIG_2 register */ > +#define TMAG5273_OP_MODE_MASK GENMASK(1, 0) > +#define TMAG5273_OP_MODE_STANDBY FIELD_PREP(TMAG5273_OP_MODE_MASK, 0) > +#define TMAG5273_OP_MODE_SLEEP FIELD_PREP(TMAG5273_OP_MODE_MASK, 1) > +#define TMAG5273_OP_MODE_CONT FIELD_PREP(TMAG5273_OP_MODE_MASK, 2) > +#define TMAG5273_OP_MODE_WAKEUP FIELD_PREP(TMAG5273_OP_MODE_MASK, 3) > + > +/* bits in the TMAG5273_SENSOR_CONFIG_1 register */ > +#define TMAG5273_MAG_CH_EN_MASK GENMASK(7, 4) > +#define TMAG5273_MAG_CH_EN_X_Y_Z 0x07 > + > +/* bits in the TMAG5273_SENSOR_CONFIG_2 register */ > +#define TMAG5273_Z_RANGE_MASK BIT(0) > +#define TMAG5273_X_Y_RANGE_MASK BIT(1) > +#define TMAG5273_ANGLE_EN_MASK GENMASK(3, 2) > +#define TMAG5273_ANGLE_EN_X_Y 1 > +#define TMAG5273_ANGLE_EN_Y_Z 2 > +#define TMAG5273_ANGLE_EN_X_Z 3 > + > +/* bits in the TMAG5273_T_CONFIG register */ > +#define TMAG5273_T_CH_EN BIT(0) > + > +/* bits in the TMAG5273_DEVICE_ID register */ > +#define TMAG5273_VERSION_MASK GENMASK(1, 0) > + > +/* bits in the TMAG5273_CONV_STATUS register */ > +#define TMAG5273_CONV_STATUS_COMPLETE BIT(0) > + > +/* state container for the TMAG5273 driver */ > +struct tmag5273_data { > + struct device *dev; > + unsigned int devid; > + unsigned int version; > + char name[16]; > + int conv_avg; > + int max_avg; > + int range; > + u32 angle_en; > + struct regmap *map; > + struct regulator *vcc; > + /* Locks the sensor for exclusive use during a measurement (which > + * involves several register transactions so the regmap lock is not > + * enough) so that measurements get serialized in a first-come-first- > + * serve manner. > + */ > + struct mutex lock; > +}; > + > +static const struct { > + int avg; > + u8 reg_val; > +} tmag5273_avg_table[] = { > + { 1, 0x00 }, { 2, 0x01 }, { 4, 0x02 }, > + { 8, 0x03 }, { 16, 0x04 }, { 32, 0x05 }, > +}; > + > +/* > + * magnetic range in mT for different TMAG5273 versions > + * only version 1 and 2 are valid, version 0 and 3 are reserved > + */ > +static const struct { > + int range; > + u8 reg_val; > +} tmag5273_range_table[4][2] = { > + { { 0, 0 }, { 0, 3 } }, > + { { 40, 0 }, { 80, 3 } }, > + { { 133, 0 }, { 266, 3 } }, > + { { 0, 0 }, { 0, 3 } }, > +}; > + > +/* /** It's kernel doc style, so mark it as such, but also run the kernel doc script over it and fix any warnings. > + * tmag5273_measure() - Make a measure from the hardware > + * @tmag5273: The device state > + * @t: the processed temperature measurement > + * @x: the raw x axis measurement > + * @y: the raw x axis measurement > + * @z: the raw x axis measurement > + * @angle: the calculated angle > + * @magnitude: the calculated magnitude > + * @return: 0 on success or error code > + */ > +static int tmag5273_measure(struct tmag5273_data *data, u16 *t, u16 *x, u16 *y, > + u16 *z, u16 *angle, u16 *magnitude) > +{ > + unsigned int status; > + u8 reg_data[12]; > + int ret; > + u16 val; > + > + mutex_lock(&data->lock); > + ret = regmap_read(data->map, TMAG5273_CONV_STATUS, &status); > + if (ret < 0) > + goto out_unlock; > + > + /* > + * Conversion time is 2425 us in 32x averaging mode for all three > + * channels. Since we are in continuous measurement mode, a measurement > + * may already be there, so poll for completed measurement with > + * timeout. Ideally adjust timeout to match what we expect for current mode. Can be a later optimization if you prefer. > + */ > + ret = regmap_read_poll_timeout(data->map, TMAG5273_CONV_STATUS, status, > + status & TMAG5273_CONV_STATUS_COMPLETE, > + 100, 10000); > + if (ret) { > + dev_err_probe(data->dev, ret, "timeout waiting for measurement\n"); > + goto out_unlock; > + } > + > + ret = regmap_bulk_read(data->map, TMAG5273_T_MSB_RESULT, reg_data, > + sizeof(reg_data)); Is there a need to read it in bulk? Seems like it makes sense for the first 8 registers. If you split it there those can go directly into __be16 reg_data[4] then you can do a 2 byte read to get the angle and a separate one byte read to get the magnitude. Not as efficient though so maybe this dance is worthwhile. > + if (ret) > + goto out_unlock; > + > + ret = regmap_read(data->map, TMAG5273_CONV_STATUS, &status); > + if (ret < 0) > + goto out_unlock; > + > + mutex_unlock(&data->lock); > + > + *t = get_unaligned_be16(®_data[0]); > + *x = get_unaligned_be16(®_data[2]); > + *y = get_unaligned_be16(®_data[4]); > + *z = get_unaligned_be16(®_data[6]); As below - drop these directly into the s16 variables rather than going via a u16. > + /* > + * angle has 9 bits integer value and 4 bits fractional part > + * 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 > + * 0 0 0 a a a a a a a a a f f f f > + */ > + val = get_unaligned_be16(®_data[9]); > + *angle = FIELD_GET(GENMASK(12, 0), val); > + *magnitude = reg_data[11]; > + return ret; > + > +out_unlock: > + mutex_unlock(&data->lock); > + return ret; > +} > + > +/* > + * tmag5273_get_measure() - Measure a sample of all axis and process > + * @tmag5273: The device state > + * @to: Temperature out > + * @xo: X axis out > + * @yo: Y axis out > + * @zo: Z axis out > + * @ao: Angle out > + * @mo: Magnitude out > + * @return: 0 on success or error code > + */ > +static int tmag5273_get_measure(struct tmag5273_data *data, s32 *to, s32 *xo, > + s32 *yo, s32 *zo, u16 *ao, u16 *mo) > +{ > + u16 t, x, y, z, angle, magnitude; If these are signed, use signed variables. > + int ret; > + > + ret = tmag5273_measure(data, &t, &x, &y, &z, &angle, &magnitude); > + if (ret) > + return ret; > + > + /* > + * convert device specific value to millicelsius > + * use multiply by 16639 and divide by 10000 to achieve divide by 60.1 > + * and convert to millicelsius > + */ > + *to = (((s32)t - 17508) * 16639) / 1000 + 25000; That looks like it should be presented as a _raw channel and the offset and scale exported to userspace (which is much better at doign this sort of maths) > + *xo = (s16)x; > + *yo = (s16)y; > + *zo = (s16)z; If these are 16 bit, why expand to 32 bits? Also if you are doing that do it with sign_extend32() not like this so it is clear wht is going on. > + *ao = angle; > + *mo = magnitude; > + return 0; > +} > + > +static int tmag5273_read_raw(struct iio_dev *indio_dev, > + const struct iio_chan_spec *chan, int *val, > + int *val2, long mask) > +{ > + struct tmag5273_data *data = iio_priv(indio_dev); > + s32 t, x, y, z; > + u16 angle, magnitude; > + int ret; > + > + switch (mask) { > + case IIO_CHAN_INFO_PROCESSED: > + case IIO_CHAN_INFO_RAW: > + ret = pm_runtime_resume_and_get(data->dev); > + if (ret < 0) > + return ret; blank line here. > + ret = tmag5273_get_measure(data, &t, &x, &y, &z, &angle, > + &magnitude); > + if (ret) > + return ret; blank line here. > + pm_runtime_mark_last_busy(data->dev); > + pm_runtime_put_autosuspend(data->dev); > + switch (chan->scan_index) { Better to use chan->addr and put enum values in there for the different channels. scan_index has a different purpose that you aren't yet supporting in this driver (buffered mode / chardev access). > + case 0: > + *val = t; return IIO_VAL_INT; in each of these rather than break. > + break; > + case 1: > + *val = x; > + break; > + case 2: > + *val = y; > + break; > + case 3: > + *val = z; > + break; > + case 4: > + *val = angle; > + break; > + case 5: > + *val = magnitude; > + break; > + default: > + return dev_err_probe(data->dev, -EINVAL, "unknown channel\n"); Not in probe, so not appropriate to use dev_err_probe(). > + } > + return IIO_VAL_INT; > + case IIO_CHAN_INFO_SCALE: > + switch (chan->type) { > + case IIO_TEMP: > + /* 1 LSB = 1 millidegree Celsius */ Don't expose scale if it's already in the base units. You can't get here anyway as you haven't set relevant bit in the mask for the temp channesl. > + *val = 1; > + return IIO_VAL_INT; > + case IIO_MAGN: > + /* > + * The axis values are in stored in 2^15 / range LSB/mT. > + * Since 1 mT = 10 Gauss, we need to multiply by 10 and > + * divide by [range] to get Gauss from the raw value. > + */ > + *val = data->range * 10; > + *val2 = 32768; > + return IIO_VAL_FRACTIONAL; > + case IIO_ANGL: > + /* > + * Angle is in degrees and has four fractional bits, > + * therefore use 1/16 * pi/180 to convert to radiants. > + */ > + *val = 1000; > + *val2 = 916732; > + return IIO_VAL_FRACTIONAL; > + case IIO_DISTANCE: > + /* Magnitude is unscaled */ If it's unscaled then don't provide a scale and make it a raw reading not a processed one. This is common for various types of proximity sensor where we don't have any way to know the scaling. > + *val = 1; > + return IIO_VAL_INT; > + default: > + return -EINVAL; > + } > + default: > + /* Unknown request */ > + return -EINVAL; > + } > +} > +static struct attribute *tmag5273_attributes[] = { > + &iio_dev_attr_conv_avg.dev_attr.attr, > + &iio_dev_attr_conv_avg_available.dev_attr.attr, Needs documentation so we can review what it is for and see if it either fits with existing ABI or we need new generic ABI. Documentation/ABI/testing/sysfs-bus-iio-* Superficially I'm guessing this is an oversampling control and we have standard ABI for that. > + &iio_dev_attr_range.dev_attr.attr, > + &iio_dev_attr_range_available.dev_attr.attr, It's rare we allow a range attribute. I most cases the same can be derived from SCALE plus available for the raw channel. There have been a few really obscure corner cases where it has been only way of exposing things, but those always came with detailed justification. > + NULL, No trailing comma on NULL terminators. > +}; > + > +static const struct attribute_group tmag5273_attrs_group = { > + .attrs = tmag5273_attributes, > +}; > + > +#define TMAG5273_AXIS_CHANNEL(axis, index) \ > + { \ > + .type = IIO_MAGN, \ > + .modified = 1, \ > + .channel2 = IIO_MOD_##axis, \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE), \ > + .scan_index = index, \ > + .scan_type = { \ > + .sign = 's', \ > + .realbits = 32, \ > + .storagebits = 32, \ From above these seem to be 16 bit... > + .endianness = IIO_CPU, \ > + }, \ > + } > + > +static const struct iio_chan_spec tmag5273_channels[] = { > + { > + .type = IIO_TEMP, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), > + .scan_index = 0, > + .scan_type = { > + .sign = 's', > + .realbits = 32, > + .storagebits = 32, > + .endianness = IIO_CPU, > + }, > + }, > + TMAG5273_AXIS_CHANNEL(X, 1), > + TMAG5273_AXIS_CHANNEL(Y, 2), > + TMAG5273_AXIS_CHANNEL(Z, 3), > + { > + .type = IIO_ANGL, > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), > + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), > + .scan_index = 4, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_CPU, > + }, > + }, > + { > + .type = IIO_DISTANCE, > + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), Is it processed, or is it > + .scan_index = 5, > + .scan_type = { > + .sign = 'u', > + .realbits = 16, > + .storagebits = 16, > + .endianness = IIO_CPU, > + }, > + }, > + IIO_CHAN_SOFT_TIMESTAMP(6), > +}; ... > + > +static int tmag5273_chip_init(struct tmag5273_data *data) > +{ > + int ret; > + > + ret = regmap_write(data->map, TMAG5273_DEVICE_CONFIG_1, > + TMAG5273_AVG_32_MODE); > + if (ret) > + return ret; > + data->conv_avg = 32; Blank line here to separate blocks doing more or less unconnected things. > + ret = regmap_write(data->map, TMAG5273_DEVICE_CONFIG_2, > + TMAG5273_OP_MODE_CONT); Register a devm_add_action_or_reset() to handle putting it in sleep in error paths / remove(). > + if (ret) > + return ret; Blank line here helps readability a little. > + ret = regmap_write(data->map, TMAG5273_SENSOR_CONFIG_1, > + FIELD_PREP(TMAG5273_MAG_CH_EN_MASK, > + TMAG5273_MAG_CH_EN_X_Y_Z)); > + if (ret) > + return ret; and here > + ret = regmap_write(data->map, TMAG5273_SENSOR_CONFIG_2, > + FIELD_PREP(TMAG5273_ANGLE_EN_MASK, data->angle_en)); > + if (ret) > + return ret; > + > + data->range = tmag5273_range_table[data->version][0].range; > + return regmap_write(data->map, TMAG5273_T_CONFIG, TMAG5273_T_CH_EN); > +} > + > +static int tmag5273_probe(struct i2c_client *i2c, > + const struct i2c_device_id *id) > +{ > + struct iio_dev *indio_dev; > + struct device *dev = &i2c->dev; > + struct device_node *node = dev->of_node; > + struct tmag5273_data *data; > + int val, ret; > + > + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); > + if (!indio_dev) > + return -ENOMEM; > + > + data = iio_priv(indio_dev); > + i2c_set_clientdata(i2c, indio_dev); > + data->dev = dev; > + mutex_init(&data->lock); > + > + data->vcc = devm_regulator_get(dev, "vcc"); > + if (IS_ERR(data->vcc)) > + return dev_err_probe(dev, PTR_ERR(data->vcc), > + "failed to get VCC regulator\n"); > + > + /* Operating voltage 1.7V .. 3.6V according to datasheet */ That is normally considered a job for device tree binding or similar rather than a driver to configure it. AS such, can probably just use devm_regulator_get_enabled() and drop the need to manually turn it off again > + ret = regulator_set_voltage(data->vcc, 1700000, 3600000); > + if (ret) > + return dev_err_probe(dev, ret, "failed to set VCC voltage\n"); > + > + ret = regulator_enable(data->vcc); > + if (ret) > + return dev_err_probe(dev, ret, > + "failed to enable VCC regulator\n"); > + > + /* > + * Regulators have to ramp up with 3V/ms, additional time to go to > + * stand-by mode is 270us typically. We give 1 ms to 2 ms time. That should be a problem for the regulator driver, not this one. The regulator_enable() should not return until it is powered up and we can carry on. > + */ > + usleep_range(1000, 2000); > + > + data->map = devm_regmap_init_i2c(i2c, &tmag5273_regmap_config); > + if (IS_ERR(data->map)) { > + ret = PTR_ERR(data->map); > + dev_err_probe(dev, ret, "failed to allocate register map\n"); > + goto out_disable_vcc; > + } > + > + ret = regmap_read(data->map, TMAG5273_DEVICE_ID, &val); > + if (ret) { > + /* > + * If we come from sleep with power already activated, the > + * first I2C command wakes up the chip but will fail. Minor but I'd make this unconditional. It's a small cost in probe() time but simplifies the code flow a little. > + * Time to go to stand-by mode from sleep mode is 50us > + * typically. During this time no I2C access is possible. > + */ > + usleep_range(80, 200); > + > + ret = regmap_read(data->map, TMAG5273_DEVICE_ID, &val); > + if (ret) > + goto out_disable_vcc; > + } > + data->version = FIELD_PREP(TMAG5273_VERSION_MASK, val); > + > + ret = regmap_bulk_read(data->map, TMAG5273_MANUFACTURER_ID_LSB, > + &data->devid, 2); > + if (ret) > + goto out_disable_vcc; > + > + switch (data->devid) { > + case TMAG5273_MANUFACTURER_ID: > + strncpy(data->name, "TMAG5273", sizeof(data->name) - 2); Lowercase preferred as much more common for IIO device names. (no idea why, but we have ended up like that ;) I'd also be lazy / efficient as there are only two names switch (data->version) { case 1: indio_dev->name = "tmag5273x1"; break; case 2: indio_dev->name = "tmag5273x2"; break; default: return -ENODEV; //after switch to devm for everything. } > + switch (data->version) { > + case 1: > + strncat(data->name, "x1", 2); > + break; > + case 2: > + strncat(data->name, "x2", 2); > + break; > + default: > + break; > + } > + dev_info(dev, "%s", data->name); noisy so don't print it. Can read from sysfs anyway. > + data->max_avg = 32; Given this is only valid option, why is it in here? > + break; > + default: > + ret = -ENODEV; We generally only warn on such cases. Often manufacturers spin new versions of chips that are compatible enough that we give them a fallback compatible in device tree. That is only a useful thing to do if they'll 'work' with old drivers. A warning that it might be the wrong device is considered fine though. "Unknown device ID... Bit tricky in this case as we have to pick between two values for some things. Chopse a default maybe? > + dev_err_probe(dev, ret, "unhandled device ID 0x%x\n", data->devid); > + goto out_disable_vcc; > + } > + > + /* > + * Angle-enable is optional and set to 1 (enable X-Y plane) by default, > + * the value is modified only if a valid u32 value can be decoded. > + */ > + data->angle_en = TMAG5273_ANGLE_EN_X_Y; > + of_property_read_u32(node, "tmag5273,angle-enable", &data->angle_en); Use generic firmware properties linux/property.h so this will work with other types of firmware. > + > + ret = tmag5273_chip_init(data); > + if (ret) > + goto out_disable_vcc; > + > + indio_dev->info = &tmag5273_info; > + indio_dev->modes = INDIO_DIRECT_MODE; > + indio_dev->name = data->name; > + indio_dev->channels = tmag5273_channels; > + indio_dev->num_channels = ARRAY_SIZE(tmag5273_channels); > + > + pm_runtime_set_autosuspend_delay(dev, TMAG5273_AUTOSLEEP_DELAY); > + pm_runtime_use_autosuspend(dev); > + pm_runtime_mark_last_busy(dev); > + ret = pm_runtime_set_active(dev); > + if (ret < 0) > + goto out_disable_vcc; > + pm_runtime_enable(dev); devm_pm_runtime_enable() > + pm_runtime_idle(dev); Why idle rather than a pm_runtime_put_autosuspend()? > + > + ret = iio_device_register(indio_dev); devm_iio_device_register() > + if (ret) { > + dev_err_probe(dev, ret, "device register failed\n"); > + goto cleanup_runtime; > + } > + > + return 0; > + > +cleanup_runtime: > + pm_runtime_dont_use_autosuspend(dev); > + pm_runtime_disable(dev); > +out_disable_vcc: > + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); > + regulator_disable(data->vcc); > + return ret; > +} > + > +static void tmag5273_remove(struct i2c_client *i2c) > +{ > + struct iio_dev *indio_dev = i2c_get_clientdata(i2c); > + struct tmag5273_data *data = iio_priv(indio_dev); > + struct device *dev = &i2c->dev; > + > + iio_device_unregister(indio_dev); > + > + pm_runtime_dont_use_autosuspend(dev); > + pm_runtime_disable(dev); > + pm_runtime_set_suspended(dev); > + > + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); With use of devm_add_action_or_reset() for this you should be able to move to a simpler fully devm_ managed flow and not have a remove() callback at all. > + regulator_disable(data->vcc); > +} > + > +static int tmag5273_runtime_suspend(struct device *dev) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct tmag5273_data *data = iio_priv(indio_dev); > + > + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); > + > + return 0; > +} > + > +static int tmag5273_runtime_resume(struct device *dev) > +{ > + struct iio_dev *indio_dev = dev_get_drvdata(dev); > + struct tmag5273_data *data = iio_priv(indio_dev); > + int ret; > + > + ret = tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_CONT); > + if (ret) { > + /* > + * Time to go to stand-by mode from sleep mode is 50us > + * typically. During this time no I2C access is possible. > + */ How would we ever not have entered sleep mode and yet be calling runtime resume? I'd just make the i2c access twice unconditionally. > + usleep_range(80, 200); > + ret = tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_CONT); > + } > + > + return ret; > +} > + > +static const struct dev_pm_ops tmag5273_pm_ops = { > + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) > + RUNTIME_PM_OPS(tmag5273_runtime_suspend, tmag5273_runtime_resume, NULL) > +}; This looks familiar. static const DEFINE_RUNTIME_DEV_PM_OPS(...) > + > +static const struct i2c_device_id tmag5273_id[] = { > + { > + "tmag5273", > + }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(i2c, tmag5273_id); > + > +static const struct of_device_id tmag5273_of_match[] = { > + { > + .compatible = "ti,tmag5273", > + }, > + { /* sentinel */ }, Drop the trailing commas on these entries as we can never add anything after them so the comma never makes sense. > +}; > +MODULE_DEVICE_TABLE(of, tmag5273_of_match); Thanks, Jonathan
diff --git a/MAINTAINERS b/MAINTAINERS index ea7acec52f8b..9d20b5780051 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20618,6 +20618,7 @@ M: Gerald Loacker <gerald.loacker@wolfvision.net> L: linux-iio@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml +F: drivers/iio/magnetometer/tmag5273.c TI TRF7970A NFC DRIVER M: Mark Greer <mgreer@animalcreek.com> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig index b91fc5e6a26e..467819335588 100644 --- a/drivers/iio/magnetometer/Kconfig +++ b/drivers/iio/magnetometer/Kconfig @@ -208,6 +208,18 @@ config SENSORS_RM3100_SPI To compile this driver as a module, choose M here: the module will be called rm3100-spi. +config TI_TMAG5273 + tristate "TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor" + depends on I2C + select REGMAP_I2C + help + Say Y here to add support for the TI TMAG5273 Low-Power + Linear 3D Hall-Effect Sensor. + + This driver can also be compiled as a module. + To compile this driver as a module, choose M here: the module + will be called tmag5273. + config YAMAHA_YAS530 tristate "Yamaha YAS530 family of 3-Axis Magnetometers (I2C)" depends on I2C diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile index b9f45b7fafc3..b1c784ea71c8 100644 --- a/drivers/iio/magnetometer/Makefile +++ b/drivers/iio/magnetometer/Makefile @@ -29,4 +29,6 @@ obj-$(CONFIG_SENSORS_RM3100) += rm3100-core.o obj-$(CONFIG_SENSORS_RM3100_I2C) += rm3100-i2c.o obj-$(CONFIG_SENSORS_RM3100_SPI) += rm3100-spi.o +obj-$(CONFIG_TI_TMAG5273) += tmag5273.o + obj-$(CONFIG_YAMAHA_YAS530) += yamaha-yas530.o diff --git a/drivers/iio/magnetometer/tmag5273.c b/drivers/iio/magnetometer/tmag5273.c new file mode 100644 index 000000000000..549ed1ddba61 --- /dev/null +++ b/drivers/iio/magnetometer/tmag5273.c @@ -0,0 +1,809 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Driver for the TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor + * + * Copyright (C) 2022 WolfVision GmbH + * + * Author: Gerald Loacker <gerald.loacker@wolfvision.net> + */ + +#include <linux/delay.h> +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/regmap.h> +#include <linux/pm_runtime.h> +#include <linux/iio/iio.h> +#include <linux/iio/sysfs.h> + +#include <asm/unaligned.h> + +#define TMAG5273_DEVICE_CONFIG_1 0x00 +#define TMAG5273_DEVICE_CONFIG_2 0x01 +#define TMAG5273_SENSOR_CONFIG_1 0x02 +#define TMAG5273_SENSOR_CONFIG_2 0x03 +#define TMAG5273_X_THR_CONFIG 0x04 +#define TMAG5273_Y_THR_CONFIG 0x05 +#define TMAG5273_Z_THR_CONFIG 0x06 +#define TMAG5273_T_CONFIG 0x07 +#define TMAG5273_INT_CONFIG_1 0x08 +#define TMAG5273_MAG_GAIN_CONFIG 0x09 +#define TMAG5273_MAG_OFFSET_CONFIG_1 0x0A +#define TMAG5273_MAG_OFFSET_CONFIG_2 0x0B +#define TMAG5273_I2C_ADDRESS 0x0C +#define TMAG5273_DEVICE_ID 0x0D +#define TMAG5273_MANUFACTURER_ID_LSB 0x0E +#define TMAG5273_MANUFACTURER_ID_MSB 0x0F +#define TMAG5273_T_MSB_RESULT 0x10 +#define TMAG5273_T_LSB_RESULT 0x11 +#define TMAG5273_X_MSB_RESULT 0x12 +#define TMAG5273_X_LSB_RESULT 0x13 +#define TMAG5273_Y_MSB_RESULT 0x14 +#define TMAG5273_Y_LSB_RESULT 0x15 +#define TMAG5273_Z_MSB_RESULT 0x16 +#define TMAG5273_Z_LSB_RESULT 0x17 +#define TMAG5273_CONV_STATUS 0x18 +#define TMAG5273_ANGLE_RESULT_MSB 0x19 +#define TMAG5273_ANGLE_RESULT_LSB 0x1A +#define TMAG5273_MAGNITUDE_RESULT 0x1B +#define TMAG5273_DEVICE_STATUS 0x1C + +#define TMAG5273_MANUFACTURER_ID 0x5449 + +#define TMAG5273_AUTOSLEEP_DELAY 5000 + +/* Bits in the TMAG5273_DEVICE_CONFIG_1 register */ +#define TMAG5273_AVG_MODE_MASK GENMASK(4, 2) +#define TMAG5273_AVG_1_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 0) +#define TMAG5273_AVG_2_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 1) +#define TMAG5273_AVG_4_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 2) +#define TMAG5273_AVG_8_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 3) +#define TMAG5273_AVG_16_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 4) +#define TMAG5273_AVG_32_MODE FIELD_PREP(TMAG5273_AVG_MODE_MASK, 5) + +/* bits in the TMAG5273_DEVICE_CONFIG_2 register */ +#define TMAG5273_OP_MODE_MASK GENMASK(1, 0) +#define TMAG5273_OP_MODE_STANDBY FIELD_PREP(TMAG5273_OP_MODE_MASK, 0) +#define TMAG5273_OP_MODE_SLEEP FIELD_PREP(TMAG5273_OP_MODE_MASK, 1) +#define TMAG5273_OP_MODE_CONT FIELD_PREP(TMAG5273_OP_MODE_MASK, 2) +#define TMAG5273_OP_MODE_WAKEUP FIELD_PREP(TMAG5273_OP_MODE_MASK, 3) + +/* bits in the TMAG5273_SENSOR_CONFIG_1 register */ +#define TMAG5273_MAG_CH_EN_MASK GENMASK(7, 4) +#define TMAG5273_MAG_CH_EN_X_Y_Z 0x07 + +/* bits in the TMAG5273_SENSOR_CONFIG_2 register */ +#define TMAG5273_Z_RANGE_MASK BIT(0) +#define TMAG5273_X_Y_RANGE_MASK BIT(1) +#define TMAG5273_ANGLE_EN_MASK GENMASK(3, 2) +#define TMAG5273_ANGLE_EN_X_Y 1 +#define TMAG5273_ANGLE_EN_Y_Z 2 +#define TMAG5273_ANGLE_EN_X_Z 3 + +/* bits in the TMAG5273_T_CONFIG register */ +#define TMAG5273_T_CH_EN BIT(0) + +/* bits in the TMAG5273_DEVICE_ID register */ +#define TMAG5273_VERSION_MASK GENMASK(1, 0) + +/* bits in the TMAG5273_CONV_STATUS register */ +#define TMAG5273_CONV_STATUS_COMPLETE BIT(0) + +/* state container for the TMAG5273 driver */ +struct tmag5273_data { + struct device *dev; + unsigned int devid; + unsigned int version; + char name[16]; + int conv_avg; + int max_avg; + int range; + u32 angle_en; + struct regmap *map; + struct regulator *vcc; + /* Locks the sensor for exclusive use during a measurement (which + * involves several register transactions so the regmap lock is not + * enough) so that measurements get serialized in a first-come-first- + * serve manner. + */ + struct mutex lock; +}; + +static const struct { + int avg; + u8 reg_val; +} tmag5273_avg_table[] = { + { 1, 0x00 }, { 2, 0x01 }, { 4, 0x02 }, + { 8, 0x03 }, { 16, 0x04 }, { 32, 0x05 }, +}; + +/* + * magnetic range in mT for different TMAG5273 versions + * only version 1 and 2 are valid, version 0 and 3 are reserved + */ +static const struct { + int range; + u8 reg_val; +} tmag5273_range_table[4][2] = { + { { 0, 0 }, { 0, 3 } }, + { { 40, 0 }, { 80, 3 } }, + { { 133, 0 }, { 266, 3 } }, + { { 0, 0 }, { 0, 3 } }, +}; + +/* + * tmag5273_measure() - Make a measure from the hardware + * @tmag5273: The device state + * @t: the processed temperature measurement + * @x: the raw x axis measurement + * @y: the raw x axis measurement + * @z: the raw x axis measurement + * @angle: the calculated angle + * @magnitude: the calculated magnitude + * @return: 0 on success or error code + */ +static int tmag5273_measure(struct tmag5273_data *data, u16 *t, u16 *x, u16 *y, + u16 *z, u16 *angle, u16 *magnitude) +{ + unsigned int status; + u8 reg_data[12]; + int ret; + u16 val; + + mutex_lock(&data->lock); + ret = regmap_read(data->map, TMAG5273_CONV_STATUS, &status); + if (ret < 0) + goto out_unlock; + + /* + * Conversion time is 2425 us in 32x averaging mode for all three + * channels. Since we are in continuous measurement mode, a measurement + * may already be there, so poll for completed measurement with + * timeout. + */ + ret = regmap_read_poll_timeout(data->map, TMAG5273_CONV_STATUS, status, + status & TMAG5273_CONV_STATUS_COMPLETE, + 100, 10000); + if (ret) { + dev_err_probe(data->dev, ret, "timeout waiting for measurement\n"); + goto out_unlock; + } + + ret = regmap_bulk_read(data->map, TMAG5273_T_MSB_RESULT, reg_data, + sizeof(reg_data)); + if (ret) + goto out_unlock; + + ret = regmap_read(data->map, TMAG5273_CONV_STATUS, &status); + if (ret < 0) + goto out_unlock; + + mutex_unlock(&data->lock); + + *t = get_unaligned_be16(®_data[0]); + *x = get_unaligned_be16(®_data[2]); + *y = get_unaligned_be16(®_data[4]); + *z = get_unaligned_be16(®_data[6]); + /* + * angle has 9 bits integer value and 4 bits fractional part + * 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 + * 0 0 0 a a a a a a a a a f f f f + */ + val = get_unaligned_be16(®_data[9]); + *angle = FIELD_GET(GENMASK(12, 0), val); + *magnitude = reg_data[11]; + return ret; + +out_unlock: + mutex_unlock(&data->lock); + return ret; +} + +/* + * tmag5273_get_measure() - Measure a sample of all axis and process + * @tmag5273: The device state + * @to: Temperature out + * @xo: X axis out + * @yo: Y axis out + * @zo: Z axis out + * @ao: Angle out + * @mo: Magnitude out + * @return: 0 on success or error code + */ +static int tmag5273_get_measure(struct tmag5273_data *data, s32 *to, s32 *xo, + s32 *yo, s32 *zo, u16 *ao, u16 *mo) +{ + u16 t, x, y, z, angle, magnitude; + int ret; + + ret = tmag5273_measure(data, &t, &x, &y, &z, &angle, &magnitude); + if (ret) + return ret; + + /* + * convert device specific value to millicelsius + * use multiply by 16639 and divide by 10000 to achieve divide by 60.1 + * and convert to millicelsius + */ + *to = (((s32)t - 17508) * 16639) / 1000 + 25000; + *xo = (s16)x; + *yo = (s16)y; + *zo = (s16)z; + *ao = angle; + *mo = magnitude; + return 0; +} + +static int tmag5273_read_raw(struct iio_dev *indio_dev, + const struct iio_chan_spec *chan, int *val, + int *val2, long mask) +{ + struct tmag5273_data *data = iio_priv(indio_dev); + s32 t, x, y, z; + u16 angle, magnitude; + int ret; + + switch (mask) { + case IIO_CHAN_INFO_PROCESSED: + case IIO_CHAN_INFO_RAW: + ret = pm_runtime_resume_and_get(data->dev); + if (ret < 0) + return ret; + ret = tmag5273_get_measure(data, &t, &x, &y, &z, &angle, + &magnitude); + if (ret) + return ret; + pm_runtime_mark_last_busy(data->dev); + pm_runtime_put_autosuspend(data->dev); + switch (chan->scan_index) { + case 0: + *val = t; + break; + case 1: + *val = x; + break; + case 2: + *val = y; + break; + case 3: + *val = z; + break; + case 4: + *val = angle; + break; + case 5: + *val = magnitude; + break; + default: + return dev_err_probe(data->dev, -EINVAL, "unknown channel\n"); + } + return IIO_VAL_INT; + case IIO_CHAN_INFO_SCALE: + switch (chan->type) { + case IIO_TEMP: + /* 1 LSB = 1 millidegree Celsius */ + *val = 1; + return IIO_VAL_INT; + case IIO_MAGN: + /* + * The axis values are in stored in 2^15 / range LSB/mT. + * Since 1 mT = 10 Gauss, we need to multiply by 10 and + * divide by [range] to get Gauss from the raw value. + */ + *val = data->range * 10; + *val2 = 32768; + return IIO_VAL_FRACTIONAL; + case IIO_ANGL: + /* + * Angle is in degrees and has four fractional bits, + * therefore use 1/16 * pi/180 to convert to radiants. + */ + *val = 1000; + *val2 = 916732; + return IIO_VAL_FRACTIONAL; + case IIO_DISTANCE: + /* Magnitude is unscaled */ + *val = 1; + return IIO_VAL_INT; + default: + return -EINVAL; + } + default: + /* Unknown request */ + return -EINVAL; + } +} + +static ssize_t tmag5273_show_conv_avg(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + + return sprintf(buf, "%d\n", data->conv_avg); +} + +static ssize_t tmag5273_write_conv_avg(struct tmag5273_data *data, + unsigned int val) +{ + int ret; + int i; + + for (i = 0; i < ARRAY_SIZE(tmag5273_avg_table); i++) { + if (tmag5273_avg_table[i].avg == val) + break; + } + + if (i == ARRAY_SIZE(tmag5273_avg_table)) + return -EINVAL; + + ret = regmap_update_bits(data->map, TMAG5273_DEVICE_CONFIG_1, + TMAG5273_AVG_MODE_MASK, + tmag5273_avg_table[i].reg_val); + if (ret) + return ret; + + data->conv_avg = val; + + return 0; +} + +static ssize_t tmag5273_store_conv_avg(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + int ret; + int integer; + + ret = kstrtoint(buf, 0, &integer); + if (ret) + return ret; + + ret = tmag5273_write_conv_avg(data, integer); + if (ret < 0) + return ret; + + return len; +} + +static IIO_DEVICE_ATTR(conv_avg, 0644, tmag5273_show_conv_avg, + tmag5273_store_conv_avg, 0); + +static ssize_t tmag5273_conv_avg_available(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + ssize_t len = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(tmag5273_avg_table); i++) { + if (tmag5273_avg_table[i].avg > data->max_avg) + break; + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", + tmag5273_avg_table[i].avg); + } + /* replace last space with a newline */ + if (len > 0) + buf[len - 1] = '\n'; + + return len; +} + +static IIO_DEVICE_ATTR(conv_avg_available, 0444, tmag5273_conv_avg_available, + NULL, 0); + +static ssize_t tmag5273_show_range(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + + return sprintf(buf, "%d\n", data->range); +} + +static ssize_t tmag5273_write_range(struct tmag5273_data *data, + unsigned int val) +{ + int i, ret; + + for (i = 0; i < ARRAY_SIZE(tmag5273_range_table[0]); i++) { + if (tmag5273_range_table[data->version][i].range == val) + break; + } + + if (i == ARRAY_SIZE(tmag5273_range_table[0])) + return -EINVAL; + + ret = regmap_update_bits(data->map, + TMAG5273_SENSOR_CONFIG_2, + TMAG5273_Z_RANGE_MASK | TMAG5273_X_Y_RANGE_MASK, + tmag5273_range_table[data->version][i].reg_val); + if (ret) + return ret; + + data->range = val; + + return 0; +} + +static ssize_t tmag5273_store_range(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t len) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + int range, ret; + + ret = kstrtoint(buf, 0, &range); + if (ret) + return ret; + + ret = tmag5273_write_range(data, range); + if (ret < 0) + return ret; + + return len; +} + +static IIO_DEVICE_ATTR(range, 0644, tmag5273_show_range, + tmag5273_store_range, 0); + +static ssize_t tmag5273_range_available(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct iio_dev *indio_dev = dev_to_iio_dev(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + ssize_t len = 0; + int i; + + for (i = 0; i < ARRAY_SIZE(tmag5273_range_table[0]); i++) { + len += scnprintf(buf + len, PAGE_SIZE - len, "%d ", + tmag5273_range_table[data->version][i].range); + } + /* replace last space with a newline */ + if (len > 0) + buf[len - 1] = '\n'; + + return len; +} + +static IIO_DEVICE_ATTR(range_available, 0444, tmag5273_range_available, NULL, + 0); + +static struct attribute *tmag5273_attributes[] = { + &iio_dev_attr_conv_avg.dev_attr.attr, + &iio_dev_attr_conv_avg_available.dev_attr.attr, + &iio_dev_attr_range.dev_attr.attr, + &iio_dev_attr_range_available.dev_attr.attr, + NULL, +}; + +static const struct attribute_group tmag5273_attrs_group = { + .attrs = tmag5273_attributes, +}; + +#define TMAG5273_AXIS_CHANNEL(axis, index) \ + { \ + .type = IIO_MAGN, \ + .modified = 1, \ + .channel2 = IIO_MOD_##axis, \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE), \ + .scan_index = index, \ + .scan_type = { \ + .sign = 's', \ + .realbits = 32, \ + .storagebits = 32, \ + .endianness = IIO_CPU, \ + }, \ + } + +static const struct iio_chan_spec tmag5273_channels[] = { + { + .type = IIO_TEMP, + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .scan_index = 0, + .scan_type = { + .sign = 's', + .realbits = 32, + .storagebits = 32, + .endianness = IIO_CPU, + }, + }, + TMAG5273_AXIS_CHANNEL(X, 1), + TMAG5273_AXIS_CHANNEL(Y, 2), + TMAG5273_AXIS_CHANNEL(Z, 3), + { + .type = IIO_ANGL, + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), + .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), + .scan_index = 4, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + { + .type = IIO_DISTANCE, + .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED), + .scan_index = 5, + .scan_type = { + .sign = 'u', + .realbits = 16, + .storagebits = 16, + .endianness = IIO_CPU, + }, + }, + IIO_CHAN_SOFT_TIMESTAMP(6), +}; + +static const struct iio_info tmag5273_info = { + .attrs = &tmag5273_attrs_group, + .read_raw = &tmag5273_read_raw, +}; + +static bool tmag5273_volatile_reg(struct device *dev, unsigned int reg) +{ + return (reg >= TMAG5273_T_MSB_RESULT && + reg <= TMAG5273_MAGNITUDE_RESULT); +} + +static const struct regmap_config tmag5273_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + .max_register = 0xff, + .volatile_reg = tmag5273_volatile_reg, +}; + +static int tmag5273_set_operating_mode(struct tmag5273_data *data, + unsigned int val) +{ + return regmap_write(data->map, TMAG5273_DEVICE_CONFIG_2, val); +} + +static int tmag5273_chip_init(struct tmag5273_data *data) +{ + int ret; + + ret = regmap_write(data->map, TMAG5273_DEVICE_CONFIG_1, + TMAG5273_AVG_32_MODE); + if (ret) + return ret; + data->conv_avg = 32; + ret = regmap_write(data->map, TMAG5273_DEVICE_CONFIG_2, + TMAG5273_OP_MODE_CONT); + if (ret) + return ret; + ret = regmap_write(data->map, TMAG5273_SENSOR_CONFIG_1, + FIELD_PREP(TMAG5273_MAG_CH_EN_MASK, + TMAG5273_MAG_CH_EN_X_Y_Z)); + if (ret) + return ret; + ret = regmap_write(data->map, TMAG5273_SENSOR_CONFIG_2, + FIELD_PREP(TMAG5273_ANGLE_EN_MASK, data->angle_en)); + if (ret) + return ret; + + data->range = tmag5273_range_table[data->version][0].range; + return regmap_write(data->map, TMAG5273_T_CONFIG, TMAG5273_T_CH_EN); +} + +static int tmag5273_probe(struct i2c_client *i2c, + const struct i2c_device_id *id) +{ + struct iio_dev *indio_dev; + struct device *dev = &i2c->dev; + struct device_node *node = dev->of_node; + struct tmag5273_data *data; + int val, ret; + + indio_dev = devm_iio_device_alloc(dev, sizeof(*data)); + if (!indio_dev) + return -ENOMEM; + + data = iio_priv(indio_dev); + i2c_set_clientdata(i2c, indio_dev); + data->dev = dev; + mutex_init(&data->lock); + + data->vcc = devm_regulator_get(dev, "vcc"); + if (IS_ERR(data->vcc)) + return dev_err_probe(dev, PTR_ERR(data->vcc), + "failed to get VCC regulator\n"); + + /* Operating voltage 1.7V .. 3.6V according to datasheet */ + ret = regulator_set_voltage(data->vcc, 1700000, 3600000); + if (ret) + return dev_err_probe(dev, ret, "failed to set VCC voltage\n"); + + ret = regulator_enable(data->vcc); + if (ret) + return dev_err_probe(dev, ret, + "failed to enable VCC regulator\n"); + + /* + * Regulators have to ramp up with 3V/ms, additional time to go to + * stand-by mode is 270us typically. We give 1 ms to 2 ms time. + */ + usleep_range(1000, 2000); + + data->map = devm_regmap_init_i2c(i2c, &tmag5273_regmap_config); + if (IS_ERR(data->map)) { + ret = PTR_ERR(data->map); + dev_err_probe(dev, ret, "failed to allocate register map\n"); + goto out_disable_vcc; + } + + ret = regmap_read(data->map, TMAG5273_DEVICE_ID, &val); + if (ret) { + /* + * If we come from sleep with power already activated, the + * first I2C command wakes up the chip but will fail. + * Time to go to stand-by mode from sleep mode is 50us + * typically. During this time no I2C access is possible. + */ + usleep_range(80, 200); + + ret = regmap_read(data->map, TMAG5273_DEVICE_ID, &val); + if (ret) + goto out_disable_vcc; + } + data->version = FIELD_PREP(TMAG5273_VERSION_MASK, val); + + ret = regmap_bulk_read(data->map, TMAG5273_MANUFACTURER_ID_LSB, + &data->devid, 2); + if (ret) + goto out_disable_vcc; + + switch (data->devid) { + case TMAG5273_MANUFACTURER_ID: + strncpy(data->name, "TMAG5273", sizeof(data->name) - 2); + switch (data->version) { + case 1: + strncat(data->name, "x1", 2); + break; + case 2: + strncat(data->name, "x2", 2); + break; + default: + break; + } + dev_info(dev, "%s", data->name); + data->max_avg = 32; + break; + default: + ret = -ENODEV; + dev_err_probe(dev, ret, "unhandled device ID 0x%x\n", data->devid); + goto out_disable_vcc; + } + + /* + * Angle-enable is optional and set to 1 (enable X-Y plane) by default, + * the value is modified only if a valid u32 value can be decoded. + */ + data->angle_en = TMAG5273_ANGLE_EN_X_Y; + of_property_read_u32(node, "tmag5273,angle-enable", &data->angle_en); + + ret = tmag5273_chip_init(data); + if (ret) + goto out_disable_vcc; + + indio_dev->info = &tmag5273_info; + indio_dev->modes = INDIO_DIRECT_MODE; + indio_dev->name = data->name; + indio_dev->channels = tmag5273_channels; + indio_dev->num_channels = ARRAY_SIZE(tmag5273_channels); + + pm_runtime_set_autosuspend_delay(dev, TMAG5273_AUTOSLEEP_DELAY); + pm_runtime_use_autosuspend(dev); + pm_runtime_mark_last_busy(dev); + ret = pm_runtime_set_active(dev); + if (ret < 0) + goto out_disable_vcc; + pm_runtime_enable(dev); + pm_runtime_idle(dev); + + ret = iio_device_register(indio_dev); + if (ret) { + dev_err_probe(dev, ret, "device register failed\n"); + goto cleanup_runtime; + } + + return 0; + +cleanup_runtime: + pm_runtime_dont_use_autosuspend(dev); + pm_runtime_disable(dev); +out_disable_vcc: + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); + regulator_disable(data->vcc); + return ret; +} + +static void tmag5273_remove(struct i2c_client *i2c) +{ + struct iio_dev *indio_dev = i2c_get_clientdata(i2c); + struct tmag5273_data *data = iio_priv(indio_dev); + struct device *dev = &i2c->dev; + + iio_device_unregister(indio_dev); + + pm_runtime_dont_use_autosuspend(dev); + pm_runtime_disable(dev); + pm_runtime_set_suspended(dev); + + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); + regulator_disable(data->vcc); +} + +static int tmag5273_runtime_suspend(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + + tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_SLEEP); + + return 0; +} + +static int tmag5273_runtime_resume(struct device *dev) +{ + struct iio_dev *indio_dev = dev_get_drvdata(dev); + struct tmag5273_data *data = iio_priv(indio_dev); + int ret; + + ret = tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_CONT); + if (ret) { + /* + * Time to go to stand-by mode from sleep mode is 50us + * typically. During this time no I2C access is possible. + */ + usleep_range(80, 200); + ret = tmag5273_set_operating_mode(data, TMAG5273_OP_MODE_CONT); + } + + return ret; +} + +static const struct dev_pm_ops tmag5273_pm_ops = { + SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume) + RUNTIME_PM_OPS(tmag5273_runtime_suspend, tmag5273_runtime_resume, NULL) +}; + +static const struct i2c_device_id tmag5273_id[] = { + { + "tmag5273", + }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(i2c, tmag5273_id); + +static const struct of_device_id tmag5273_of_match[] = { + { + .compatible = "ti,tmag5273", + }, + { /* sentinel */ }, +}; +MODULE_DEVICE_TABLE(of, tmag5273_of_match); + +static struct i2c_driver tmag5273_driver = { + .driver = { + .name = "tmag5273", + .of_match_table = tmag5273_of_match, + .pm = pm_ptr(&tmag5273_pm_ops), + }, + .probe = tmag5273_probe, + .remove = tmag5273_remove, + .id_table = tmag5273_id, +}; +module_i2c_driver(tmag5273_driver); + +MODULE_DESCRIPTION("TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor driver"); +MODULE_AUTHOR("Gerald Loacker <gerald.loacker@wolfvision.net>"); +MODULE_LICENSE("GPL");
Add support for TI TMAG5273 Low-Power Linear 3D Hall-Effect Sensor. Additionally to temperature and magnetic X, Y and Z-axes the angle and magnitude are reported. The sensor is operating in continuous measurement mode and changes to sleep mode if not used for 5 seconds. Datasheet: https://www.ti.com/lit/gpn/tmag5273 Signed-off-by: Gerald Loacker <gerald.loacker@wolfvision.net> --- MAINTAINERS | 1 + drivers/iio/magnetometer/Kconfig | 12 + drivers/iio/magnetometer/Makefile | 2 + drivers/iio/magnetometer/tmag5273.c | 809 ++++++++++++++++++++++++++++ 4 files changed, 824 insertions(+) create mode 100644 drivers/iio/magnetometer/tmag5273.c