Linux-IIO Archive on lore.kernel.org
 help / color / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Hartmut Knaack <knaack.h@gmx.de>,
	Lars-Peter Clausen <lars@metafoo.de>,
	Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove labels from probe
Date: Sat, 12 Oct 2019 14:37:22 +0100
Message-ID: <20191012143722.7cb7015d@archlinux> (raw)
In-Reply-To: <20191007024131.22708-1-brgl@bgdev.pl>

On Mon,  7 Oct 2019 04:41:31 +0200
Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> We can drop some duplicate code if we use devm_action for disabling
> regulators and pm and the managed variant of iio_device_register().
> 
> This allows us to completely remove all remove() callbacks from both
> i2c and spi code.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
This is on top of the bulk regulator patch which is awaiting precusors
getting to my upstream.  I'll hold this one as well on that.

If it looks like I've forgotten it then give me a poke.

Thanks,

Jonathan

> ---
> v1 -> v2:
> - squash the patches using devm_iio_device_register() and devm_action
>   to keep the changes bisectable
> 
>  drivers/iio/pressure/bmp280-core.c | 62 +++++++++++++++---------------
>  drivers/iio/pressure/bmp280-i2c.c  |  6 ---
>  drivers/iio/pressure/bmp280-spi.c  |  6 ---
>  drivers/iio/pressure/bmp280.h      |  1 -
>  4 files changed, 30 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iio/pressure/bmp280-core.c b/drivers/iio/pressure/bmp280-core.c
> index c2988dbdb1a7..79254dd26dfd 100644
> --- a/drivers/iio/pressure/bmp280-core.c
> +++ b/drivers/iio/pressure/bmp280-core.c
> @@ -984,6 +984,22 @@ static int bmp085_fetch_eoc_irq(struct device *dev,
>  	return 0;
>  }
>  
> +static void bmp280_pm_disable(void *data)
> +{
> +	struct device *dev = data;
> +
> +	pm_runtime_get_sync(dev);
> +	pm_runtime_put_noidle(dev);
> +	pm_runtime_disable(dev);
> +}
> +
> +static void bmp280_regulators_disable(void *data)
> +{
> +	struct regulator_bulk_data *supplies = data;
> +
> +	regulator_bulk_disable(BMP280_NUM_SUPPLIES, supplies);
> +}
> +
>  int bmp280_common_probe(struct device *dev,
>  			struct regmap *regmap,
>  			unsigned int chip,
> @@ -1055,6 +1071,11 @@ int bmp280_common_probe(struct device *dev,
>  		return ret;
>  	}
>  
> +	ret = devm_add_action_or_reset(dev, bmp280_regulators_disable,
> +				       data->supplies);
> +	if (ret)
> +		return ret;
> +
>  	/* Wait to make sure we started up properly */
>  	usleep_range(data->start_up_time, data->start_up_time + 100);
>  
> @@ -1069,17 +1090,16 @@ int bmp280_common_probe(struct device *dev,
>  	data->regmap = regmap;
>  	ret = regmap_read(regmap, BMP280_REG_ID, &chip_id);
>  	if (ret < 0)
> -		goto out_disable_regulators;
> +		return ret;
>  	if (chip_id != chip) {
>  		dev_err(dev, "bad chip id: expected %x got %x\n",
>  			chip, chip_id);
> -		ret = -EINVAL;
> -		goto out_disable_regulators;
> +		return -EINVAL;
>  	}
>  
>  	ret = data->chip_info->chip_config(data);
>  	if (ret < 0)
> -		goto out_disable_regulators;
> +		return ret;
>  
>  	dev_set_drvdata(dev, indio_dev);
>  
> @@ -1093,14 +1113,14 @@ int bmp280_common_probe(struct device *dev,
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");
> -			goto out_disable_regulators;
> +			return ret;
>  		}
>  	} else if (chip_id == BMP280_CHIP_ID || chip_id == BME280_CHIP_ID) {
>  		ret = bmp280_read_calib(data, &data->calib.bmp280, chip_id);
>  		if (ret < 0) {
>  			dev_err(data->dev,
>  				"failed to read calibration coefficients\n");
> -			goto out_disable_regulators;
> +			return ret;
>  		}
>  	}
>  
> @@ -1112,7 +1132,7 @@ int bmp280_common_probe(struct device *dev,
>  	if (irq > 0 || (chip_id  == BMP180_CHIP_ID)) {
>  		ret = bmp085_fetch_eoc_irq(dev, name, irq, data);
>  		if (ret)
> -			goto out_disable_regulators;
> +			return ret;
>  	}
>  
>  	/* Enable runtime PM */
> @@ -1127,36 +1147,14 @@ int bmp280_common_probe(struct device *dev,
>  	pm_runtime_use_autosuspend(dev);
>  	pm_runtime_put(dev);
>  
> -	ret = iio_device_register(indio_dev);
> +	ret = devm_add_action_or_reset(dev, bmp280_pm_disable, dev);
>  	if (ret)
> -		goto out_runtime_pm_disable;
> -
> -	return 0;
> +		return ret;
>  
> -out_runtime_pm_disable:
> -	pm_runtime_get_sync(data->dev);
> -	pm_runtime_put_noidle(data->dev);
> -	pm_runtime_disable(data->dev);
> -out_disable_regulators:
> -	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> -	return ret;
> +	return devm_iio_device_register(dev, indio_dev);
>  }
>  EXPORT_SYMBOL(bmp280_common_probe);
>  
> -int bmp280_common_remove(struct device *dev)
> -{
> -	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> -	struct bmp280_data *data = iio_priv(indio_dev);
> -
> -	iio_device_unregister(indio_dev);
> -	pm_runtime_get_sync(data->dev);
> -	pm_runtime_put_noidle(data->dev);
> -	pm_runtime_disable(data->dev);
> -	regulator_bulk_disable(BMP280_NUM_SUPPLIES, data->supplies);
> -	return 0;
> -}
> -EXPORT_SYMBOL(bmp280_common_remove);
> -
>  #ifdef CONFIG_PM
>  static int bmp280_runtime_suspend(struct device *dev)
>  {
> diff --git a/drivers/iio/pressure/bmp280-i2c.c b/drivers/iio/pressure/bmp280-i2c.c
> index acd9a3784fb4..3109c8e2cc11 100644
> --- a/drivers/iio/pressure/bmp280-i2c.c
> +++ b/drivers/iio/pressure/bmp280-i2c.c
> @@ -38,11 +38,6 @@ static int bmp280_i2c_probe(struct i2c_client *client,
>  				   client->irq);
>  }
>  
> -static int bmp280_i2c_remove(struct i2c_client *client)
> -{
> -	return bmp280_common_remove(&client->dev);
> -}
> -
>  static const struct acpi_device_id bmp280_acpi_i2c_match[] = {
>  	{"BMP0280", BMP280_CHIP_ID },
>  	{"BMP0180", BMP180_CHIP_ID },
> @@ -82,7 +77,6 @@ static struct i2c_driver bmp280_i2c_driver = {
>  		.pm = &bmp280_dev_pm_ops,
>  	},
>  	.probe		= bmp280_i2c_probe,
> -	.remove		= bmp280_i2c_remove,
>  	.id_table	= bmp280_i2c_id,
>  };
>  module_i2c_driver(bmp280_i2c_driver);
> diff --git a/drivers/iio/pressure/bmp280-spi.c b/drivers/iio/pressure/bmp280-spi.c
> index 9d57b7a3b134..625b86878ad8 100644
> --- a/drivers/iio/pressure/bmp280-spi.c
> +++ b/drivers/iio/pressure/bmp280-spi.c
> @@ -86,11 +86,6 @@ static int bmp280_spi_probe(struct spi_device *spi)
>  				   spi->irq);
>  }
>  
> -static int bmp280_spi_remove(struct spi_device *spi)
> -{
> -	return bmp280_common_remove(&spi->dev);
> -}
> -
>  static const struct of_device_id bmp280_of_spi_match[] = {
>  	{ .compatible = "bosch,bmp085", },
>  	{ .compatible = "bosch,bmp180", },
> @@ -118,7 +113,6 @@ static struct spi_driver bmp280_spi_driver = {
>  	},
>  	.id_table = bmp280_spi_id,
>  	.probe = bmp280_spi_probe,
> -	.remove = bmp280_spi_remove,
>  };
>  module_spi_driver(bmp280_spi_driver);
>  
> diff --git a/drivers/iio/pressure/bmp280.h b/drivers/iio/pressure/bmp280.h
> index eda50ef65706..57ba0e85db91 100644
> --- a/drivers/iio/pressure/bmp280.h
> +++ b/drivers/iio/pressure/bmp280.h
> @@ -112,7 +112,6 @@ int bmp280_common_probe(struct device *dev,
>  			unsigned int chip,
>  			const char *name,
>  			int irq);
> -int bmp280_common_remove(struct device *dev);
>  
>  /* PM ops */
>  extern const struct dev_pm_ops bmp280_dev_pm_ops;


  reply index

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-07  2:41 Bartosz Golaszewski
2019-10-12 13:37 ` Jonathan Cameron [this message]
2019-10-21 12:47   ` Bartosz Golaszewski
2019-10-22 10:15     ` Jonathan Cameron
2019-10-31  6:46       ` Bartosz Golaszewski
2019-11-02 14:22         ` Jonathan Cameron

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191012143722.7cb7015d@archlinux \
    --to=jic23@kernel.org \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=knaack.h@gmx.de \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmeerw@pmeerw.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org
	public-inbox-index linux-iio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git