Re: [PATCH 2/4] iio: magnetometer: bmc150: Add rudimentary regulator support

[Stephan Gerhold <stephan@gerhold.net>]

Hi Jonathan,

On Wed, Dec 02, 2020 at 10:33:20AM +0100, Stephan Gerhold wrote:
> BMC150 needs VDD and VDDIO regulators that might need to be explicitly
> enabled. Add some rudimentary support to obtain and enable these
> regulators during probe() and disable them during remove()
> or on the error path.
> 
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>

I think we forgot about the bmc150_magn part of this patch series. :)

This is likely my mistake - I only sent a v2 for the bmg160 part of this
patch series, but not for the bmc150_magn part (since it's not affected
by the problem you mentioned for the bmg160 patch). Sorry for the confusion!

Should I re-send the bmc150_magn part (with or without some changes)?

Thanks!
Stephan

> ---
> This is mostly copy-paste of
> 079c1c3f2082 ("iio: accel: bmc150-accel: Add rudimentary regulator support")
> from Linus Walleij but for the BMC150 magnetometer driver.
> ---
>  drivers/iio/magnetometer/bmc150_magn.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/magnetometer/bmc150_magn.c b/drivers/iio/magnetometer/bmc150_magn.c
> index fa09fcab620a..b2f3129e1b4f 100644
> --- a/drivers/iio/magnetometer/bmc150_magn.c
> +++ b/drivers/iio/magnetometer/bmc150_magn.c
> @@ -25,6 +25,7 @@
>  #include <linux/iio/trigger_consumer.h>
>  #include <linux/iio/triggered_buffer.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include "bmc150_magn.h"
>  
> @@ -135,6 +136,7 @@ struct bmc150_magn_data {
>  	 */
>  	struct mutex mutex;
>  	struct regmap *regmap;
> +	struct regulator_bulk_data regulators[2];
>  	struct iio_mount_matrix orientation;
>  	/* 4 x 32 bits for x, y z, 4 bytes align, 64 bits timestamp */
>  	s32 buffer[6];
> @@ -692,12 +694,24 @@ static int bmc150_magn_init(struct bmc150_magn_data *data)
>  	int ret, chip_id;
>  	struct bmc150_magn_preset preset;
>  
> +	ret = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
> +				    data->regulators);
> +	if (ret < 0) {
> +		dev_err(data->dev, "Failed to enable regulators: %d\n", ret);
> +		return ret;
> +	}
> +	/*
> +	 * 3ms power-on time according to datasheet, let's better
> +	 * be safe than sorry and set this delay to 5ms.
> +	 */
> +	msleep(5);
> +
>  	ret = bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND,
>  					 false);
>  	if (ret < 0) {
>  		dev_err(data->dev,
>  			"Failed to bring up device from suspend mode\n");
> -		return ret;
> +		goto err_regulator_disable;
>  	}
>  
>  	ret = regmap_read(data->regmap, BMC150_MAGN_REG_CHIP_ID, &chip_id);
> @@ -752,6 +766,8 @@ static int bmc150_magn_init(struct bmc150_magn_data *data)
>  
>  err_poweroff:
>  	bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);
> +err_regulator_disable:
> +	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
>  	return ret;
>  }
>  
> @@ -867,6 +883,13 @@ int bmc150_magn_probe(struct device *dev, struct regmap *regmap,
>  	data->irq = irq;
>  	data->dev = dev;
>  
> +	data->regulators[0].supply = "vdd";
> +	data->regulators[1].supply = "vddio";
> +	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(data->regulators),
> +				      data->regulators);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to get regulators\n");
> +
>  	ret = iio_read_mount_matrix(dev, "mount-matrix",
>  				&data->orientation);
>  	if (ret)
> @@ -984,6 +1007,7 @@ int bmc150_magn_remove(struct device *dev)
>  	bmc150_magn_set_power_mode(data, BMC150_MAGN_POWER_MODE_SUSPEND, true);
>  	mutex_unlock(&data->mutex);
>  
> +	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
>  	return 0;
>  }
>  EXPORT_SYMBOL(bmc150_magn_remove);
> -- 
> 2.29.2
>