[PATCH 1/4] iio: accel: bma180: Add dev helper variable

[PATCH 1/4] iio: accel: bma180: Add dev helper variable

[Linus Walleij]

Having a shorthand "dev" instead of &client->dev everywhere
makes the code easier to read (more compact).

Cc: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/bma180.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index 1574e4604a4f..518efbe4eaf6 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -712,12 +712,13 @@ static const struct iio_trigger_ops bma180_trigger_ops = {
 static int bma180_probe(struct i2c_client *client,
 		const struct i2c_device_id *id)
 {
+	struct device *dev = &client->dev;
 	struct bma180_data *data;
 	struct iio_dev *indio_dev;
 	enum chip_ids chip;
 	int ret;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -725,12 +726,12 @@ static int bma180_probe(struct i2c_client *client,
 	i2c_set_clientdata(client, indio_dev);
 	data->client = client;
 	if (client->dev.of_node)
-		chip = (enum chip_ids)of_device_get_match_data(&client->dev);
+		chip = (enum chip_ids)of_device_get_match_data(dev);
 	else
 		chip = id->driver_data;
 	data->part_info = &bma180_part_info[chip];
 
-	ret = iio_read_mount_matrix(&client->dev, "mount-matrix",
+	ret = iio_read_mount_matrix(dev, "mount-matrix",
 				&data->orientation);
 	if (ret)
 		return ret;
@@ -740,7 +741,7 @@ static int bma180_probe(struct i2c_client *client,
 		goto err_chip_disable;
 
 	mutex_init(&data->mutex);
-	indio_dev->dev.parent = &client->dev;
+	indio_dev->dev.parent = dev;
 	indio_dev->channels = data->part_info->channels;
 	indio_dev->num_channels = data->part_info->num_channels;
 	indio_dev->name = id->name;
@@ -755,15 +756,15 @@ static int bma180_probe(struct i2c_client *client,
 			goto err_chip_disable;
 		}
 
-		ret = devm_request_irq(&client->dev, client->irq,
+		ret = devm_request_irq(dev, client->irq,
 			iio_trigger_generic_data_rdy_poll, IRQF_TRIGGER_RISING,
 			"bma180_event", data->trig);
 		if (ret) {
-			dev_err(&client->dev, "unable to request IRQ\n");
+			dev_err(dev, "unable to request IRQ\n");
 			goto err_trigger_free;
 		}
 
-		data->trig->dev.parent = &client->dev;
+		data->trig->dev.parent = dev;
 		data->trig->ops = &bma180_trigger_ops;
 		iio_trigger_set_drvdata(data->trig, indio_dev);
 		indio_dev->trig = iio_trigger_get(data->trig);
@@ -776,13 +777,13 @@ static int bma180_probe(struct i2c_client *client,
 	ret = iio_triggered_buffer_setup(indio_dev, NULL,
 			bma180_trigger_handler, NULL);
 	if (ret < 0) {
-		dev_err(&client->dev, "unable to setup iio triggered buffer\n");
+		dev_err(dev, "unable to setup iio triggered buffer\n");
 		goto err_trigger_unregister;
 	}
 
 	ret = iio_device_register(indio_dev);
 	if (ret < 0) {
-		dev_err(&client->dev, "unable to register iio device\n");
+		dev_err(dev, "unable to register iio device\n");
 		goto err_buffer_cleanup;
 	}
 
-- 
2.21.0

[PATCH 2/4] iio: accel: bma180: Basic regulator support

[Linus Walleij]

This brings up the VDD and VDDIO regulators using the
regulator framework. Platforms that do not use regulators
will provide stubs or dummy regulators.

Cc: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/bma180.c | 43 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index 518efbe4eaf6..4a619b5a544a 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -18,6 +18,7 @@
 #include <linux/of_device.h>
 #include <linux/of.h>
 #include <linux/bitops.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/iio/iio.h>
@@ -110,6 +111,8 @@ struct bma180_part_info {
 #define BMA250_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
 
 struct bma180_data {
+	struct regulator *vdd_supply;
+	struct regulator *vddio_supply;
 	struct i2c_client *client;
 	struct iio_trigger *trig;
 	const struct bma180_part_info *part_info;
@@ -736,6 +739,40 @@ static int bma180_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
 
+	data->vdd_supply = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(data->vdd_supply)) {
+		if (PTR_ERR(data->vdd_supply) != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get vdd regulator %d\n",
+				(int)PTR_ERR(data->vdd_supply));
+		return PTR_ERR(data->vdd_supply);
+	}
+	data->vddio_supply = devm_regulator_get(dev, "vddio");
+	if (IS_ERR(data->vddio_supply)) {
+		if (PTR_ERR(data->vddio_supply) != -EPROBE_DEFER)
+			dev_err(dev, "Failed to get vddio regulator %d\n",
+				(int)PTR_ERR(data->vddio_supply));
+		return PTR_ERR(data->vddio_supply);
+	}
+	/* Typical voltage 2.4V these are min and max */
+	ret = regulator_set_voltage(data->vdd_supply, 1620000, 3600000);
+	if (ret)
+		return ret;
+	ret = regulator_set_voltage(data->vddio_supply, 1200000, 3600000);
+	if (ret)
+		return ret;
+	ret = regulator_enable(data->vdd_supply);
+	if (ret) {
+		dev_err(dev, "Failed to enable vdd regulator: %d\n", ret);
+		return ret;
+	}
+	ret = regulator_enable(data->vddio_supply);
+	if (ret) {
+		dev_err(dev, "Failed to enable vddio regulator: %d\n", ret);
+		goto err_disable_vdd;
+	}
+	/* Wait to make sure we started up properly (3 ms at least) */
+	usleep_range(3000, 5000);
+
 	ret = data->part_info->chip_config(data);
 	if (ret < 0)
 		goto err_chip_disable;
@@ -798,7 +835,9 @@ static int bma180_probe(struct i2c_client *client,
 	iio_trigger_free(data->trig);
 err_chip_disable:
 	data->part_info->chip_disable(data);
-
+	regulator_disable(data->vddio_supply);
+err_disable_vdd:
+	regulator_disable(data->vdd_supply);
 	return ret;
 }
 
@@ -817,6 +856,8 @@ static int bma180_remove(struct i2c_client *client)
 	mutex_lock(&data->mutex);
 	data->part_info->chip_disable(data);
 	mutex_unlock(&data->mutex);
+	regulator_disable(data->vddio_supply);
+	regulator_disable(data->vdd_supply);
 
 	return 0;
 }
-- 
2.21.0

[PATCH 3/4] iio: accel: bma180: Use explicit member assignment

[Linus Walleij]

This uses the C99 explicit .member assignment for the
variant data in struct bma180_part_info. This makes it
easier to understand and add new variants.

Cc: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/iio/accel/bma180.c | 68 ++++++++++++++++++++++++--------------
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index 4a619b5a544a..f583f10ccbb9 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -632,32 +632,52 @@ static const struct iio_chan_spec bma250_channels[] = {
 
 static const struct bma180_part_info bma180_part_info[] = {
 	[BMA180] = {
-		bma180_channels, ARRAY_SIZE(bma180_channels),
-		bma180_scale_table, ARRAY_SIZE(bma180_scale_table),
-		bma180_bw_table, ARRAY_SIZE(bma180_bw_table),
-		BMA180_CTRL_REG0, BMA180_RESET_INT,
-		BMA180_CTRL_REG0, BMA180_SLEEP,
-		BMA180_BW_TCS, BMA180_BW,
-		BMA180_OFFSET_LSB1, BMA180_RANGE,
-		BMA180_TCO_Z, BMA180_MODE_CONFIG, BMA180_LOW_POWER,
-		BMA180_CTRL_REG3, BMA180_NEW_DATA_INT,
-		BMA180_RESET,
-		bma180_chip_config,
-		bma180_chip_disable,
+		.channels = bma180_channels,
+		.num_channels = ARRAY_SIZE(bma180_channels),
+		.scale_table = bma180_scale_table,
+		.num_scales = ARRAY_SIZE(bma180_scale_table),
+		.bw_table = bma180_bw_table,
+		.num_bw = ARRAY_SIZE(bma180_bw_table),
+		.int_reset_reg = BMA180_CTRL_REG0,
+		.int_reset_mask = BMA180_RESET_INT,
+		.sleep_reg = BMA180_CTRL_REG0,
+		.sleep_mask = BMA180_SLEEP,
+		.bw_reg = BMA180_BW_TCS,
+		.bw_mask = BMA180_BW,
+		.scale_reg = BMA180_OFFSET_LSB1,
+		.scale_mask = BMA180_RANGE,
+		.power_reg = BMA180_TCO_Z,
+		.power_mask = BMA180_MODE_CONFIG,
+		.lowpower_val = BMA180_LOW_POWER,
+		.int_enable_reg = BMA180_CTRL_REG3,
+		.int_enable_mask = BMA180_NEW_DATA_INT,
+		.softreset_reg = BMA180_RESET,
+		.chip_config = bma180_chip_config,
+		.chip_disable = bma180_chip_disable,
 	},
 	[BMA250] = {
-		bma250_channels, ARRAY_SIZE(bma250_channels),
-		bma250_scale_table, ARRAY_SIZE(bma250_scale_table),
-		bma250_bw_table, ARRAY_SIZE(bma250_bw_table),
-		BMA250_INT_RESET_REG, BMA250_INT_RESET_MASK,
-		BMA250_POWER_REG, BMA250_SUSPEND_MASK,
-		BMA250_BW_REG, BMA250_BW_MASK,
-		BMA250_RANGE_REG, BMA250_RANGE_MASK,
-		BMA250_POWER_REG, BMA250_LOWPOWER_MASK, 1,
-		BMA250_INT_ENABLE_REG, BMA250_DATA_INTEN_MASK,
-		BMA250_RESET_REG,
-		bma250_chip_config,
-		bma250_chip_disable,
+		.channels = bma250_channels,
+		.num_channels = ARRAY_SIZE(bma250_channels),
+		.scale_table = bma250_scale_table,
+		.num_scales = ARRAY_SIZE(bma250_scale_table),
+		.bw_table = bma250_bw_table,
+		.num_bw = ARRAY_SIZE(bma250_bw_table),
+		.int_reset_reg = BMA250_INT_RESET_REG,
+		.int_reset_mask = BMA250_INT_RESET_MASK,
+		.sleep_reg = BMA250_POWER_REG,
+		.sleep_mask = BMA250_SUSPEND_MASK,
+		.bw_reg = BMA250_BW_REG,
+		.bw_mask = BMA250_BW_MASK,
+		.scale_reg = BMA250_RANGE_REG,
+		.scale_mask = BMA250_RANGE_MASK,
+		.power_reg = BMA250_POWER_REG,
+		.power_mask = BMA250_LOWPOWER_MASK,
+		.lowpower_val = 1,
+		.int_enable_reg = BMA250_INT_ENABLE_REG,
+		.int_enable_mask = BMA250_DATA_INTEN_MASK,
+		.softreset_reg = BMA250_RESET_REG,
+		.chip_config = bma250_chip_config,
+		.chip_disable = bma250_chip_disable,
 	},
 };
 
-- 
2.21.0

[PATCH 4/4] iio: accel: bma180: BMA254 support

[Linus Walleij]

This adds support for the BMA254 variant of this
accelerometer. The only difference for the simple IIO
driver is that values are 12 bit and the temperature
offset differs by 1 degree.

Cc: Peter Meerwald <pmeerw@pmeerw.net>
Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/iio/accel/bma180.txt  |   7 +-
 drivers/iio/accel/Kconfig                     |   4 +-
 drivers/iio/accel/bma180.c                    | 108 +++++++++++++++---
 3 files changed, 100 insertions(+), 19 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/accel/bma180.txt b/Documentation/devicetree/bindings/iio/accel/bma180.txt
index 3b25b4c4d446..f53237270b32 100644
--- a/Documentation/devicetree/bindings/iio/accel/bma180.txt
+++ b/Documentation/devicetree/bindings/iio/accel/bma180.txt
@@ -1,11 +1,14 @@
-* Bosch BMA180 / BMA250 triaxial acceleration sensor
+* Bosch BMA180 / BMA25x triaxial acceleration sensor
 
 http://omapworld.com/BMA180_111_1002839.pdf
 http://ae-bst.resource.bosch.com/media/products/dokumente/bma250/bst-bma250-ds002-05.pdf
 
 Required properties:
 
-  - compatible : should be "bosch,bma180" or "bosch,bma250"
+  - compatible : should be one of:
+    "bosch,bma180"
+    "bosch,bma250"
+    "bosch,bma254"
   - reg : the I2C address of the sensor
 
 Optional properties:
diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
index d4ef35aeb579..2f10fd7954fc 100644
--- a/drivers/iio/accel/Kconfig
+++ b/drivers/iio/accel/Kconfig
@@ -89,13 +89,13 @@ config ADXL372_I2C
 	  module will be called adxl372_i2c.
 
 config BMA180
-	tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
+	tristate "Bosch BMA180/BMA25x 3-Axis Accelerometer Driver"
 	depends on I2C
 	select IIO_BUFFER
 	select IIO_TRIGGERED_BUFFER
 	help
 	  Say Y here if you want to build a driver for the Bosch BMA180 or
-	  BMA250 triaxial acceleration sensor.
+	  BMA25x triaxial acceleration sensor.
 
 	  To compile this driver as a module, choose M here: the
 	  module will be called bma180.
diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
index f583f10ccbb9..952b2918525b 100644
--- a/drivers/iio/accel/bma180.c
+++ b/drivers/iio/accel/bma180.c
@@ -9,6 +9,7 @@
  * SPI is not supported by driver
  * BMA180: 7-bit I2C slave address 0x40 or 0x41
  * BMA250: 7-bit I2C slave address 0x18 or 0x19
+ * BMA254: 7-bit I2C slave address 0x18 or 0x19
  */
 
 #include <linux/module.h>
@@ -34,17 +35,20 @@
 enum chip_ids {
 	BMA180,
 	BMA250,
+	BMA254,
 };
 
 struct bma180_data;
 
 struct bma180_part_info {
+	u8 chip_id;
 	const struct iio_chan_spec *channels;
 	unsigned int num_channels;
 	const int *scale_table;
 	unsigned int num_scales;
 	const int *bw_table;
 	unsigned int num_bw;
+	int center_temp;
 
 	u8 int_reset_reg, int_reset_mask;
 	u8 sleep_reg, sleep_mask;
@@ -52,6 +56,7 @@ struct bma180_part_info {
 	u8 scale_reg, scale_mask;
 	u8 power_reg, power_mask, lowpower_val;
 	u8 int_enable_reg, int_enable_mask;
+	u8 int_map_reg, int_enable_dataready_int1_mask;
 	u8 softreset_reg;
 
 	int (*chip_config)(struct bma180_data *data);
@@ -90,6 +95,8 @@ struct bma180_part_info {
 #define BMA180_RESET_VAL	0xb6
 
 #define BMA180_ID_REG_VAL	0x03
+#define BMA250_ID_REG_VAL	0x03
+#define BMA254_ID_REG_VAL	0xfa /* 250 decimal */
 
 /* Chip power modes */
 #define BMA180_LOW_POWER	0x03
@@ -110,6 +117,23 @@ struct bma180_part_info {
 #define BMA250_INT1_DATA_MASK	BIT(0)
 #define BMA250_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
 
+#define BMA254_RANGE_REG	0x0f
+#define BMA254_BW_REG		0x10
+#define BMA254_POWER_REG	0x11
+#define BMA254_RESET_REG	0x14
+#define BMA254_INT_ENABLE_REG	0x17
+#define BMA254_INT_MAP_REG	0x1a
+#define BMA254_INT_RESET_REG	0x21
+
+#define BMA254_RANGE_MASK	GENMASK(3, 0) /* Range of accel values */
+#define BMA254_BW_MASK		GENMASK(4, 0) /* Accel bandwidth */
+#define BMA254_SUSPEND_MASK	BIT(7) /* chip will sleep */
+#define BMA254_LOWPOWER_MASK	BIT(6)
+#define BMA254_DATA_INTEN_MASK	BIT(4)
+#define BMA254_INT2_DATA_MASK	BIT(7)
+#define BMA254_INT1_DATA_MASK	BIT(0)
+#define BMA254_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
+
 struct bma180_data {
 	struct regulator *vdd_supply;
 	struct regulator *vddio_supply;
@@ -135,8 +159,8 @@ enum bma180_chan {
 static int bma180_bw_table[] = { 10, 20, 40, 75, 150, 300 }; /* Hz */
 static int bma180_scale_table[] = { 1275, 1863, 2452, 3727, 4903, 9709, 19417 };
 
-static int bma250_bw_table[] = { 8, 16, 31, 63, 125, 250 }; /* Hz */
-static int bma250_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
+static int bma25x_bw_table[] = { 8, 16, 31, 63, 125, 250 }; /* Hz */
+static int bma25x_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
 	0, 0, 306458 };
 
 static int bma180_get_data_reg(struct bma180_data *data, enum bma180_chan chan)
@@ -310,8 +334,11 @@ static int bma180_chip_init(struct bma180_data *data)
 
 	if (ret < 0)
 		return ret;
-	if (ret != BMA180_ID_REG_VAL)
+	if (ret != data->part_info->chip_id) {
+		dev_err(&data->client->dev, "wrong chip ID %d expected %d\n",
+			ret, data->part_info->chip_id);
 		return -ENODEV;
+	}
 
 	ret = bma180_soft_reset(data);
 	if (ret)
@@ -358,7 +385,7 @@ static int bma180_chip_config(struct bma180_data *data)
 	return ret;
 }
 
-static int bma250_chip_config(struct bma180_data *data)
+static int bma25x_chip_config(struct bma180_data *data)
 {
 	int ret = bma180_chip_init(data);
 
@@ -370,8 +397,12 @@ static int bma250_chip_config(struct bma180_data *data)
 	ret = bma180_set_scale(data, 38344); /* 2 G */
 	if (ret)
 		goto err;
-	ret = bma180_set_bits(data, BMA250_INT_MAP_REG,
-		BMA250_INT1_DATA_MASK, 1);
+	/*
+	 * This enables dataready interrupt on the INT1 pin
+	 * FIXME: support using the INT2 pin
+	 */
+	ret = bma180_set_bits(data, data->part_info->int_map_reg,
+		data->part_info->int_enable_dataready_int1_mask, 1);
 	if (ret)
 		goto err;
 
@@ -397,7 +428,7 @@ static void bma180_chip_disable(struct bma180_data *data)
 	dev_err(&data->client->dev, "failed to disable the chip\n");
 }
 
-static void bma250_chip_disable(struct bma180_data *data)
+static void bma25x_chip_disable(struct bma180_data *data)
 {
 	if (bma180_set_new_data_intr_state(data, false))
 		goto err;
@@ -500,7 +531,7 @@ static int bma180_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_OFFSET:
-		*val = 48; /* 0 LSB @ 24 degree C */
+		*val = data->part_info->center_temp;
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -630,14 +661,24 @@ static const struct iio_chan_spec bma250_channels[] = {
 	IIO_CHAN_SOFT_TIMESTAMP(4),
 };
 
+static const struct iio_chan_spec bma254_channels[] = {
+	BMA180_ACC_CHANNEL(X, 12),
+	BMA180_ACC_CHANNEL(Y, 12),
+	BMA180_ACC_CHANNEL(Z, 12),
+	BMA180_TEMP_CHANNEL,
+	IIO_CHAN_SOFT_TIMESTAMP(4),
+};
+
 static const struct bma180_part_info bma180_part_info[] = {
 	[BMA180] = {
+		.chip_id = BMA180_ID_REG_VAL,
 		.channels = bma180_channels,
 		.num_channels = ARRAY_SIZE(bma180_channels),
 		.scale_table = bma180_scale_table,
 		.num_scales = ARRAY_SIZE(bma180_scale_table),
 		.bw_table = bma180_bw_table,
 		.num_bw = ARRAY_SIZE(bma180_bw_table),
+		.center_temp = 48, /* 0 LSB @ 24 degree C */
 		.int_reset_reg = BMA180_CTRL_REG0,
 		.int_reset_mask = BMA180_RESET_INT,
 		.sleep_reg = BMA180_CTRL_REG0,
@@ -656,12 +697,14 @@ static const struct bma180_part_info bma180_part_info[] = {
 		.chip_disable = bma180_chip_disable,
 	},
 	[BMA250] = {
+		.chip_id = BMA250_ID_REG_VAL,
 		.channels = bma250_channels,
 		.num_channels = ARRAY_SIZE(bma250_channels),
-		.scale_table = bma250_scale_table,
-		.num_scales = ARRAY_SIZE(bma250_scale_table),
-		.bw_table = bma250_bw_table,
-		.num_bw = ARRAY_SIZE(bma250_bw_table),
+		.scale_table = bma25x_scale_table,
+		.num_scales = ARRAY_SIZE(bma25x_scale_table),
+		.bw_table = bma25x_bw_table,
+		.num_bw = ARRAY_SIZE(bma25x_bw_table),
+		.center_temp = 48, /* 0 LSB @ 24 degree C */
 		.int_reset_reg = BMA250_INT_RESET_REG,
 		.int_reset_mask = BMA250_INT_RESET_MASK,
 		.sleep_reg = BMA250_POWER_REG,
@@ -675,9 +718,39 @@ static const struct bma180_part_info bma180_part_info[] = {
 		.lowpower_val = 1,
 		.int_enable_reg = BMA250_INT_ENABLE_REG,
 		.int_enable_mask = BMA250_DATA_INTEN_MASK,
+		.int_map_reg = BMA250_INT_MAP_REG,
+		.int_enable_dataready_int1_mask = BMA250_INT1_DATA_MASK,
 		.softreset_reg = BMA250_RESET_REG,
-		.chip_config = bma250_chip_config,
-		.chip_disable = bma250_chip_disable,
+		.chip_config = bma25x_chip_config,
+		.chip_disable = bma25x_chip_disable,
+	},
+	[BMA254] = {
+		.chip_id = BMA254_ID_REG_VAL,
+		.channels = bma254_channels,
+		.num_channels = ARRAY_SIZE(bma254_channels),
+		.scale_table = bma25x_scale_table,
+		.num_scales = ARRAY_SIZE(bma25x_scale_table),
+		.bw_table = bma25x_bw_table,
+		.num_bw = ARRAY_SIZE(bma25x_bw_table),
+		.center_temp = 46, /* 0 LSB @ 23 degree C */
+		.int_reset_reg = BMA254_INT_RESET_REG,
+		.int_reset_mask = BMA254_INT_RESET_MASK,
+		.sleep_reg = BMA254_POWER_REG,
+		.sleep_mask = BMA254_SUSPEND_MASK,
+		.bw_reg = BMA254_BW_REG,
+		.bw_mask = BMA254_BW_MASK,
+		.scale_reg = BMA254_RANGE_REG,
+		.scale_mask = BMA254_RANGE_MASK,
+		.power_reg = BMA254_POWER_REG,
+		.power_mask = BMA254_LOWPOWER_MASK,
+		.lowpower_val = 1,
+		.int_enable_reg = BMA254_INT_ENABLE_REG,
+		.int_enable_mask = BMA254_DATA_INTEN_MASK,
+		.int_map_reg = BMA254_INT_MAP_REG,
+		.int_enable_dataready_int1_mask = BMA254_INT1_DATA_MASK,
+		.softreset_reg = BMA254_RESET_REG,
+		.chip_config = bma25x_chip_config,
+		.chip_disable = bma25x_chip_disable,
 	},
 };
 
@@ -918,6 +991,7 @@ static SIMPLE_DEV_PM_OPS(bma180_pm_ops, bma180_suspend, bma180_resume);
 static const struct i2c_device_id bma180_ids[] = {
 	{ "bma180", BMA180 },
 	{ "bma250", BMA250 },
+	{ "bma254", BMA254 },
 	{ }
 };
 
@@ -932,6 +1006,10 @@ static const struct of_device_id bma180_of_match[] = {
 		.compatible = "bosch,bma250",
 		.data = (void *)BMA250
 	},
+	{
+		.compatible = "bosch,bma254",
+		.data = (void *)BMA254
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, bma180_of_match);
@@ -951,5 +1029,5 @@ module_i2c_driver(bma180_driver);
 
 MODULE_AUTHOR("Kravchenko Oleksandr <x0199363@ti.com>");
 MODULE_AUTHOR("Texas Instruments, Inc.");
-MODULE_DESCRIPTION("Bosch BMA180/BMA250 triaxial acceleration sensor");
+MODULE_DESCRIPTION("Bosch BMA180/BMA25x triaxial acceleration sensor");
 MODULE_LICENSE("GPL");
-- 
2.21.0

Re: [PATCH 4/4] iio: accel: bma180: BMA254 support

[Rob Herring]

On Wed, 11 Dec 2019 22:38:19 +0100, Linus Walleij wrote:
> This adds support for the BMA254 variant of this
> accelerometer. The only difference for the simple IIO
> driver is that values are 12 bit and the temperature
> offset differs by 1 degree.
> 
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  .../devicetree/bindings/iio/accel/bma180.txt  |   7 +-
>  drivers/iio/accel/Kconfig                     |   4 +-
>  drivers/iio/accel/bma180.c                    | 108 +++++++++++++++---
>  3 files changed, 100 insertions(+), 19 deletions(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 1/4] iio: accel: bma180: Add dev helper variable

[Jonathan Cameron]

On Wed, 11 Dec 2019 22:38:16 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> Having a shorthand "dev" instead of &client->dev everywhere
> makes the code easier to read (more compact).
> 
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Seems sensible to me.

Applied to the togreg branch of iio.git and pushed out as testing for
the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/iio/accel/bma180.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index 1574e4604a4f..518efbe4eaf6 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -712,12 +712,13 @@ static const struct iio_trigger_ops bma180_trigger_ops = {
>  static int bma180_probe(struct i2c_client *client,
>  		const struct i2c_device_id *id)
>  {
> +	struct device *dev = &client->dev;
>  	struct bma180_data *data;
>  	struct iio_dev *indio_dev;
>  	enum chip_ids chip;
>  	int ret;
>  
> -	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>  	if (!indio_dev)
>  		return -ENOMEM;
>  
> @@ -725,12 +726,12 @@ static int bma180_probe(struct i2c_client *client,
>  	i2c_set_clientdata(client, indio_dev);
>  	data->client = client;
>  	if (client->dev.of_node)
> -		chip = (enum chip_ids)of_device_get_match_data(&client->dev);
> +		chip = (enum chip_ids)of_device_get_match_data(dev);
>  	else
>  		chip = id->driver_data;
>  	data->part_info = &bma180_part_info[chip];
>  
> -	ret = iio_read_mount_matrix(&client->dev, "mount-matrix",
> +	ret = iio_read_mount_matrix(dev, "mount-matrix",
>  				&data->orientation);
>  	if (ret)
>  		return ret;
> @@ -740,7 +741,7 @@ static int bma180_probe(struct i2c_client *client,
>  		goto err_chip_disable;
>  
>  	mutex_init(&data->mutex);
> -	indio_dev->dev.parent = &client->dev;
> +	indio_dev->dev.parent = dev;
>  	indio_dev->channels = data->part_info->channels;
>  	indio_dev->num_channels = data->part_info->num_channels;
>  	indio_dev->name = id->name;
> @@ -755,15 +756,15 @@ static int bma180_probe(struct i2c_client *client,
>  			goto err_chip_disable;
>  		}
>  
> -		ret = devm_request_irq(&client->dev, client->irq,
> +		ret = devm_request_irq(dev, client->irq,
>  			iio_trigger_generic_data_rdy_poll, IRQF_TRIGGER_RISING,
>  			"bma180_event", data->trig);
>  		if (ret) {
> -			dev_err(&client->dev, "unable to request IRQ\n");
> +			dev_err(dev, "unable to request IRQ\n");
>  			goto err_trigger_free;
>  		}
>  
> -		data->trig->dev.parent = &client->dev;
> +		data->trig->dev.parent = dev;
>  		data->trig->ops = &bma180_trigger_ops;
>  		iio_trigger_set_drvdata(data->trig, indio_dev);
>  		indio_dev->trig = iio_trigger_get(data->trig);
> @@ -776,13 +777,13 @@ static int bma180_probe(struct i2c_client *client,
>  	ret = iio_triggered_buffer_setup(indio_dev, NULL,
>  			bma180_trigger_handler, NULL);
>  	if (ret < 0) {
> -		dev_err(&client->dev, "unable to setup iio triggered buffer\n");
> +		dev_err(dev, "unable to setup iio triggered buffer\n");
>  		goto err_trigger_unregister;
>  	}
>  
>  	ret = iio_device_register(indio_dev);
>  	if (ret < 0) {
> -		dev_err(&client->dev, "unable to register iio device\n");
> +		dev_err(dev, "unable to register iio device\n");
>  		goto err_buffer_cleanup;
>  	}
>  

Re: [PATCH 2/4] iio: accel: bma180: Basic regulator support

[Jonathan Cameron]

On Wed, 11 Dec 2019 22:38:17 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> This brings up the VDD and VDDIO regulators using the
> regulator framework. Platforms that do not use regulators
> will provide stubs or dummy regulators.
> 
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
I made a totally trivial tweak. See below.

Applied to the togreg branch of iio.git and pushed out as testing..

etc. etc.

Jonathan

> ---
>  drivers/iio/accel/bma180.c | 43 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index 518efbe4eaf6..4a619b5a544a 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_device.h>
>  #include <linux/of.h>
>  #include <linux/bitops.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/iio/iio.h>
> @@ -110,6 +111,8 @@ struct bma180_part_info {
>  #define BMA250_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
>  
>  struct bma180_data {
> +	struct regulator *vdd_supply;
> +	struct regulator *vddio_supply;
>  	struct i2c_client *client;
>  	struct iio_trigger *trig;
>  	const struct bma180_part_info *part_info;
> @@ -736,6 +739,40 @@ static int bma180_probe(struct i2c_client *client,
>  	if (ret)
>  		return ret;
>  
> +	data->vdd_supply = devm_regulator_get(dev, "vdd");
> +	if (IS_ERR(data->vdd_supply)) {
> +		if (PTR_ERR(data->vdd_supply) != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get vdd regulator %d\n",
> +				(int)PTR_ERR(data->vdd_supply));
> +		return PTR_ERR(data->vdd_supply);
> +	}
> +	data->vddio_supply = devm_regulator_get(dev, "vddio");
> +	if (IS_ERR(data->vddio_supply)) {
> +		if (PTR_ERR(data->vddio_supply) != -EPROBE_DEFER)
> +			dev_err(dev, "Failed to get vddio regulator %d\n",
> +				(int)PTR_ERR(data->vddio_supply));
> +		return PTR_ERR(data->vddio_supply);
> +	}
> +	/* Typical voltage 2.4V these are min and max */
> +	ret = regulator_set_voltage(data->vdd_supply, 1620000, 3600000);
> +	if (ret)
> +		return ret;
> +	ret = regulator_set_voltage(data->vddio_supply, 1200000, 3600000);
> +	if (ret)
> +		return ret;
> +	ret = regulator_enable(data->vdd_supply);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable vdd regulator: %d\n", ret);
> +		return ret;
> +	}
> +	ret = regulator_enable(data->vddio_supply);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable vddio regulator: %d\n", ret);
> +		goto err_disable_vdd;
> +	}
> +	/* Wait to make sure we started up properly (3 ms at least) */
> +	usleep_range(3000, 5000);
> +
>  	ret = data->part_info->chip_config(data);
>  	if (ret < 0)
>  		goto err_chip_disable;
> @@ -798,7 +835,9 @@ static int bma180_probe(struct i2c_client *client,
>  	iio_trigger_free(data->trig);
>  err_chip_disable:
>  	data->part_info->chip_disable(data);
> -
If I were feeling really nit pick friendly.  Don't mess with the whitespace!

> +	regulator_disable(data->vddio_supply);
> +err_disable_vdd:
> +	regulator_disable(data->vdd_supply);
>  	return ret;
>  }
>  
> @@ -817,6 +856,8 @@ static int bma180_remove(struct i2c_client *client)
>  	mutex_lock(&data->mutex);
>  	data->part_info->chip_disable(data);
>  	mutex_unlock(&data->mutex);
> +	regulator_disable(data->vddio_supply);
> +	regulator_disable(data->vdd_supply);
>  
>  	return 0;
>  }

Re: [PATCH 3/4] iio: accel: bma180: Use explicit member assignment

[Jonathan Cameron]

On Wed, 11 Dec 2019 22:38:18 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> This uses the C99 explicit .member assignment for the
> variant data in struct bma180_part_info. This makes it
> easier to understand and add new variants.
> 
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

This I like.  Sensible improvement to readability.
Not sure why I let it through in the form it was in originally.
oh well.

Applied,

Thanks,

Jonathan

> ---
>  drivers/iio/accel/bma180.c | 68 ++++++++++++++++++++++++--------------
>  1 file changed, 44 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index 4a619b5a544a..f583f10ccbb9 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -632,32 +632,52 @@ static const struct iio_chan_spec bma250_channels[] = {
>  
>  static const struct bma180_part_info bma180_part_info[] = {
>  	[BMA180] = {
> -		bma180_channels, ARRAY_SIZE(bma180_channels),
> -		bma180_scale_table, ARRAY_SIZE(bma180_scale_table),
> -		bma180_bw_table, ARRAY_SIZE(bma180_bw_table),
> -		BMA180_CTRL_REG0, BMA180_RESET_INT,
> -		BMA180_CTRL_REG0, BMA180_SLEEP,
> -		BMA180_BW_TCS, BMA180_BW,
> -		BMA180_OFFSET_LSB1, BMA180_RANGE,
> -		BMA180_TCO_Z, BMA180_MODE_CONFIG, BMA180_LOW_POWER,
> -		BMA180_CTRL_REG3, BMA180_NEW_DATA_INT,
> -		BMA180_RESET,
> -		bma180_chip_config,
> -		bma180_chip_disable,
> +		.channels = bma180_channels,
> +		.num_channels = ARRAY_SIZE(bma180_channels),
> +		.scale_table = bma180_scale_table,
> +		.num_scales = ARRAY_SIZE(bma180_scale_table),
> +		.bw_table = bma180_bw_table,
> +		.num_bw = ARRAY_SIZE(bma180_bw_table),
> +		.int_reset_reg = BMA180_CTRL_REG0,
> +		.int_reset_mask = BMA180_RESET_INT,
> +		.sleep_reg = BMA180_CTRL_REG0,
> +		.sleep_mask = BMA180_SLEEP,
> +		.bw_reg = BMA180_BW_TCS,
> +		.bw_mask = BMA180_BW,
> +		.scale_reg = BMA180_OFFSET_LSB1,
> +		.scale_mask = BMA180_RANGE,
> +		.power_reg = BMA180_TCO_Z,
> +		.power_mask = BMA180_MODE_CONFIG,
> +		.lowpower_val = BMA180_LOW_POWER,
> +		.int_enable_reg = BMA180_CTRL_REG3,
> +		.int_enable_mask = BMA180_NEW_DATA_INT,
> +		.softreset_reg = BMA180_RESET,
> +		.chip_config = bma180_chip_config,
> +		.chip_disable = bma180_chip_disable,
>  	},
>  	[BMA250] = {
> -		bma250_channels, ARRAY_SIZE(bma250_channels),
> -		bma250_scale_table, ARRAY_SIZE(bma250_scale_table),
> -		bma250_bw_table, ARRAY_SIZE(bma250_bw_table),
> -		BMA250_INT_RESET_REG, BMA250_INT_RESET_MASK,
> -		BMA250_POWER_REG, BMA250_SUSPEND_MASK,
> -		BMA250_BW_REG, BMA250_BW_MASK,
> -		BMA250_RANGE_REG, BMA250_RANGE_MASK,
> -		BMA250_POWER_REG, BMA250_LOWPOWER_MASK, 1,
> -		BMA250_INT_ENABLE_REG, BMA250_DATA_INTEN_MASK,
> -		BMA250_RESET_REG,
> -		bma250_chip_config,
> -		bma250_chip_disable,
> +		.channels = bma250_channels,
> +		.num_channels = ARRAY_SIZE(bma250_channels),
> +		.scale_table = bma250_scale_table,
> +		.num_scales = ARRAY_SIZE(bma250_scale_table),
> +		.bw_table = bma250_bw_table,
> +		.num_bw = ARRAY_SIZE(bma250_bw_table),
> +		.int_reset_reg = BMA250_INT_RESET_REG,
> +		.int_reset_mask = BMA250_INT_RESET_MASK,
> +		.sleep_reg = BMA250_POWER_REG,
> +		.sleep_mask = BMA250_SUSPEND_MASK,
> +		.bw_reg = BMA250_BW_REG,
> +		.bw_mask = BMA250_BW_MASK,
> +		.scale_reg = BMA250_RANGE_REG,
> +		.scale_mask = BMA250_RANGE_MASK,
> +		.power_reg = BMA250_POWER_REG,
> +		.power_mask = BMA250_LOWPOWER_MASK,
> +		.lowpower_val = 1,
> +		.int_enable_reg = BMA250_INT_ENABLE_REG,
> +		.int_enable_mask = BMA250_DATA_INTEN_MASK,
> +		.softreset_reg = BMA250_RESET_REG,
> +		.chip_config = bma250_chip_config,
> +		.chip_disable = bma250_chip_disable,
>  	},
>  };
>  

Re: [PATCH 4/4] iio: accel: bma180: BMA254 support

[Jonathan Cameron]

On Wed, 11 Dec 2019 22:38:19 +0100
Linus Walleij <linus.walleij@linaro.org> wrote:

> This adds support for the BMA254 variant of this
> accelerometer. The only difference for the simple IIO
> driver is that values are 12 bit and the temperature
> offset differs by 1 degree.
> 
> Cc: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I'm never keen on wildcards, and there is a bma253.

So please go back to using explicit part numbers.
If you feel a need to indicate a particular structure
applies to multiple devices, then use a comment to do so.

I thought about doing this myself, but you might disagree :)

Jonathan

> ---
>  .../devicetree/bindings/iio/accel/bma180.txt  |   7 +-
>  drivers/iio/accel/Kconfig                     |   4 +-
>  drivers/iio/accel/bma180.c                    | 108 +++++++++++++++---
>  3 files changed, 100 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/bma180.txt b/Documentation/devicetree/bindings/iio/accel/bma180.txt
> index 3b25b4c4d446..f53237270b32 100644
> --- a/Documentation/devicetree/bindings/iio/accel/bma180.txt
> +++ b/Documentation/devicetree/bindings/iio/accel/bma180.txt
> @@ -1,11 +1,14 @@
> -* Bosch BMA180 / BMA250 triaxial acceleration sensor
> +* Bosch BMA180 / BMA25x triaxial acceleration sensor
>  
>  http://omapworld.com/BMA180_111_1002839.pdf
>  http://ae-bst.resource.bosch.com/media/products/dokumente/bma250/bst-bma250-ds002-05.pdf
>  
>  Required properties:
>  
> -  - compatible : should be "bosch,bma180" or "bosch,bma250"
> +  - compatible : should be one of:
> +    "bosch,bma180"
> +    "bosch,bma250"
> +    "bosch,bma254"
>    - reg : the I2C address of the sensor
>  
>  Optional properties:
> diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
> index d4ef35aeb579..2f10fd7954fc 100644
> --- a/drivers/iio/accel/Kconfig
> +++ b/drivers/iio/accel/Kconfig
> @@ -89,13 +89,13 @@ config ADXL372_I2C
>  	  module will be called adxl372_i2c.
>  
>  config BMA180
> -	tristate "Bosch BMA180/BMA250 3-Axis Accelerometer Driver"
> +	tristate "Bosch BMA180/BMA25x 3-Axis Accelerometer Driver"
>  	depends on I2C
>  	select IIO_BUFFER
>  	select IIO_TRIGGERED_BUFFER
>  	help
>  	  Say Y here if you want to build a driver for the Bosch BMA180 or
> -	  BMA250 triaxial acceleration sensor.
> +	  BMA25x triaxial acceleration sensor.
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called bma180.
> diff --git a/drivers/iio/accel/bma180.c b/drivers/iio/accel/bma180.c
> index f583f10ccbb9..952b2918525b 100644
> --- a/drivers/iio/accel/bma180.c
> +++ b/drivers/iio/accel/bma180.c
> @@ -9,6 +9,7 @@
>   * SPI is not supported by driver
>   * BMA180: 7-bit I2C slave address 0x40 or 0x41
>   * BMA250: 7-bit I2C slave address 0x18 or 0x19
> + * BMA254: 7-bit I2C slave address 0x18 or 0x19
>   */
>  
>  #include <linux/module.h>
> @@ -34,17 +35,20 @@
>  enum chip_ids {
>  	BMA180,
>  	BMA250,
> +	BMA254,
>  };
>  
>  struct bma180_data;
>  
>  struct bma180_part_info {
> +	u8 chip_id;
>  	const struct iio_chan_spec *channels;
>  	unsigned int num_channels;
>  	const int *scale_table;
>  	unsigned int num_scales;
>  	const int *bw_table;
>  	unsigned int num_bw;
> +	int center_temp;
>  
>  	u8 int_reset_reg, int_reset_mask;
>  	u8 sleep_reg, sleep_mask;
> @@ -52,6 +56,7 @@ struct bma180_part_info {
>  	u8 scale_reg, scale_mask;
>  	u8 power_reg, power_mask, lowpower_val;
>  	u8 int_enable_reg, int_enable_mask;
> +	u8 int_map_reg, int_enable_dataready_int1_mask;
>  	u8 softreset_reg;
>  
>  	int (*chip_config)(struct bma180_data *data);
> @@ -90,6 +95,8 @@ struct bma180_part_info {
>  #define BMA180_RESET_VAL	0xb6
>  
>  #define BMA180_ID_REG_VAL	0x03
> +#define BMA250_ID_REG_VAL	0x03
> +#define BMA254_ID_REG_VAL	0xfa /* 250 decimal */
>  
>  /* Chip power modes */
>  #define BMA180_LOW_POWER	0x03
> @@ -110,6 +117,23 @@ struct bma180_part_info {
>  #define BMA250_INT1_DATA_MASK	BIT(0)
>  #define BMA250_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
>  
> +#define BMA254_RANGE_REG	0x0f
> +#define BMA254_BW_REG		0x10
> +#define BMA254_POWER_REG	0x11
> +#define BMA254_RESET_REG	0x14
> +#define BMA254_INT_ENABLE_REG	0x17
> +#define BMA254_INT_MAP_REG	0x1a
> +#define BMA254_INT_RESET_REG	0x21
> +
> +#define BMA254_RANGE_MASK	GENMASK(3, 0) /* Range of accel values */
> +#define BMA254_BW_MASK		GENMASK(4, 0) /* Accel bandwidth */
> +#define BMA254_SUSPEND_MASK	BIT(7) /* chip will sleep */
> +#define BMA254_LOWPOWER_MASK	BIT(6)
> +#define BMA254_DATA_INTEN_MASK	BIT(4)
> +#define BMA254_INT2_DATA_MASK	BIT(7)
> +#define BMA254_INT1_DATA_MASK	BIT(0)
> +#define BMA254_INT_RESET_MASK	BIT(7) /* Reset pending interrupts */
> +
>  struct bma180_data {
>  	struct regulator *vdd_supply;
>  	struct regulator *vddio_supply;
> @@ -135,8 +159,8 @@ enum bma180_chan {
>  static int bma180_bw_table[] = { 10, 20, 40, 75, 150, 300 }; /* Hz */
>  static int bma180_scale_table[] = { 1275, 1863, 2452, 3727, 4903, 9709, 19417 };
>  
> -static int bma250_bw_table[] = { 8, 16, 31, 63, 125, 250 }; /* Hz */
> -static int bma250_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
> +static int bma25x_bw_table[] = { 8, 16, 31, 63, 125, 250 }; /* Hz */
> +static int bma25x_scale_table[] = { 0, 0, 0, 38344, 0, 76590, 0, 0, 153180, 0,
>  	0, 0, 306458 };
>  
>  static int bma180_get_data_reg(struct bma180_data *data, enum bma180_chan chan)
> @@ -310,8 +334,11 @@ static int bma180_chip_init(struct bma180_data *data)
>  
>  	if (ret < 0)
>  		return ret;
> -	if (ret != BMA180_ID_REG_VAL)
> +	if (ret != data->part_info->chip_id) {
> +		dev_err(&data->client->dev, "wrong chip ID %d expected %d\n",
> +			ret, data->part_info->chip_id);
>  		return -ENODEV;
> +	}
>  
>  	ret = bma180_soft_reset(data);
>  	if (ret)
> @@ -358,7 +385,7 @@ static int bma180_chip_config(struct bma180_data *data)
>  	return ret;
>  }
>  
> -static int bma250_chip_config(struct bma180_data *data)
> +static int bma25x_chip_config(struct bma180_data *data)
>  {
>  	int ret = bma180_chip_init(data);
>  
> @@ -370,8 +397,12 @@ static int bma250_chip_config(struct bma180_data *data)
>  	ret = bma180_set_scale(data, 38344); /* 2 G */
>  	if (ret)
>  		goto err;
> -	ret = bma180_set_bits(data, BMA250_INT_MAP_REG,
> -		BMA250_INT1_DATA_MASK, 1);
> +	/*
> +	 * This enables dataready interrupt on the INT1 pin
> +	 * FIXME: support using the INT2 pin
> +	 */
> +	ret = bma180_set_bits(data, data->part_info->int_map_reg,
> +		data->part_info->int_enable_dataready_int1_mask, 1);
>  	if (ret)
>  		goto err;
>  
> @@ -397,7 +428,7 @@ static void bma180_chip_disable(struct bma180_data *data)
>  	dev_err(&data->client->dev, "failed to disable the chip\n");
>  }
>  
> -static void bma250_chip_disable(struct bma180_data *data)
> +static void bma25x_chip_disable(struct bma180_data *data)
>  {
>  	if (bma180_set_new_data_intr_state(data, false))
>  		goto err;
> @@ -500,7 +531,7 @@ static int bma180_read_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  	case IIO_CHAN_INFO_OFFSET:
> -		*val = 48; /* 0 LSB @ 24 degree C */
> +		*val = data->part_info->center_temp;
>  		return IIO_VAL_INT;
>  	default:
>  		return -EINVAL;
> @@ -630,14 +661,24 @@ static const struct iio_chan_spec bma250_channels[] = {
>  	IIO_CHAN_SOFT_TIMESTAMP(4),
>  };
>  
> +static const struct iio_chan_spec bma254_channels[] = {
> +	BMA180_ACC_CHANNEL(X, 12),
> +	BMA180_ACC_CHANNEL(Y, 12),
> +	BMA180_ACC_CHANNEL(Z, 12),
> +	BMA180_TEMP_CHANNEL,
> +	IIO_CHAN_SOFT_TIMESTAMP(4),
> +};
> +
>  static const struct bma180_part_info bma180_part_info[] = {
>  	[BMA180] = {
> +		.chip_id = BMA180_ID_REG_VAL,
>  		.channels = bma180_channels,
>  		.num_channels = ARRAY_SIZE(bma180_channels),
>  		.scale_table = bma180_scale_table,
>  		.num_scales = ARRAY_SIZE(bma180_scale_table),
>  		.bw_table = bma180_bw_table,
>  		.num_bw = ARRAY_SIZE(bma180_bw_table),
> +		.center_temp = 48, /* 0 LSB @ 24 degree C */
>  		.int_reset_reg = BMA180_CTRL_REG0,
>  		.int_reset_mask = BMA180_RESET_INT,
>  		.sleep_reg = BMA180_CTRL_REG0,
> @@ -656,12 +697,14 @@ static const struct bma180_part_info bma180_part_info[] = {
>  		.chip_disable = bma180_chip_disable,
>  	},
>  	[BMA250] = {
> +		.chip_id = BMA250_ID_REG_VAL,
>  		.channels = bma250_channels,
>  		.num_channels = ARRAY_SIZE(bma250_channels),
> -		.scale_table = bma250_scale_table,
> -		.num_scales = ARRAY_SIZE(bma250_scale_table),
> -		.bw_table = bma250_bw_table,
> -		.num_bw = ARRAY_SIZE(bma250_bw_table),
> +		.scale_table = bma25x_scale_table,
> +		.num_scales = ARRAY_SIZE(bma25x_scale_table),
> +		.bw_table = bma25x_bw_table,
> +		.num_bw = ARRAY_SIZE(bma25x_bw_table),
> +		.center_temp = 48, /* 0 LSB @ 24 degree C */
>  		.int_reset_reg = BMA250_INT_RESET_REG,
>  		.int_reset_mask = BMA250_INT_RESET_MASK,
>  		.sleep_reg = BMA250_POWER_REG,
> @@ -675,9 +718,39 @@ static const struct bma180_part_info bma180_part_info[] = {
>  		.lowpower_val = 1,
>  		.int_enable_reg = BMA250_INT_ENABLE_REG,
>  		.int_enable_mask = BMA250_DATA_INTEN_MASK,
> +		.int_map_reg = BMA250_INT_MAP_REG,
> +		.int_enable_dataready_int1_mask = BMA250_INT1_DATA_MASK,
>  		.softreset_reg = BMA250_RESET_REG,
> -		.chip_config = bma250_chip_config,
> -		.chip_disable = bma250_chip_disable,
> +		.chip_config = bma25x_chip_config,
> +		.chip_disable = bma25x_chip_disable,
> +	},
> +	[BMA254] = {
> +		.chip_id = BMA254_ID_REG_VAL,
> +		.channels = bma254_channels,
> +		.num_channels = ARRAY_SIZE(bma254_channels),
> +		.scale_table = bma25x_scale_table,
> +		.num_scales = ARRAY_SIZE(bma25x_scale_table),
> +		.bw_table = bma25x_bw_table,
> +		.num_bw = ARRAY_SIZE(bma25x_bw_table),
> +		.center_temp = 46, /* 0 LSB @ 23 degree C */
> +		.int_reset_reg = BMA254_INT_RESET_REG,
> +		.int_reset_mask = BMA254_INT_RESET_MASK,
> +		.sleep_reg = BMA254_POWER_REG,
> +		.sleep_mask = BMA254_SUSPEND_MASK,
> +		.bw_reg = BMA254_BW_REG,
> +		.bw_mask = BMA254_BW_MASK,
> +		.scale_reg = BMA254_RANGE_REG,
> +		.scale_mask = BMA254_RANGE_MASK,
> +		.power_reg = BMA254_POWER_REG,
> +		.power_mask = BMA254_LOWPOWER_MASK,
> +		.lowpower_val = 1,
> +		.int_enable_reg = BMA254_INT_ENABLE_REG,
> +		.int_enable_mask = BMA254_DATA_INTEN_MASK,
> +		.int_map_reg = BMA254_INT_MAP_REG,
> +		.int_enable_dataready_int1_mask = BMA254_INT1_DATA_MASK,
> +		.softreset_reg = BMA254_RESET_REG,
> +		.chip_config = bma25x_chip_config,
> +		.chip_disable = bma25x_chip_disable,
>  	},
>  };
>  
> @@ -918,6 +991,7 @@ static SIMPLE_DEV_PM_OPS(bma180_pm_ops, bma180_suspend, bma180_resume);
>  static const struct i2c_device_id bma180_ids[] = {
>  	{ "bma180", BMA180 },
>  	{ "bma250", BMA250 },
> +	{ "bma254", BMA254 },
>  	{ }
>  };
>  
> @@ -932,6 +1006,10 @@ static const struct of_device_id bma180_of_match[] = {
>  		.compatible = "bosch,bma250",
>  		.data = (void *)BMA250
>  	},
> +	{
> +		.compatible = "bosch,bma254",
> +		.data = (void *)BMA254
> +	},
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, bma180_of_match);
> @@ -951,5 +1029,5 @@ module_i2c_driver(bma180_driver);
>  
>  MODULE_AUTHOR("Kravchenko Oleksandr <x0199363@ti.com>");
>  MODULE_AUTHOR("Texas Instruments, Inc.");
> -MODULE_DESCRIPTION("Bosch BMA180/BMA250 triaxial acceleration sensor");
> +MODULE_DESCRIPTION("Bosch BMA180/BMA25x triaxial acceleration sensor");
>  MODULE_LICENSE("GPL");

Re: [PATCH 4/4] iio: accel: bma180: BMA254 support

[Linus Walleij]

On Mon, Dec 23, 2019 at 6:28 PM Jonathan Cameron <jic23@kernel.org> wrote:
> On Wed, 11 Dec 2019 22:38:19 +0100
> Linus Walleij <linus.walleij@linaro.org> wrote:
>
> > This adds support for the BMA254 variant of this
> > accelerometer. The only difference for the simple IIO
> > driver is that values are 12 bit and the temperature
> > offset differs by 1 degree.
> >
> > Cc: Peter Meerwald <pmeerw@pmeerw.net>
> > Cc: Oleksandr Kravchenko <o.v.kravchenko@globallogic.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>
> I'm never keen on wildcards, and there is a bma253.
>
> So please go back to using explicit part numbers.
> If you feel a need to indicate a particular structure
> applies to multiple devices, then use a comment to do so.

Actually when writing the patch I had the datasheets for
BMA250, BMA253 and BMA255 at hand as well.

The cases where I have labeled variables "25x" is where the
models are identical, so as to make things easier for people
that want to add support for BMA253 and BMA255.

What about copy/paste the previous paragraph into the
commit message and call it a day?

Or do you prefer that I edit that comment into the driver
file and resend? Either works with me.

Yours,
Linus Walleij