| Message ID | a2dca9435f7f1f727c696a1faa0ab9e27927f9f3.1654118389.git.william.gray@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: Implement and utilize register structures for | expand |
On Mon, 6 Jun 2022 10:15:17 -0400 William Breathitt Gray <william.gray@linaro.org> wrote: > Reduce magic numbers and improve code readability by implementing and > utilizing named register data structures. > > Signed-off-by: William Breathitt Gray <william.gray@linaro.org> A few comments inline, but looks fine to me otherwise. Jonathan > --- > drivers/iio/adc/stx104.c | 70 +++++++++++++++++++++++++++------------- > 1 file changed, 47 insertions(+), 23 deletions(-) > > diff --git a/drivers/iio/adc/stx104.c b/drivers/iio/adc/stx104.c > index 7552351bfed9..7656b363e281 100644 > --- a/drivers/iio/adc/stx104.c > +++ b/drivers/iio/adc/stx104.c > @@ -16,6 +16,7 @@ > #include <linux/module.h> > #include <linux/moduleparam.h> > #include <linux/spinlock.h> > +#include <linux/types.h> > > #define STX104_OUT_CHAN(chan) { \ > .type = IIO_VOLTAGE, \ > @@ -44,14 +45,36 @@ static unsigned int num_stx104; > module_param_hw_array(base, uint, ioport, &num_stx104, 0); > MODULE_PARM_DESC(base, "Apex Embedded Systems STX104 base addresses"); > > +/** > + * struct stx104_reg - device register structure > + * @ad: ADC Data > + * @achan: ADC Channel > + * @dio: Digital I/O > + * @dac: DAC Channels > + * @cir_asr: Clear Interrupts and ADC Status > + * @acr: ADC Control > + * @pccr_fsh: Pacer Clock Control and FIFO Status MSB > + * @acfg: ADC Configuration > + */ > +struct stx104_reg { > + u16 ad; > + u8 achan; > + u8 dio; > + u16 dac[2]; > + u8 cir_asr; > + u8 acr; > + u8 pccr_fsh; > + u8 acfg; > +}; > + > /** > * struct stx104_iio - IIO device private data structure > * @chan_out_states: channels' output states > - * @base: base port address of the IIO device > + * @reg: I/O address offset for the device registers > */ > struct stx104_iio { > unsigned int chan_out_states[STX104_NUM_OUT_CHAN]; > - void __iomem *base; > + struct stx104_reg __iomem *reg; > }; > > /** > @@ -64,7 +87,7 @@ struct stx104_iio { > struct stx104_gpio { > struct gpio_chip chip; > spinlock_t lock; > - void __iomem *base; > + u8 __iomem *base; > unsigned int out_state; > }; > > @@ -72,6 +95,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > struct iio_chan_spec const *chan, int *val, int *val2, long mask) > { > struct stx104_iio *const priv = iio_priv(indio_dev); > + struct stx104_reg __iomem *const reg = priv->reg; > unsigned int adc_config; > int adbu; > int gain; > @@ -79,7 +103,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > switch (mask) { > case IIO_CHAN_INFO_HARDWAREGAIN: > /* get gain configuration */ > - adc_config = ioread8(priv->base + 11); > + adc_config = ioread8(®->acfg); > gain = adc_config & 0x3; > > *val = 1 << gain; > @@ -91,24 +115,24 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > } > > /* select ADC channel */ > - iowrite8(chan->channel | (chan->channel << 4), priv->base + 2); > + iowrite8(chan->channel | (chan->channel << 4), ®->achan); > > /* trigger ADC sample capture and wait for completion */ > - iowrite8(0, priv->base); > - while (ioread8(priv->base + 8) & BIT(7)); > + iowrite8(0, ®->ad); Curious - 8 bit write to a 16 bit address? Maybe worth a comment on why. > + while (ioread8(®->cir_asr) & BIT(7)); > > - *val = ioread16(priv->base); > + *val = ioread16(®->ad); > return IIO_VAL_INT; > case IIO_CHAN_INFO_OFFSET: > /* get ADC bipolar/unipolar configuration */ > - adc_config = ioread8(priv->base + 11); > + adc_config = ioread8(®->acfg); > adbu = !(adc_config & BIT(2)); > > *val = -32768 * adbu; > return IIO_VAL_INT; > case IIO_CHAN_INFO_SCALE: > /* get ADC bipolar/unipolar and gain configuration */ > - adc_config = ioread8(priv->base + 11); > + adc_config = ioread8(®->acfg); > adbu = !(adc_config & BIT(2)); > gain = adc_config & 0x3; > > @@ -130,16 +154,16 @@ static int stx104_write_raw(struct iio_dev *indio_dev, > /* Only four gain states (x1, x2, x4, x8) */ > switch (val) { > case 1: > - iowrite8(0, priv->base + 11); > + iowrite8(0, &priv->reg->acfg); > break; > case 2: > - iowrite8(1, priv->base + 11); > + iowrite8(1, &priv->reg->acfg); > break; > case 4: > - iowrite8(2, priv->base + 11); > + iowrite8(2, &priv->reg->acfg); > break; > case 8: > - iowrite8(3, priv->base + 11); > + iowrite8(3, &priv->reg->acfg); > break; > default: > return -EINVAL; > @@ -153,7 +177,7 @@ static int stx104_write_raw(struct iio_dev *indio_dev, > return -EINVAL; > > priv->chan_out_states[chan->channel] = val; > - iowrite16(val, priv->base + 4 + 2 * chan->channel); > + iowrite16(val, priv->reg->dac + chan->channel); Perhaps for consistency with below go with &priv->reg->dac[chan->channels]; > > return 0; > } > @@ -307,15 +331,15 @@ static int stx104_probe(struct device *dev, unsigned int id) > } > > priv = iio_priv(indio_dev); > - priv->base = devm_ioport_map(dev, base[id], STX104_EXTENT); > - if (!priv->base) > + priv->reg = devm_ioport_map(dev, base[id], STX104_EXTENT); > + if (!priv->reg) > return -ENOMEM; > > indio_dev->info = &stx104_info; > indio_dev->modes = INDIO_DIRECT_MODE; > > /* determine if differential inputs */ > - if (ioread8(priv->base + 8) & BIT(5)) { > + if (ioread8(&priv->reg->cir_asr) & BIT(5)) { > indio_dev->num_channels = ARRAY_SIZE(stx104_channels_diff); > indio_dev->channels = stx104_channels_diff; > } else { > @@ -326,14 +350,14 @@ static int stx104_probe(struct device *dev, unsigned int id) > indio_dev->name = dev_name(dev); > > /* configure device for software trigger operation */ > - iowrite8(0, priv->base + 9); > + iowrite8(0, &priv->reg->acr); > > /* initialize gain setting to x1 */ > - iowrite8(0, priv->base + 11); > + iowrite8(0, &priv->reg->acfg); > > /* initialize DAC output to 0V */ > - iowrite16(0, priv->base + 4); > - iowrite16(0, priv->base + 6); > + iowrite16(0, &priv->reg->dac[0]); > + iowrite16(0, &priv->reg->dac[1]); > > stx104gpio->chip.label = dev_name(dev); > stx104gpio->chip.parent = dev; > @@ -348,7 +372,7 @@ static int stx104_probe(struct device *dev, unsigned int id) > stx104gpio->chip.get_multiple = stx104_gpio_get_multiple; > stx104gpio->chip.set = stx104_gpio_set; > stx104gpio->chip.set_multiple = stx104_gpio_set_multiple; > - stx104gpio->base = priv->base + 3; > + stx104gpio->base = &priv->reg->dio; > stx104gpio->out_state = 0x0; > > spin_lock_init(&stx104gpio->lock);
On Tue, Jun 14, 2022 at 12:22:48PM +0100, Jonathan Cameron wrote: > On Mon, 6 Jun 2022 10:15:17 -0400 > William Breathitt Gray <william.gray@linaro.org> wrote: > > > Reduce magic numbers and improve code readability by implementing and > > utilizing named register data structures. > > > > Signed-off-by: William Breathitt Gray <william.gray@linaro.org> > > A few comments inline, but looks fine to me otherwise. > > Jonathan > > > --- > > drivers/iio/adc/stx104.c | 70 +++++++++++++++++++++++++++------------- > > 1 file changed, 47 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/iio/adc/stx104.c b/drivers/iio/adc/stx104.c > > index 7552351bfed9..7656b363e281 100644 > > --- a/drivers/iio/adc/stx104.c > > +++ b/drivers/iio/adc/stx104.c > > @@ -16,6 +16,7 @@ > > #include <linux/module.h> > > #include <linux/moduleparam.h> > > #include <linux/spinlock.h> > > +#include <linux/types.h> > > > > #define STX104_OUT_CHAN(chan) { \ > > .type = IIO_VOLTAGE, \ > > @@ -44,14 +45,36 @@ static unsigned int num_stx104; > > module_param_hw_array(base, uint, ioport, &num_stx104, 0); > > MODULE_PARM_DESC(base, "Apex Embedded Systems STX104 base addresses"); > > > > +/** > > + * struct stx104_reg - device register structure > > + * @ad: ADC Data > > + * @achan: ADC Channel > > + * @dio: Digital I/O > > + * @dac: DAC Channels > > + * @cir_asr: Clear Interrupts and ADC Status > > + * @acr: ADC Control > > + * @pccr_fsh: Pacer Clock Control and FIFO Status MSB > > + * @acfg: ADC Configuration > > + */ > > +struct stx104_reg { > > + u16 ad; > > + u8 achan; > > + u8 dio; > > + u16 dac[2]; > > + u8 cir_asr; > > + u8 acr; > > + u8 pccr_fsh; > > + u8 acfg; > > +}; > > + > > /** > > * struct stx104_iio - IIO device private data structure > > * @chan_out_states: channels' output states > > - * @base: base port address of the IIO device > > + * @reg: I/O address offset for the device registers > > */ > > struct stx104_iio { > > unsigned int chan_out_states[STX104_NUM_OUT_CHAN]; > > - void __iomem *base; > > + struct stx104_reg __iomem *reg; > > }; > > > > /** > > @@ -64,7 +87,7 @@ struct stx104_iio { > > struct stx104_gpio { > > struct gpio_chip chip; > > spinlock_t lock; > > - void __iomem *base; > > + u8 __iomem *base; > > unsigned int out_state; > > }; > > > > @@ -72,6 +95,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > > struct iio_chan_spec const *chan, int *val, int *val2, long mask) > > { > > struct stx104_iio *const priv = iio_priv(indio_dev); > > + struct stx104_reg __iomem *const reg = priv->reg; > > unsigned int adc_config; > > int adbu; > > int gain; > > @@ -79,7 +103,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > > switch (mask) { > > case IIO_CHAN_INFO_HARDWAREGAIN: > > /* get gain configuration */ > > - adc_config = ioread8(priv->base + 11); > > + adc_config = ioread8(®->acfg); > > gain = adc_config & 0x3; > > > > *val = 1 << gain; > > @@ -91,24 +115,24 @@ static int stx104_read_raw(struct iio_dev *indio_dev, > > } > > > > /* select ADC channel */ > > - iowrite8(chan->channel | (chan->channel << 4), priv->base + 2); > > + iowrite8(chan->channel | (chan->channel << 4), ®->achan); > > > > /* trigger ADC sample capture and wait for completion */ > > - iowrite8(0, priv->base); > > - while (ioread8(priv->base + 8) & BIT(7)); > > + iowrite8(0, ®->ad); > > Curious - 8 bit write to a 16 bit address? Maybe worth a comment > on why. This address doubles as the 8-bit Software Strobe Register; writing an 8-bit value to this address will initiate an ADC sample capture. I'll rename this member to ssr_ad and add some comments to make this more explicit. > > > + while (ioread8(®->cir_asr) & BIT(7)); > > > > - *val = ioread16(priv->base); > > + *val = ioread16(®->ad); > > return IIO_VAL_INT; > > case IIO_CHAN_INFO_OFFSET: > > /* get ADC bipolar/unipolar configuration */ > > - adc_config = ioread8(priv->base + 11); > > + adc_config = ioread8(®->acfg); > > adbu = !(adc_config & BIT(2)); > > > > *val = -32768 * adbu; > > return IIO_VAL_INT; > > case IIO_CHAN_INFO_SCALE: > > /* get ADC bipolar/unipolar and gain configuration */ > > - adc_config = ioread8(priv->base + 11); > > + adc_config = ioread8(®->acfg); > > adbu = !(adc_config & BIT(2)); > > gain = adc_config & 0x3; > > > > @@ -130,16 +154,16 @@ static int stx104_write_raw(struct iio_dev *indio_dev, > > /* Only four gain states (x1, x2, x4, x8) */ > > switch (val) { > > case 1: > > - iowrite8(0, priv->base + 11); > > + iowrite8(0, &priv->reg->acfg); > > break; > > case 2: > > - iowrite8(1, priv->base + 11); > > + iowrite8(1, &priv->reg->acfg); > > break; > > case 4: > > - iowrite8(2, priv->base + 11); > > + iowrite8(2, &priv->reg->acfg); > > break; > > case 8: > > - iowrite8(3, priv->base + 11); > > + iowrite8(3, &priv->reg->acfg); > > break; > > default: > > return -EINVAL; > > @@ -153,7 +177,7 @@ static int stx104_write_raw(struct iio_dev *indio_dev, > > return -EINVAL; > > > > priv->chan_out_states[chan->channel] = val; > > - iowrite16(val, priv->base + 4 + 2 * chan->channel); > > + iowrite16(val, priv->reg->dac + chan->channel); > Perhaps for consistency with below go with > &priv->reg->dac[chan->channels]; Ack. William Breathitt Gray > > > > return 0; > > } > > @@ -307,15 +331,15 @@ static int stx104_probe(struct device *dev, unsigned int id) > > } > > > > priv = iio_priv(indio_dev); > > - priv->base = devm_ioport_map(dev, base[id], STX104_EXTENT); > > - if (!priv->base) > > + priv->reg = devm_ioport_map(dev, base[id], STX104_EXTENT); > > + if (!priv->reg) > > return -ENOMEM; > > > > indio_dev->info = &stx104_info; > > indio_dev->modes = INDIO_DIRECT_MODE; > > > > /* determine if differential inputs */ > > - if (ioread8(priv->base + 8) & BIT(5)) { > > + if (ioread8(&priv->reg->cir_asr) & BIT(5)) { > > indio_dev->num_channels = ARRAY_SIZE(stx104_channels_diff); > > indio_dev->channels = stx104_channels_diff; > > } else { > > @@ -326,14 +350,14 @@ static int stx104_probe(struct device *dev, unsigned int id) > > indio_dev->name = dev_name(dev); > > > > /* configure device for software trigger operation */ > > - iowrite8(0, priv->base + 9); > > + iowrite8(0, &priv->reg->acr); > > > > /* initialize gain setting to x1 */ > > - iowrite8(0, priv->base + 11); > > + iowrite8(0, &priv->reg->acfg); > > > > /* initialize DAC output to 0V */ > > - iowrite16(0, priv->base + 4); > > - iowrite16(0, priv->base + 6); > > + iowrite16(0, &priv->reg->dac[0]); > > + iowrite16(0, &priv->reg->dac[1]); > > > > stx104gpio->chip.label = dev_name(dev); > > stx104gpio->chip.parent = dev; > > @@ -348,7 +372,7 @@ static int stx104_probe(struct device *dev, unsigned int id) > > stx104gpio->chip.get_multiple = stx104_gpio_get_multiple; > > stx104gpio->chip.set = stx104_gpio_set; > > stx104gpio->chip.set_multiple = stx104_gpio_set_multiple; > > - stx104gpio->base = priv->base + 3; > > + stx104gpio->base = &priv->reg->dio; > > stx104gpio->out_state = 0x0; > > > > spin_lock_init(&stx104gpio->lock); >
On Mon, Jun 6, 2022 at 4:27 PM William Breathitt Gray <william.gray@linaro.org> wrote: > > Reduce magic numbers and improve code readability by implementing and > utilizing named register data structures. Can we consider using regmap APIs instead?
On Wed, Jun 15, 2022 at 11:44:54AM +0200, Andy Shevchenko wrote: > On Mon, Jun 6, 2022 at 4:27 PM William Breathitt Gray > <william.gray@linaro.org> wrote: > > > > Reduce magic numbers and improve code readability by implementing and > > utilizing named register data structures. > > Can we consider using regmap APIs instead? > > > -- > With Best Regards, > Andy Shevchenko The regmap API may be more appropriate here. I'll investigate and see if I can convert this over to it. William Breathitt Gray
On Wed, Jun 15, 2022 at 1:55 PM William Breathitt Gray <william.gray@linaro.org> wrote: > On Wed, Jun 15, 2022 at 11:44:54AM +0200, Andy Shevchenko wrote: > > On Mon, Jun 6, 2022 at 4:27 PM William Breathitt Gray > > <william.gray@linaro.org> wrote: > > > > > > Reduce magic numbers and improve code readability by implementing and > > > utilizing named register data structures. > > > > Can we consider using regmap APIs instead? > The regmap API may be more appropriate here. I'll investigate and see if > I can convert this over to it. I just realized that this driver is for the old PC104 (like?) hardware that most likely uses IO ports, I don't remember if we have support for IO ports in regmap (MMIO -- yes for sure).
On Wed, Jun 15, 2022 at 02:00:26PM +0200, Andy Shevchenko wrote: > On Wed, Jun 15, 2022 at 1:55 PM William Breathitt Gray > <william.gray@linaro.org> wrote: > > On Wed, Jun 15, 2022 at 11:44:54AM +0200, Andy Shevchenko wrote: > > > On Mon, Jun 6, 2022 at 4:27 PM William Breathitt Gray > > > <william.gray@linaro.org> wrote: > > > > > > > > Reduce magic numbers and improve code readability by implementing and > > > > utilizing named register data structures. > > > > > > Can we consider using regmap APIs instead? > > > The regmap API may be more appropriate here. I'll investigate and see if > > I can convert this over to it. > > I just realized that this driver is for the old PC104 (like?) hardware > that most likely uses IO ports, I don't remember if we have support > for IO ports in regmap (MMIO -- yes for sure). > > -- > With Best Regards, > Andy Shevchenko Hmm, I don't see IO ports mentioned in include/linux/regmap.h, so I don't think the regmap API directly supports it (maybe someone familiar with regmap knows). Although we do get a virtual mapping cookie via ioport_map() in this driver, I don't know if we can pass that to the regmap functions and have it actually work. William Breathitt Gray
On Wed, Jun 15, 2022 at 2:19 PM William Breathitt Gray <william.gray@linaro.org> wrote: > On Wed, Jun 15, 2022 at 02:00:26PM +0200, Andy Shevchenko wrote: > > On Wed, Jun 15, 2022 at 1:55 PM William Breathitt Gray > > <william.gray@linaro.org> wrote: > > > On Wed, Jun 15, 2022 at 11:44:54AM +0200, Andy Shevchenko wrote: > > > > On Mon, Jun 6, 2022 at 4:27 PM William Breathitt Gray > > > > <william.gray@linaro.org> wrote: > > > > > > > > > > Reduce magic numbers and improve code readability by implementing and > > > > > utilizing named register data structures. > > > > > > > > Can we consider using regmap APIs instead? > > > > > The regmap API may be more appropriate here. I'll investigate and see if > > > I can convert this over to it. > > > > I just realized that this driver is for the old PC104 (like?) hardware > > that most likely uses IO ports, I don't remember if we have support > > for IO ports in regmap (MMIO -- yes for sure). > Hmm, I don't see IO ports mentioned in include/linux/regmap.h, so I > don't think the regmap API directly supports it (maybe someone familiar > with regmap knows). Although we do get a virtual mapping cookie via > ioport_map() in this driver, I don't know if we can pass that to the > regmap functions and have it actually work. The problem is with accessors which are inconsistent in regmap MMIO implementation. I think it should be converted to use ioreadXX()/iowriteXX() in all cases (currently only BE cases use them). Another variant is to provide read*_be() / write*_be() for all architectures, replace corresponding ops in regmap MMIO and introduce regmap IO with inX()/outX. The former seems to me the best option, while the latter is cleaner. +Cc: Mark if he knows more about this.
On Wed, Jun 15, 2022 at 02:43:27PM +0200, Andy Shevchenko wrote: > On Wed, Jun 15, 2022 at 2:19 PM William Breathitt Gray > > On Wed, Jun 15, 2022 at 02:00:26PM +0200, Andy Shevchenko wrote: > > > I just realized that this driver is for the old PC104 (like?) hardware > > > that most likely uses IO ports, I don't remember if we have support > > > for IO ports in regmap (MMIO -- yes for sure). > > Hmm, I don't see IO ports mentioned in include/linux/regmap.h, so I > > don't think the regmap API directly supports it (maybe someone familiar > > with regmap knows). Although we do get a virtual mapping cookie via > > ioport_map() in this driver, I don't know if we can pass that to the > > regmap functions and have it actually work. > The problem is with accessors which are inconsistent in regmap MMIO > implementation. I think it should be converted to use > ioreadXX()/iowriteXX() in all cases (currently only BE cases use > them). Another variant is to provide read*_be() / write*_be() for all > architectures, replace corresponding ops in regmap MMIO and introduce > regmap IO with inX()/outX. The former seems to me the best option, > while the latter is cleaner. I don't know what driver this is, but yes someone would have to add ioport support to use them with regmap.
diff --git a/drivers/iio/adc/stx104.c b/drivers/iio/adc/stx104.c index 7552351bfed9..7656b363e281 100644 --- a/drivers/iio/adc/stx104.c +++ b/drivers/iio/adc/stx104.c @@ -16,6 +16,7 @@ #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/spinlock.h> +#include <linux/types.h> #define STX104_OUT_CHAN(chan) { \ .type = IIO_VOLTAGE, \ @@ -44,14 +45,36 @@ static unsigned int num_stx104; module_param_hw_array(base, uint, ioport, &num_stx104, 0); MODULE_PARM_DESC(base, "Apex Embedded Systems STX104 base addresses"); +/** + * struct stx104_reg - device register structure + * @ad: ADC Data + * @achan: ADC Channel + * @dio: Digital I/O + * @dac: DAC Channels + * @cir_asr: Clear Interrupts and ADC Status + * @acr: ADC Control + * @pccr_fsh: Pacer Clock Control and FIFO Status MSB + * @acfg: ADC Configuration + */ +struct stx104_reg { + u16 ad; + u8 achan; + u8 dio; + u16 dac[2]; + u8 cir_asr; + u8 acr; + u8 pccr_fsh; + u8 acfg; +}; + /** * struct stx104_iio - IIO device private data structure * @chan_out_states: channels' output states - * @base: base port address of the IIO device + * @reg: I/O address offset for the device registers */ struct stx104_iio { unsigned int chan_out_states[STX104_NUM_OUT_CHAN]; - void __iomem *base; + struct stx104_reg __iomem *reg; }; /** @@ -64,7 +87,7 @@ struct stx104_iio { struct stx104_gpio { struct gpio_chip chip; spinlock_t lock; - void __iomem *base; + u8 __iomem *base; unsigned int out_state; }; @@ -72,6 +95,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan, int *val, int *val2, long mask) { struct stx104_iio *const priv = iio_priv(indio_dev); + struct stx104_reg __iomem *const reg = priv->reg; unsigned int adc_config; int adbu; int gain; @@ -79,7 +103,7 @@ static int stx104_read_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_HARDWAREGAIN: /* get gain configuration */ - adc_config = ioread8(priv->base + 11); + adc_config = ioread8(®->acfg); gain = adc_config & 0x3; *val = 1 << gain; @@ -91,24 +115,24 @@ static int stx104_read_raw(struct iio_dev *indio_dev, } /* select ADC channel */ - iowrite8(chan->channel | (chan->channel << 4), priv->base + 2); + iowrite8(chan->channel | (chan->channel << 4), ®->achan); /* trigger ADC sample capture and wait for completion */ - iowrite8(0, priv->base); - while (ioread8(priv->base + 8) & BIT(7)); + iowrite8(0, ®->ad); + while (ioread8(®->cir_asr) & BIT(7)); - *val = ioread16(priv->base); + *val = ioread16(®->ad); return IIO_VAL_INT; case IIO_CHAN_INFO_OFFSET: /* get ADC bipolar/unipolar configuration */ - adc_config = ioread8(priv->base + 11); + adc_config = ioread8(®->acfg); adbu = !(adc_config & BIT(2)); *val = -32768 * adbu; return IIO_VAL_INT; case IIO_CHAN_INFO_SCALE: /* get ADC bipolar/unipolar and gain configuration */ - adc_config = ioread8(priv->base + 11); + adc_config = ioread8(®->acfg); adbu = !(adc_config & BIT(2)); gain = adc_config & 0x3; @@ -130,16 +154,16 @@ static int stx104_write_raw(struct iio_dev *indio_dev, /* Only four gain states (x1, x2, x4, x8) */ switch (val) { case 1: - iowrite8(0, priv->base + 11); + iowrite8(0, &priv->reg->acfg); break; case 2: - iowrite8(1, priv->base + 11); + iowrite8(1, &priv->reg->acfg); break; case 4: - iowrite8(2, priv->base + 11); + iowrite8(2, &priv->reg->acfg); break; case 8: - iowrite8(3, priv->base + 11); + iowrite8(3, &priv->reg->acfg); break; default: return -EINVAL; @@ -153,7 +177,7 @@ static int stx104_write_raw(struct iio_dev *indio_dev, return -EINVAL; priv->chan_out_states[chan->channel] = val; - iowrite16(val, priv->base + 4 + 2 * chan->channel); + iowrite16(val, priv->reg->dac + chan->channel); return 0; } @@ -307,15 +331,15 @@ static int stx104_probe(struct device *dev, unsigned int id) } priv = iio_priv(indio_dev); - priv->base = devm_ioport_map(dev, base[id], STX104_EXTENT); - if (!priv->base) + priv->reg = devm_ioport_map(dev, base[id], STX104_EXTENT); + if (!priv->reg) return -ENOMEM; indio_dev->info = &stx104_info; indio_dev->modes = INDIO_DIRECT_MODE; /* determine if differential inputs */ - if (ioread8(priv->base + 8) & BIT(5)) { + if (ioread8(&priv->reg->cir_asr) & BIT(5)) { indio_dev->num_channels = ARRAY_SIZE(stx104_channels_diff); indio_dev->channels = stx104_channels_diff; } else { @@ -326,14 +350,14 @@ static int stx104_probe(struct device *dev, unsigned int id) indio_dev->name = dev_name(dev); /* configure device for software trigger operation */ - iowrite8(0, priv->base + 9); + iowrite8(0, &priv->reg->acr); /* initialize gain setting to x1 */ - iowrite8(0, priv->base + 11); + iowrite8(0, &priv->reg->acfg); /* initialize DAC output to 0V */ - iowrite16(0, priv->base + 4); - iowrite16(0, priv->base + 6); + iowrite16(0, &priv->reg->dac[0]); + iowrite16(0, &priv->reg->dac[1]); stx104gpio->chip.label = dev_name(dev); stx104gpio->chip.parent = dev; @@ -348,7 +372,7 @@ static int stx104_probe(struct device *dev, unsigned int id) stx104gpio->chip.get_multiple = stx104_gpio_get_multiple; stx104gpio->chip.set = stx104_gpio_set; stx104gpio->chip.set_multiple = stx104_gpio_set_multiple; - stx104gpio->base = priv->base + 3; + stx104gpio->base = &priv->reg->dio; stx104gpio->out_state = 0x0; spin_lock_init(&stx104gpio->lock);
Reduce magic numbers and improve code readability by implementing and utilizing named register data structures. Signed-off-by: William Breathitt Gray <william.gray@linaro.org> --- drivers/iio/adc/stx104.c | 70 +++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 23 deletions(-)