Re: [PATCH 2/2] iio: dac: add support for Microchip MCP48FEB02

[Ariana.Lazar]

Hello Andy,

please see my comments below


> 
> > +static int mcp48feb02_read_avail(struct iio_dev *indio_dev, struct
> > iio_chan_spec const *ch,
> > +                              const int **vals, int *type, int
> > *length, long info)
> > +{
> > +     struct mcp48feb02_data *data = iio_priv(indio_dev);
> > +
> > +     switch (info) {
> > +     case IIO_CHAN_INFO_SCALE:
> > +             switch (ch->type) {
> > +             case IIO_VOLTAGE:
> > +                     if (data->phys_channels >= 4 && (ch->address
> > % 2))
> > +                             *vals = data->scale_1;
> > +                     else
> > +                             *vals = data->scale;
> 
> Actually, if you put the scales as
> 
>         int scales[2][2 * MCP48FEB02_MAX_SCALES_CH];
> 
> this will become as simple as
> 
>                         if (data->phys_channels >= 4)
>                                 *vals = data->scales[ch->address];
>                         else
>                                 *vals = data->scales[0];
> 
> OTOH, I am not sure if it can be always as
> 
>                         *vals = data->scales[ch->address];
> 
> which would be the best approach.
> 
> 
> 

> 
I am not quite sure I have understood your point of view. In order to
remove the channel parity check, I would have to declare int
scales[MCP48FEB02_MAX_CH][2 * MCP48FEB02_MAX_SCALES_CH] (int
scales[8][6])
regardless of device's number of channels and number of voltage
references. This will be quite a lot unnecessary space allocated
compared to using only two arrays of [2 * MCP48FEB02_MAX_SCALES_CH].

Another way to avoid these checks is to use a dynamically allocated
array of scales. Each member points to an array of [2 *
MCP48FEB02_MAX_SCALES_CH]
and stores corresponding scale values for each channel, while allowing
to allocate the actual number of channels the device has rather than
the maximum.

Please tell me which version you prefer.

Best regards,
Ariana