On Tue, Jun 14, 2022 at 12:26:18PM +0100, Jonathan Cameron wrote: > On Mon, 6 Jun 2022 10:15:18 -0400 > William Breathitt Gray <william.gray@xxxxxxxxxx> wrote: > > > Reduce magic numbers and improve code readability by implementing and > > utilizing named register data structures. > > > > Signed-off-by: William Breathitt Gray <william.gray@xxxxxxxxxx> > > I'm unconvinced this one really helps readability seeing > as you are only indexing a straight forward array. > > Simply using u16 __iomem * > would provide the main cleanup which is avoiding the indexing > via * 2. > > Thanks, > > Jonathan I agree, that is a much simpler approach and reduces the changes we need to make to this file. I'll adjust this to u16 __iomem * in v2. William Breathitt Gray > > > > --- > > drivers/iio/dac/cio-dac.c | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/iio/dac/cio-dac.c b/drivers/iio/dac/cio-dac.c > > index 8080984dcb03..7860450ceaf3 100644 > > --- a/drivers/iio/dac/cio-dac.c > > +++ b/drivers/iio/dac/cio-dac.c > > @@ -16,6 +16,7 @@ > > #include <linux/isa.h> > > #include <linux/module.h> > > #include <linux/moduleparam.h> > > +#include <linux/types.h> > > > > #define CIO_DAC_NUM_CHAN 16 > > > > @@ -34,14 +35,22 @@ static unsigned int num_cio_dac; > > module_param_hw_array(base, uint, ioport, &num_cio_dac, 0); > > MODULE_PARM_DESC(base, "Measurement Computing CIO-DAC base addresses"); > > > > +/** > > + * struct cio_dac_reg - device register structure > > + * @da: D/A data > > + */ > > +struct cio_dac_reg { > > + u16 da[CIO_DAC_NUM_CHAN]; > > +}; > > + > > /** > > * struct cio_dac_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 cio_dac_iio { > > int chan_out_states[CIO_DAC_NUM_CHAN]; > > - void __iomem *base; > > + struct cio_dac_reg __iomem *reg; > > }; > > > > static int cio_dac_read_raw(struct iio_dev *indio_dev, > > @@ -61,7 +70,6 @@ static int cio_dac_write_raw(struct iio_dev *indio_dev, > > struct iio_chan_spec const *chan, int val, int val2, long mask) > > { > > struct cio_dac_iio *const priv = iio_priv(indio_dev); > > - const unsigned int chan_addr_offset = 2 * chan->channel; > > > > if (mask != IIO_CHAN_INFO_RAW) > > return -EINVAL; > > @@ -71,7 +79,7 @@ static int cio_dac_write_raw(struct iio_dev *indio_dev, > > return -EINVAL; > > > > priv->chan_out_states[chan->channel] = val; > > - iowrite16(val, priv->base + chan_addr_offset); > > + iowrite16(val, priv->reg->da + chan->channel); > > > > return 0; > > } > > @@ -106,8 +114,8 @@ static int cio_dac_probe(struct device *dev, unsigned int id) > > } > > > > priv = iio_priv(indio_dev); > > - priv->base = devm_ioport_map(dev, base[id], CIO_DAC_EXTENT); > > - if (!priv->base) > > + priv->reg = devm_ioport_map(dev, base[id], CIO_DAC_EXTENT); > > + if (!priv->reg) > > return -ENOMEM; > > > > indio_dev->info = &cio_dac_info; > > @@ -117,8 +125,8 @@ static int cio_dac_probe(struct device *dev, unsigned int id) > > indio_dev->name = dev_name(dev); > > > > /* initialize DAC outputs to 0V */ > > - for (i = 0; i < 32; i += 2) > > - iowrite16(0, priv->base + i); > > + for (i = 0; i < CIO_DAC_NUM_CHAN; i++) > > + iowrite16(0, priv->reg->da + i); > > > > return devm_iio_device_register(dev, indio_dev); > > } >
Attachment:
signature.asc
Description: PGP signature