iio: gyro: itg3200: add suspend/resume support.

Unless we put the device to sleep when not it use, it wastes 6mA.

If the device is asleep on probe, the 'id' register sometimes mis-reads - so reset first. If the device responds at all a command sent to the address, it is almost certainly the correct device already.

Signed-off-by NeilBrown <[email protected]>

drivers/iio/gyro/itg3200_core.c +31 -0

index 6a8020d48140..394667fb23f9 100644
@@ -223,6 +223,7 @@ static int itg3200_initial_setup(struct iio_dev *indio_dev)
 	int ret;
 	u8 val;
 
+	ret = itg3200_reset(indio_dev);
 	ret = itg3200_read_reg_8(indio_dev, ITG3200_REG_ADDRESS, &val);
 	if (ret)
 		goto err_ret;

@@ -351,6 +352,35 @@ static int itg3200_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int itg3200_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct itg3200 *st = iio_priv(indio_dev);
+	int ret;
+
+	dev_dbg(&st->i2c->dev, "suspend device");
+
+	ret = itg3200_write_reg_8(indio_dev,
+			ITG3200_REG_POWER_MANAGEMENT,
+			ITG3200_SLEEP);
+	return ret;
+}
+
+static int itg3200_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+	itg3200_reset(indio_dev);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(itg3200_pm_ops, itg3200_suspend, itg3200_resume);
+#define ITG3200_PM_OPS (&itg3200_pm_ops)
+#else
+#define ITG3200_PM_OPS NULL
+#endif
+
 static const struct i2c_device_id itg3200_id[] = {
 	{ "itg3200", 0 },
 	{ }

@@ -361,6 +391,7 @@ static struct i2c_driver itg3200_driver = {
 	.driver = {
 		.owner  = THIS_MODULE,
 		.name	= "itg3200",
+		.pm	= ITG3200_PM_OPS,
 	},
 	.id_table	= itg3200_id,
 	.probe		= itg3200_probe,

Quoted 10 lines · NeilBrown

Unless we put the device to sleep when not it use, it wastes 6mA.

If the device is asleep on probe, the 'id' register sometimes mis-reads - so reset first. If the device responds at all a command sent to the address, it is almost certainly the correct device already.

Quoted 1 line

Signed-off-by NeilBrown <[email protected]>

Hi Neil,

Looks fine to me - but you should have cc'd a few people from the listed driver authors. Will give them a few days to respond...

Jonathan

Quoted 58 lines

drivers/iio/gyro/itg3200_core.c +31 -0

index 6a8020d48140..394667fb23f9 100644
@@ -223,6 +223,7 @@ static int itg3200_initial_setup(struct iio_dev *indio_dev)
 	int ret;
 	u8 val;
 
+	ret = itg3200_reset(indio_dev);
 	ret = itg3200_read_reg_8(indio_dev, ITG3200_REG_ADDRESS, &val);
 	if (ret)
 		goto err_ret;

@@ -351,6 +352,35 @@ static int itg3200_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int itg3200_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct itg3200 *st = iio_priv(indio_dev);
+	int ret;
+
+	dev_dbg(&st->i2c->dev, "suspend device");
+
+	ret = itg3200_write_reg_8(indio_dev,
+			ITG3200_REG_POWER_MANAGEMENT,
+			ITG3200_SLEEP);
+	return ret;
+}
+
+static int itg3200_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+	itg3200_reset(indio_dev);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(itg3200_pm_ops, itg3200_suspend, itg3200_resume);
+#define ITG3200_PM_OPS (&itg3200_pm_ops)
+#else
+#define ITG3200_PM_OPS NULL
+#endif
+
 static const struct i2c_device_id itg3200_id[] = {
 	{ "itg3200", 0 },
 	{ }

@@ -361,6 +391,7 @@ static struct i2c_driver itg3200_driver = {
 	.driver = {
 		.owner  = THIS_MODULE,
 		.name	= "itg3200",
+		.pm	= ITG3200_PM_OPS,
 	},
 	.id_table	= itg3200_id,
 	.probe		= itg3200_probe,

Quoted 11 lines

Unless we put the device to sleep when not it use, it wastes 6mA.

If the device is asleep on probe, the 'id' register sometimes mis-reads - so reset first. If the device responds at all a command sent to the address, it is almost certainly the correct device already.

Signed-off-by NeilBrown <[email protected]>

Hi Neil,

Looks fine to me - but you should have cc'd a few people from the listed driver authors. Will give them a few days to respond...

Jonathan

Quoted 58 lines

drivers/iio/gyro/itg3200_core.c +31 -0

index 6a8020d48140..394667fb23f9 100644
@@ -223,6 +223,7 @@ static int itg3200_initial_setup(struct iio_dev *indio_dev)
 	int ret;
 	u8 val;
 
+	ret = itg3200_reset(indio_dev);
 	ret = itg3200_read_reg_8(indio_dev, ITG3200_REG_ADDRESS, &val);
 	if (ret)
 		goto err_ret;

@@ -351,6 +352,35 @@ static int itg3200_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int itg3200_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct itg3200 *st = iio_priv(indio_dev);
+	int ret;
+
+	dev_dbg(&st->i2c->dev, "suspend device");
+
+	ret = itg3200_write_reg_8(indio_dev,
+			ITG3200_REG_POWER_MANAGEMENT,
+			ITG3200_SLEEP);
+	return ret;
+}
+
+static int itg3200_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+	itg3200_reset(indio_dev);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(itg3200_pm_ops, itg3200_suspend, itg3200_resume);
+#define ITG3200_PM_OPS (&itg3200_pm_ops)
+#else
+#define ITG3200_PM_OPS NULL
+#endif
+
 static const struct i2c_device_id itg3200_id[] = {
 	{ "itg3200", 0 },
 	{ }

@@ -361,6 +391,7 @@ static struct i2c_driver itg3200_driver = {
 	.driver = {
 		.owner  = THIS_MODULE,
 		.name	= "itg3200",
+		.pm	= ITG3200_PM_OPS,
 	},
 	.id_table	= itg3200_id,
 	.probe		= itg3200_probe,

Unless we put the device to sleep when not it use, it wastes 6mA.

If the device is asleep on probe, the 'id' register sometimes mis-reads - so reset first. If the device responds at all a command sent to the address, it is almost certainly the correct device already.

drivers/iio/gyro/itg3200_core.c +3 -0

 	ret = itg3200_read_reg_8(indio_dev, ITG3200_REG_ADDRESS, &val);
 	if (ret)
 		goto err_ret;

@@ -351,6 +352,35 @@ static int itg3200_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int itg3200_suspend(struct device *dev)
+{

drivers/iio/gyro/itg3200_core.c +17 -0

+}
+
+static int itg3200_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+	itg3200_reset(indio_dev);
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(itg3200_pm_ops, itg3200_suspend, itg3200_resume);
+#define ITG3200_PM_OPS (&itg3200_pm_ops)
+#else
+#define ITG3200_PM_OPS NULL
+#endif
+
 static const struct i2c_device_id itg3200_id[] = {
 	{ "itg3200", 0 },
 	{ }

@@ -361,6 +391,7 @@ static struct i2c_driver itg3200_driver = {
 	.driver = {
 		.owner  = THIS_MODULE,
 		.name	= "itg3200",
+		.pm	= ITG3200_PM_OPS,
 	},
 	.id_table	= itg3200_id,
 	.probe		= itg3200_probe,

NeilBrown schrieb am 08.11.2014 01:18:

Quoted 10 lines

Unless we put the device to sleep when not it use, it wastes 6mA.

If the device is asleep on probe, the 'id' register sometimes mis-reads - so reset first. If the device responds at all a command sent to the address, it is almost certainly the correct device already.

Hi Neil,
I still have some question and issues, see inline.

drivers/iio/gyro/itg3200_core.c static int itg3200_initial_setup(struct iio_dev *indio_dev) · line 226

Quoted 11 lines

Signed-off-by NeilBrown <[email protected]>

drivers/iio/gyro/itg3200_core.c +1 -0

index 6a8020d48140..394667fb23f9 100644
@@ -223,6 +223,7 @@ static int itg3200_initial_setup(struct iio_dev *indio_dev)
 	int ret;
 	u8 val;
 
+	ret = itg3200_reset(indio_dev);

You should check possible error codes here. Also, there is still another reset issued some lines further down, although in between, there is only the register-read performed, which we see right below here - I would assume this wouldn't change anything in the device to require another reset. So, in conclusion, wouldn't it be sufficient to just move the reset part from further down up here?

Quoted 10 lines

drivers/iio/gyro/itg3200_core.c +3 -0

 	ret = itg3200_read_reg_8(indio_dev, ITG3200_REG_ADDRESS, &val);
 	if (ret)
 		goto err_ret;

@@ -351,6 +352,35 @@ static int itg3200_remove(struct i2c_client *client)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int itg3200_suspend(struct device *dev)
+{

drivers/iio/gyro/itg3200_core.c static int itg3200_remove(struct i2c_client *client) · line 367

Quoted 10 lines

drivers/iio/gyro/itg3200_core.c +10 -0

+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct itg3200 *st = iio_priv(indio_dev);
+	int ret;
+
+	dev_dbg(&st->i2c->dev, "suspend device");
+
+	ret = itg3200_write_reg_8(indio_dev,
+			ITG3200_REG_POWER_MANAGEMENT,
+			ITG3200_SLEEP);
+	return ret;

No need for ret, if you do it like this (fixing also some indentation issue):

	return itg3200_write_reg_8(indio_dev, ITG3200_REG_POWER_MANAGEMENT,
				   ITG3200_SLEEP);