| Message ID | 20200522065616.10901-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: humidity: hts221: remove usage of iio_priv_to_dev() | expand |
On Fri, 22 May 2020 09:56:16 +0300 Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > We may want to get rid of the iio_priv_to_dev() helper. That's a bit > uncertain at this point. The reason is that we will hide some of the > members of the iio_dev structure (to prevent drivers from accessing them > directly), and that will also mean hiding the implementation of the > iio_priv_to_dev() helper inside the IIO core. > > Hiding the implementation of iio_priv_to_dev() implies that some fast-paths > may not be fast anymore, so a general idea is to try to get rid of the > iio_priv_to_dev() altogether. > > For this driver, removing the iio_priv_to_dev() helper means passing the > iio_dev object on hts221_allocate_buffers() & hts221_allocate_trigger(). > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> The hts221_hw structure is in iio_priv() so perhaps we could drop passing that into these two calls and get it from iio_priv within the functions? I tidied that up whilst applying. Shout if you disagree and I'll back it out :) Applied to the the togreg branch of iio.git and pushed out as testing for the autobuilders to play with it. Thanks, Jonathan > --- > drivers/iio/humidity/hts221.h | 4 ++-- > drivers/iio/humidity/hts221_buffer.c | 7 +++---- > drivers/iio/humidity/hts221_core.c | 4 ++-- > 3 files changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h > index 7d6771f7cf47..569146910885 100644 > --- a/drivers/iio/humidity/hts221.h > +++ b/drivers/iio/humidity/hts221.h > @@ -46,7 +46,7 @@ extern const struct dev_pm_ops hts221_pm_ops; > int hts221_probe(struct device *dev, int irq, const char *name, > struct regmap *regmap); > int hts221_set_enable(struct hts221_hw *hw, bool enable); > -int hts221_allocate_buffers(struct hts221_hw *hw); > -int hts221_allocate_trigger(struct hts221_hw *hw); > +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev); > +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev); > > #endif /* HTS221_H */ > diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c > index 9fb3f33614d4..48d469eeb0e6 100644 > --- a/drivers/iio/humidity/hts221_buffer.c > +++ b/drivers/iio/humidity/hts221_buffer.c > @@ -72,10 +72,9 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, void *private) > return IRQ_HANDLED; > } > > -int hts221_allocate_trigger(struct hts221_hw *hw) > +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev) > { > struct st_sensors_platform_data *pdata = dev_get_platdata(hw->dev); > - struct iio_dev *iio_dev = iio_priv_to_dev(hw); > bool irq_active_low = false, open_drain = false; > unsigned long irq_type; > int err; > @@ -190,9 +189,9 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, void *p) > return IRQ_HANDLED; > } > > -int hts221_allocate_buffers(struct hts221_hw *hw) > +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev) > { > - return devm_iio_triggered_buffer_setup(hw->dev, iio_priv_to_dev(hw), > + return devm_iio_triggered_buffer_setup(hw->dev, iio_dev, > NULL, hts221_buffer_handler_thread, > &hts221_buffer_ops); > } > diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c > index 9003671f14fb..77dfa65df841 100644 > --- a/drivers/iio/humidity/hts221_core.c > +++ b/drivers/iio/humidity/hts221_core.c > @@ -621,11 +621,11 @@ int hts221_probe(struct device *dev, int irq, const char *name, > } > > if (hw->irq > 0) { > - err = hts221_allocate_buffers(hw); > + err = hts221_allocate_buffers(hw, iio_dev); > if (err < 0) > return err; > > - err = hts221_allocate_trigger(hw); > + err = hts221_allocate_trigger(hw, iio_dev); > if (err) > return err; > }
On Sun, 2020-05-24 at 15:39 +0100, Jonathan Cameron wrote: > [External] > > On Fri, 22 May 2020 09:56:16 +0300 > Alexandru Ardelean <alexandru.ardelean@analog.com> wrote: > > > We may want to get rid of the iio_priv_to_dev() helper. That's a bit > > uncertain at this point. The reason is that we will hide some of the > > members of the iio_dev structure (to prevent drivers from accessing them > > directly), and that will also mean hiding the implementation of the > > iio_priv_to_dev() helper inside the IIO core. > > > > Hiding the implementation of iio_priv_to_dev() implies that some fast-paths > > may not be fast anymore, so a general idea is to try to get rid of the > > iio_priv_to_dev() altogether. > > > > For this driver, removing the iio_priv_to_dev() helper means passing the > > iio_dev object on hts221_allocate_buffers() & hts221_allocate_trigger(). > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > The hts221_hw structure is in iio_priv() so perhaps we could > drop passing that into these two calls and get it from iio_priv > within the functions? > > I tidied that up whilst applying. Shout if you disagree and I'll > back it out :) Fine by me. I usually go, for the lazy/narrow-view-path sometimes, when doing these changes. Which [in this case] was to get rid of iio_priv_to_dev() > > Applied to the the togreg branch of iio.git and pushed out as > testing for the autobuilders to play with it. > > Thanks, > > Jonathan > > > --- > > drivers/iio/humidity/hts221.h | 4 ++-- > > drivers/iio/humidity/hts221_buffer.c | 7 +++---- > > drivers/iio/humidity/hts221_core.c | 4 ++-- > > 3 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h > > index 7d6771f7cf47..569146910885 100644 > > --- a/drivers/iio/humidity/hts221.h > > +++ b/drivers/iio/humidity/hts221.h > > @@ -46,7 +46,7 @@ extern const struct dev_pm_ops hts221_pm_ops; > > int hts221_probe(struct device *dev, int irq, const char *name, > > struct regmap *regmap); > > int hts221_set_enable(struct hts221_hw *hw, bool enable); > > -int hts221_allocate_buffers(struct hts221_hw *hw); > > -int hts221_allocate_trigger(struct hts221_hw *hw); > > +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev); > > +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev); > > > > #endif /* HTS221_H */ > > diff --git a/drivers/iio/humidity/hts221_buffer.c > > b/drivers/iio/humidity/hts221_buffer.c > > index 9fb3f33614d4..48d469eeb0e6 100644 > > --- a/drivers/iio/humidity/hts221_buffer.c > > +++ b/drivers/iio/humidity/hts221_buffer.c > > @@ -72,10 +72,9 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, > > void *private) > > return IRQ_HANDLED; > > } > > > > -int hts221_allocate_trigger(struct hts221_hw *hw) > > +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev) > > { > > struct st_sensors_platform_data *pdata = dev_get_platdata(hw->dev); > > - struct iio_dev *iio_dev = iio_priv_to_dev(hw); > > bool irq_active_low = false, open_drain = false; > > unsigned long irq_type; > > int err; > > @@ -190,9 +189,9 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, > > void *p) > > return IRQ_HANDLED; > > } > > > > -int hts221_allocate_buffers(struct hts221_hw *hw) > > +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev) > > { > > - return devm_iio_triggered_buffer_setup(hw->dev, iio_priv_to_dev(hw), > > + return devm_iio_triggered_buffer_setup(hw->dev, iio_dev, > > NULL, hts221_buffer_handler_thread, > > &hts221_buffer_ops); > > } > > diff --git a/drivers/iio/humidity/hts221_core.c > > b/drivers/iio/humidity/hts221_core.c > > index 9003671f14fb..77dfa65df841 100644 > > --- a/drivers/iio/humidity/hts221_core.c > > +++ b/drivers/iio/humidity/hts221_core.c > > @@ -621,11 +621,11 @@ int hts221_probe(struct device *dev, int irq, const > > char *name, > > } > > > > if (hw->irq > 0) { > > - err = hts221_allocate_buffers(hw); > > + err = hts221_allocate_buffers(hw, iio_dev); > > if (err < 0) > > return err; > > > > - err = hts221_allocate_trigger(hw); > > + err = hts221_allocate_trigger(hw, iio_dev); > > if (err) > > return err; > > }
diff --git a/drivers/iio/humidity/hts221.h b/drivers/iio/humidity/hts221.h index 7d6771f7cf47..569146910885 100644 --- a/drivers/iio/humidity/hts221.h +++ b/drivers/iio/humidity/hts221.h @@ -46,7 +46,7 @@ extern const struct dev_pm_ops hts221_pm_ops; int hts221_probe(struct device *dev, int irq, const char *name, struct regmap *regmap); int hts221_set_enable(struct hts221_hw *hw, bool enable); -int hts221_allocate_buffers(struct hts221_hw *hw); -int hts221_allocate_trigger(struct hts221_hw *hw); +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev); +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev); #endif /* HTS221_H */ diff --git a/drivers/iio/humidity/hts221_buffer.c b/drivers/iio/humidity/hts221_buffer.c index 9fb3f33614d4..48d469eeb0e6 100644 --- a/drivers/iio/humidity/hts221_buffer.c +++ b/drivers/iio/humidity/hts221_buffer.c @@ -72,10 +72,9 @@ static irqreturn_t hts221_trigger_handler_thread(int irq, void *private) return IRQ_HANDLED; } -int hts221_allocate_trigger(struct hts221_hw *hw) +int hts221_allocate_trigger(struct hts221_hw *hw, struct iio_dev *iio_dev) { struct st_sensors_platform_data *pdata = dev_get_platdata(hw->dev); - struct iio_dev *iio_dev = iio_priv_to_dev(hw); bool irq_active_low = false, open_drain = false; unsigned long irq_type; int err; @@ -190,9 +189,9 @@ static irqreturn_t hts221_buffer_handler_thread(int irq, void *p) return IRQ_HANDLED; } -int hts221_allocate_buffers(struct hts221_hw *hw) +int hts221_allocate_buffers(struct hts221_hw *hw, struct iio_dev *iio_dev) { - return devm_iio_triggered_buffer_setup(hw->dev, iio_priv_to_dev(hw), + return devm_iio_triggered_buffer_setup(hw->dev, iio_dev, NULL, hts221_buffer_handler_thread, &hts221_buffer_ops); } diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c index 9003671f14fb..77dfa65df841 100644 --- a/drivers/iio/humidity/hts221_core.c +++ b/drivers/iio/humidity/hts221_core.c @@ -621,11 +621,11 @@ int hts221_probe(struct device *dev, int irq, const char *name, } if (hw->irq > 0) { - err = hts221_allocate_buffers(hw); + err = hts221_allocate_buffers(hw, iio_dev); if (err < 0) return err; - err = hts221_allocate_trigger(hw); + err = hts221_allocate_trigger(hw, iio_dev); if (err) return err; }
We may want to get rid of the iio_priv_to_dev() helper. That's a bit uncertain at this point. The reason is that we will hide some of the members of the iio_dev structure (to prevent drivers from accessing them directly), and that will also mean hiding the implementation of the iio_priv_to_dev() helper inside the IIO core. Hiding the implementation of iio_priv_to_dev() implies that some fast-paths may not be fast anymore, so a general idea is to try to get rid of the iio_priv_to_dev() altogether. For this driver, removing the iio_priv_to_dev() helper means passing the iio_dev object on hts221_allocate_buffers() & hts221_allocate_trigger(). Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/iio/humidity/hts221.h | 4 ++-- drivers/iio/humidity/hts221_buffer.c | 7 +++---- drivers/iio/humidity/hts221_core.c | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-)