| Message ID | 20240817144216.16569-1-aardelean@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: ad7606: split a 'ad7606_sw_mode_setup()' from probe | expand |
On Sat, 17 Aug 2024 17:42:15 +0300 Alexandru Ardelean <aardelean@baylibre.com> wrote: > This change moves the logic for setting up SW mode (during probe) into it's > own function. > > With the addition of some newer parts, the SW-mode part can get a little > more complicated. > So it's a bit better to have a separate function for this. This looks fine but put it on the start of the series that makes it more complex. I may pick it off that series before the bulk of the code but if it's sent in such a series the link tags etc will let people see 'why' it is a good idea. Jonathan > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > --- > drivers/iio/adc/ad7606.c | 43 ++++++++++++++++++++++++---------------- > 1 file changed, 26 insertions(+), 17 deletions(-) > > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c > index 68481e97e50a..192b9cd56b45 100644 > --- a/drivers/iio/adc/ad7606.c > +++ b/drivers/iio/adc/ad7606.c > @@ -545,6 +545,29 @@ static const struct iio_trigger_ops ad7606_trigger_ops = { > .validate_device = iio_trigger_validate_own_device, > }; > > +static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) > +{ > + struct ad7606_state *st = iio_priv(indio_dev); > + > + if (!st->bops->sw_mode_config) > + return 0; > + > + st->sw_mode_en = device_property_present(st->dev, "adi,sw-mode"); > + if (!st->sw_mode_en) > + return 0; > + > + indio_dev->info = &ad7606_info_os_range_and_debug; > + > + /* Scale of 0.076293 is only available in sw mode */ > + st->scale_avail = ad7616_sw_scale_avail; > + st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); > + > + /* After reset, in software mode, ±10 V is set by default */ > + memset32(st->range, 2, ARRAY_SIZE(st->range)); > + > + return st->bops->sw_mode_config(indio_dev); > +} > + > int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > const char *name, unsigned int id, > const struct ad7606_bus_ops *bops) > @@ -617,23 +640,9 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > st->write_scale = ad7606_write_scale_hw; > st->write_os = ad7606_write_os_hw; > > - if (st->bops->sw_mode_config) > - st->sw_mode_en = device_property_present(st->dev, > - "adi,sw-mode"); > - > - if (st->sw_mode_en) { > - /* Scale of 0.076293 is only available in sw mode */ > - st->scale_avail = ad7616_sw_scale_avail; > - st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); > - > - /* After reset, in software mode, ±10 V is set by default */ > - memset32(st->range, 2, ARRAY_SIZE(st->range)); > - indio_dev->info = &ad7606_info_os_range_and_debug; > - > - ret = st->bops->sw_mode_config(indio_dev); > - if (ret < 0) > - return ret; > - } > + ret = ad7606_sw_mode_setup(indio_dev); > + if (ret) > + return ret; > > st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", > indio_dev->name,
On Sat, Aug 17, 2024 at 5:58 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Sat, 17 Aug 2024 17:42:15 +0300 > Alexandru Ardelean <aardelean@baylibre.com> wrote: > > > This change moves the logic for setting up SW mode (during probe) into it's > > own function. > > > > With the addition of some newer parts, the SW-mode part can get a little > > more complicated. > > So it's a bit better to have a separate function for this. > This looks fine but put it on the start of the series that makes it more > complex. I may pick it off that series before the bulk of the code > but if it's sent in such a series the link tags etc will let people > see 'why' it is a good idea. Sure. Will spin-up the series. > > Jonathan > > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > --- > > drivers/iio/adc/ad7606.c | 43 ++++++++++++++++++++++++---------------- > > 1 file changed, 26 insertions(+), 17 deletions(-) > > > > diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c > > index 68481e97e50a..192b9cd56b45 100644 > > --- a/drivers/iio/adc/ad7606.c > > +++ b/drivers/iio/adc/ad7606.c > > @@ -545,6 +545,29 @@ static const struct iio_trigger_ops ad7606_trigger_ops = { > > .validate_device = iio_trigger_validate_own_device, > > }; > > > > +static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) > > +{ > > + struct ad7606_state *st = iio_priv(indio_dev); > > + > > + if (!st->bops->sw_mode_config) > > + return 0; > > + > > + st->sw_mode_en = device_property_present(st->dev, "adi,sw-mode"); > > + if (!st->sw_mode_en) > > + return 0; > > + > > + indio_dev->info = &ad7606_info_os_range_and_debug; > > + > > + /* Scale of 0.076293 is only available in sw mode */ > > + st->scale_avail = ad7616_sw_scale_avail; > > + st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); > > + > > + /* After reset, in software mode, ±10 V is set by default */ > > + memset32(st->range, 2, ARRAY_SIZE(st->range)); > > + > > + return st->bops->sw_mode_config(indio_dev); > > +} > > + > > int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > > const char *name, unsigned int id, > > const struct ad7606_bus_ops *bops) > > @@ -617,23 +640,9 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, > > st->write_scale = ad7606_write_scale_hw; > > st->write_os = ad7606_write_os_hw; > > > > - if (st->bops->sw_mode_config) > > - st->sw_mode_en = device_property_present(st->dev, > > - "adi,sw-mode"); > > - > > - if (st->sw_mode_en) { > > - /* Scale of 0.076293 is only available in sw mode */ > > - st->scale_avail = ad7616_sw_scale_avail; > > - st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); > > - > > - /* After reset, in software mode, ±10 V is set by default */ > > - memset32(st->range, 2, ARRAY_SIZE(st->range)); > > - indio_dev->info = &ad7606_info_os_range_and_debug; > > - > > - ret = st->bops->sw_mode_config(indio_dev); > > - if (ret < 0) > > - return ret; > > - } > > + ret = ad7606_sw_mode_setup(indio_dev); > > + if (ret) > > + return ret; > > > > st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", > > indio_dev->name, >
diff --git a/drivers/iio/adc/ad7606.c b/drivers/iio/adc/ad7606.c index 68481e97e50a..192b9cd56b45 100644 --- a/drivers/iio/adc/ad7606.c +++ b/drivers/iio/adc/ad7606.c @@ -545,6 +545,29 @@ static const struct iio_trigger_ops ad7606_trigger_ops = { .validate_device = iio_trigger_validate_own_device, }; +static int ad7606_sw_mode_setup(struct iio_dev *indio_dev) +{ + struct ad7606_state *st = iio_priv(indio_dev); + + if (!st->bops->sw_mode_config) + return 0; + + st->sw_mode_en = device_property_present(st->dev, "adi,sw-mode"); + if (!st->sw_mode_en) + return 0; + + indio_dev->info = &ad7606_info_os_range_and_debug; + + /* Scale of 0.076293 is only available in sw mode */ + st->scale_avail = ad7616_sw_scale_avail; + st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); + + /* After reset, in software mode, ±10 V is set by default */ + memset32(st->range, 2, ARRAY_SIZE(st->range)); + + return st->bops->sw_mode_config(indio_dev); +} + int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, const char *name, unsigned int id, const struct ad7606_bus_ops *bops) @@ -617,23 +640,9 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address, st->write_scale = ad7606_write_scale_hw; st->write_os = ad7606_write_os_hw; - if (st->bops->sw_mode_config) - st->sw_mode_en = device_property_present(st->dev, - "adi,sw-mode"); - - if (st->sw_mode_en) { - /* Scale of 0.076293 is only available in sw mode */ - st->scale_avail = ad7616_sw_scale_avail; - st->num_scales = ARRAY_SIZE(ad7616_sw_scale_avail); - - /* After reset, in software mode, ±10 V is set by default */ - memset32(st->range, 2, ARRAY_SIZE(st->range)); - indio_dev->info = &ad7606_info_os_range_and_debug; - - ret = st->bops->sw_mode_config(indio_dev); - if (ret < 0) - return ret; - } + ret = ad7606_sw_mode_setup(indio_dev); + if (ret) + return ret; st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
This change moves the logic for setting up SW mode (during probe) into it's own function. With the addition of some newer parts, the SW-mode part can get a little more complicated. So it's a bit better to have a separate function for this. Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> --- drivers/iio/adc/ad7606.c | 43 ++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 17 deletions(-)