lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191022111502.5097190e@archlinux>
Date:   Tue, 22 Oct 2019 11:15:02 +0100
From:   Jonathan Cameron <jic23@...nel.org>
To:     Bartosz Golaszewski <brgl@...ev.pl>
Cc:     Hartmut Knaack <knaack.h@....de>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Peter Meerwald-Stadler <pmeerw@...erw.net>,
        linux-iio@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>
Subject: Re: [PATCH v2] iio: pressure: bmp280: use devm action and remove
 labels from probe

On Mon, 21 Oct 2019 14:47:18 +0200
Bartosz Golaszewski <brgl@...ev.pl> wrote:

> sob., 12 paź 2019 o 15:37 Jonathan Cameron <jic23@...nel.org> napisał(a):
> >
> > On Mon,  7 Oct 2019 04:41:31 +0200
> > Bartosz Golaszewski <brgl@...ev.pl> wrote:
> >  
> > > From: Bartosz Golaszewski <bgolaszewski@...libre.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@...libre.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.
> >  
> 
> Hi Jonathan,
> 
> gentle poke after v5.4-rc4. Only one of the three patches is in next
> so far, the one using bulk regulators is missing too besides this one.
Thanks for the reminder.

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

Thanks,

Jonathan
> 
> Best regards,
> Bartosz Golaszewski
> 
> > 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;  
> >  

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ