diff mbox series

[v3,7/7] iio: pressure: bmp280: Add nvmem operations for BMP580

Message ID 013686653a60092227c842ae2fd0197f2509ad7c.1674954271.git.ang.iglesiasg@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series Add support for pressure sensor Bosch BMP580 | expand

Commit Message

Angel Iglesias Jan. 29, 2023, 1:33 a.m. UTC
The pressure sensor BMP580 contains a non-volatile memory that stores
trimming and configuration params. That memory provides an programmable
user range of three 2-byte words.

Signed-off-by: Angel Iglesias <ang.iglesiasg@gmail.com>

Comments

Andy Shevchenko Jan. 30, 2023, 12:29 p.m. UTC | #1
On Sun, Jan 29, 2023 at 02:33:09AM +0100, Angel Iglesias wrote:
> The pressure sensor BMP580 contains a non-volatile memory that stores
> trimming and configuration params. That memory provides an programmable
> user range of three 2-byte words.

...

> +#define NVM_READ	false
> +#define NVM_WRITE	true

How is it helpful and why it's not namespaced properly (can collide with
NVM framework)?

...

> +	/* Wait until NVM is ready again */
> +	do {
> +		ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
> +		if (ret) {
> +			dev_err(data->dev, "failed to check nvm status\n");
> +			reg &= ~BMP580_STATUS_NVM_RDY_MASK;
> +		}
> +	} while (time_before(jiffies, deadline) && !(reg & BMP580_STATUS_NVM_RDY_MASK));

regmap_read_poll_timeout()?

> +	if (!(reg & BMP580_STATUS_NVM_RDY_MASK)) {

	if (ret) {
		...
		return ret;
	}

> +		dev_err(data->dev,
> +			"reached timeout waiting for nvm operation completion\n");
> +		return -ETIMEDOUT;
> +	}

...

> +	while (bytes >= sizeof(u16)) {

sizeof(*dst) ?

Or sizeof(data->le16)?

> +		addr = bmp580_nvmem_addrs[offset / sizeof(u16)];

Ditto.

...

> +		bytes -= sizeof(u16);
> +		offset += sizeof(u16);

Ditto.

> +	}

...

> +static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
> +			      size_t bytes)
> +{

Same comments as per above function.

>  }
Angel Iglesias Jan. 31, 2023, 10:36 p.m. UTC | #2
On Mon, 2023-01-30 at 14:29 +0200, Andy Shevchenko wrote:
> On Sun, Jan 29, 2023 at 02:33:09AM +0100, Angel Iglesias wrote:
> > The pressure sensor BMP580 contains a non-volatile memory that stores
> > trimming and configuration params. That memory provides an programmable
> > user range of three 2-byte words.
> 
> ...
> 
> > +#define NVM_READ       false
> > +#define NVM_WRITE      true
> 
> How is it helpful and why it's not namespaced properly (can collide with
> NVM framework)?

I thought it could provide clarity using those aliases calling
bmp580_nvm_operation() helper function on nvmen callbacks, instead of using
just true or false. Not my brightest idea for sure.

> ...
> 
> > +       /* Wait until NVM is ready again */
> > +       do {
> > +               ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
> > +               if (ret) {
> > +                       dev_err(data->dev, "failed to check nvm status\n");
> > +                       reg &= ~BMP580_STATUS_NVM_RDY_MASK;
> > +               }
> > +       } while (time_before(jiffies, deadline) && !(reg &
> > BMP580_STATUS_NVM_RDY_MASK));
> 
> regmap_read_poll_timeout()?

Yes, thanks for pointing out, that's way better than the current loop.

> 
> > +       if (!(reg & BMP580_STATUS_NVM_RDY_MASK)) {
> 
>         if (ret) {
>                 ...
>                 return ret;
>         }
> 
> > +               dev_err(data->dev,
> > +                       "reached timeout waiting for nvm operation
> > completion\n");
> > +               return -ETIMEDOUT;
> > +       }
> 
> ...
> 
> > +       while (bytes >= sizeof(u16)) {
> 
> sizeof(*dst) ?
> 
> Or sizeof(data->le16)?
> 
> > +               addr = bmp580_nvmem_addrs[offset / sizeof(u16)];
> 
> Ditto.
> 
> ...
> 
> > +               bytes -= sizeof(u16);
> > +               offset += sizeof(u16);
> 
> Ditto.
> 
> > +       }
> 
> ...
> 
> > +static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
> > +                             size_t bytes)
> > +{
> 
> Same comments as per above function.

Got it!

> >  }
> 
> 

Thanks for your time,
Angel
diff mbox series

Patch

diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
index c65fb4025ad9..cc2811c9c6f4 100644
--- a/drivers/iio/pressure/bmp280-core.c
+++ b/drivers/iio/pressure/bmp280-core.c
@@ -28,6 +28,7 @@ 
 #include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/nvmem-provider.h>
 #include <linux/regmap.h>
 #include <linux/delay.h>
 #include <linux/iio/iio.h>
@@ -1290,6 +1291,88 @@  static int bmp580_soft_reset(struct bmp280_data *data)
 	return 0;
 }
 
+#define NVM_READ	false
+#define NVM_WRITE	true
+
+/**
+ * bmp580_nvm_operation() - Helper function to commit NVM memory operations
+ * @data: sensor data struct
+ * @is_write: flag to signal write operation
+ */
+static int bmp580_nvm_operation(struct bmp280_data *data, bool is_write)
+{
+	unsigned long deadline;
+	unsigned int reg;
+	int ret;
+
+	/* Check NVM ready flag */
+	ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
+	if (ret) {
+		dev_err(data->dev, "failed to check nvm status\n");
+		return ret;
+	}
+	if (!(reg & BMP580_STATUS_NVM_RDY_MASK)) {
+		dev_err(data->dev, "sensor's nvm is not ready\n");
+		return -EIO;
+	}
+
+	/* Send NVM operation sequence */
+	ret = regmap_write(data->regmap, BMP580_REG_CMD, BMP580_CMD_NVM_OP_SEQ_0);
+	if (ret) {
+		dev_err(data->dev, "failed to send nvm operation's first sequence\n");
+		return ret;
+	}
+	if (is_write) {
+		/* Send NVM write sequence */
+		ret = regmap_write(data->regmap, BMP580_REG_CMD,
+				   BMP580_CMD_NVM_WRITE_SEQ_1);
+		if (ret) {
+			dev_err(data->dev, "failed to send nvm write sequence\n");
+			return ret;
+		}
+		/* Datasheet says on 4.8.1.2 it takes approximately 10ms */
+		usleep_range(10000, 10500);
+		deadline = jiffies + msecs_to_jiffies(10);
+	} else {
+		/* Send NVM read sequence */
+		ret = regmap_write(data->regmap, BMP580_REG_CMD,
+				   BMP580_CMD_NVM_READ_SEQ_1);
+		if (ret) {
+			dev_err(data->dev, "failed to send nvm read sequence\n");
+			return ret;
+		}
+		/* Datasheet says on 4.8.1.1 it takes approximately 200us */
+		usleep_range(200, 250);
+		deadline = jiffies + usecs_to_jiffies(200);
+	}
+	if (ret) {
+		dev_err(data->dev, "failed to write command sequence\n");
+		return -EIO;
+	}
+
+	/* Wait until NVM is ready again */
+	do {
+		ret = regmap_read(data->regmap, BMP580_REG_STATUS, &reg);
+		if (ret) {
+			dev_err(data->dev, "failed to check nvm status\n");
+			reg &= ~BMP580_STATUS_NVM_RDY_MASK;
+		}
+	} while (time_before(jiffies, deadline) && !(reg & BMP580_STATUS_NVM_RDY_MASK));
+	if (!(reg & BMP580_STATUS_NVM_RDY_MASK)) {
+		dev_err(data->dev,
+			"reached timeout waiting for nvm operation completion\n");
+		return -ETIMEDOUT;
+	}
+
+	/* Check NVM error flags */
+	if ((reg & BMP580_STATUS_NVM_ERR_MASK) || (reg & BMP580_STATUS_NVM_CMD_ERR_MASK)) {
+		dev_err(data->dev, "error processing nvm operation\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
 /*
  * Contrary to previous sensors families, compensation algorithm is builtin.
  * We are only required to read the register raw data and adapt the ranges
@@ -1384,8 +1467,140 @@  static const int bmp580_odr_table[][2] = {
 	[BMP580_ODR_0_125HZ] =	{0, 125000},
 };
 
+const int bmp580_nvmem_addrs[] = { 0x20, 0x21, 0x22 };
+
+static int bmp580_nvmem_read(void *priv, unsigned int offset, void *val,
+			     size_t bytes)
+{
+	struct bmp280_data *data = priv;
+	u16 *dst = val;
+	int ret, addr;
+
+	pm_runtime_get_sync(data->dev);
+	mutex_lock(&data->lock);
+
+	/* Set sensor in standby mode */
+	ret = regmap_update_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+				 BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
+				 BMP580_ODR_DEEPSLEEP_DIS |
+				 FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
+	if (ret) {
+		dev_err(data->dev, "failed to change sensor to standby mode\n");
+		goto exit;
+	}
+	/* Wait standby transition time */
+	usleep_range(2500, 3000);
+
+	while (bytes >= sizeof(u16)) {
+		addr = bmp580_nvmem_addrs[offset / sizeof(u16)];
+
+		ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR,
+				   FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
+		if (ret) {
+			dev_err(data->dev, "error writing nvm address\n");
+			goto exit;
+		}
+
+		ret = bmp580_nvm_operation(data, NVM_READ);
+		if (ret)
+			goto exit;
+
+		ret = regmap_bulk_read(data->regmap, BMP580_REG_NVM_DATA_LSB, &data->le16,
+				       sizeof(data->le16));
+		if (ret) {
+			dev_err(data->dev, "error reading nvm data regs\n");
+			goto exit;
+		}
+
+		*dst++ = le16_to_cpu(data->le16);
+		bytes -= sizeof(u16);
+		offset += sizeof(u16);
+	}
+exit:
+	/* Restore chip config */
+	data->chip_info->chip_config(data);
+	mutex_unlock(&data->lock);
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+	return ret;
+}
+
+static int bmp580_nvmem_write(void *priv, unsigned int offset, void *val,
+			      size_t bytes)
+{
+	struct bmp280_data *data = priv;
+	u16 *buf = val;
+	int ret, addr;
+
+	pm_runtime_get_sync(data->dev);
+	mutex_lock(&data->lock);
+
+	/* Set sensor in standby mode */
+	ret = regmap_update_bits(data->regmap, BMP580_REG_ODR_CONFIG,
+				 BMP580_MODE_MASK | BMP580_ODR_DEEPSLEEP_DIS,
+				 BMP580_ODR_DEEPSLEEP_DIS |
+				 FIELD_PREP(BMP580_MODE_MASK, BMP580_MODE_SLEEP));
+	if (ret) {
+		dev_err(data->dev, "failed to change sensor to standby mode\n");
+		goto exit;
+	}
+	/* Wait standby transition time */
+	usleep_range(2500, 3000);
+
+	while (bytes >= sizeof(u16)) {
+		addr = bmp580_nvmem_addrs[offset / sizeof(u16)];
+
+		ret = regmap_write(data->regmap, BMP580_REG_NVM_ADDR, BMP580_NVM_PROG_EN |
+				   FIELD_PREP(BMP580_NVM_ROW_ADDR_MASK, addr));
+		if (ret) {
+			dev_err(data->dev, "error writing nvm address\n");
+			goto exit;
+		}
+		data->le16 = cpu_to_le16(*buf++);
+
+		ret = regmap_bulk_write(data->regmap, BMP580_REG_NVM_DATA_LSB, &data->le16,
+					sizeof(data->le16));
+		if (ret) {
+			dev_err(data->dev, "error writing LSB NVM data regs\n");
+			goto exit;
+		}
+
+		ret = bmp580_nvm_operation(data, NVM_WRITE);
+		if (ret)
+			goto exit;
+
+		/* Disable programming mode bit */
+		ret = regmap_update_bits(data->regmap, BMP580_REG_NVM_ADDR,
+					 BMP580_NVM_PROG_EN, 0);
+		if (ret) {
+			dev_err(data->dev, "error resetting nvm write\n");
+			goto exit;
+		}
+
+		bytes -= sizeof(u16);
+		offset += sizeof(u16);
+	}
+exit:
+	/* Restore chip config */
+	data->chip_info->chip_config(data);
+	mutex_unlock(&data->lock);
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+	return ret;
+}
+
 static int bmp580_preinit(struct bmp280_data *data)
 {
+	struct nvmem_config config = {
+		.dev = data->dev,
+		.priv = data,
+		.name = "bmp580_nvmem",
+		.word_size = sizeof(u16),
+		.stride = sizeof(u16),
+		.size = 3 * sizeof(u16),
+		.reg_read = bmp580_nvmem_read,
+		.reg_write = bmp580_nvmem_write,
+	};
 	unsigned int reg;
 	int ret;
 
@@ -1413,7 +1628,8 @@  static int bmp580_preinit(struct bmp280_data *data)
 		return -EIO;
 	}
 
-	return 0;
+	/* Register nvmem device */
+	return PTR_ERR_OR_ZERO(devm_nvmem_register(config.dev, &config));
 }
 
 static int bmp580_chip_config(struct bmp280_data *data)
diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
index f29d44b09efc..1c942b3dc384 100644
--- a/drivers/iio/pressure/bmp280.h
+++ b/drivers/iio/pressure/bmp280.h
@@ -104,6 +104,9 @@ 
 #define BMP580_FILTER_63X		6
 #define BMP580_FILTER_127X		7
 
+#define BMP580_NVM_ROW_ADDR_MASK	GENMASK(5, 0)
+#define BMP580_NVM_PROG_EN		BIT(6)
+
 #define BMP580_TEMP_SKIPPED		0x7f7f7f
 #define BMP580_PRESS_SKIPPED		0x7f7f7f