Re: [PATCH v2] iio: frequency: admv1013: fix NULL pointer dereference on str

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Wed, Mar 04, 2026 at 09:52:46AM +0000, Miclaus, Antoniu wrote:
> > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > Sent: Tuesday, March 3, 2026 3:55 PM
> > On Tue, Mar 03, 2026 at 12:01:31PM +0000, Miclaus, Antoniu wrote:
> > > > From: Andy Shevchenko <andriy.shevchenko@intel.com>
> > > > Sent: Tuesday, March 3, 2026 1:34 PM
> > > > On Tue, Mar 03, 2026 at 11:52:28AM +0200, Antoniu Miclaus wrote:

...

> > > > > +static const char * const admv1013_quad_se_modes[] = { "diff", "se-
> > pos",
> > > > "se-neg" };
> > > >
> > > > Taking into account the indices are not sequential, this may require another
> > > > enumerator.
> > > >
> > > > Ideally you need to list all possible modes and choose only supported by
> > > > assigning an empty string to unsupported ones.
> > > >
> > > > I haven't checked datasheet to understand why only 6, 9, 12 are in use.
> > > > Maybe they can be simply 1, 2, 3 with a formula like 3 + x * 3 ? Dunno.
> > > >
> > >
> > > static const char * const admv1013_input_modes[] = {
> > >        [ADMV1013_IQ_MODE] = "iq",
> > >        [ADMV1013_IF_MODE] = "if",
> > > };
> > >
> > > static const char * const admv1013_quad_se_modes[] = {
> > >        [ADMV1013_QUAD_SE_DIFF] = "diff",
> > >        [ADMV1013_QUAD_SE_POS] = "se-pos",
> > >        [ADMV1013_QUAD_SE_NEG] = "se-neg",
> > > };
> > >
> > > static const unsigned int admv1013_quad_se_regvals[] = {
> > >        [ADMV1013_QUAD_SE_DIFF] = ADMV1013_SE_MODE_DIFF,
> > >        [ADMV1013_QUAD_SE_POS] = ADMV1013_SE_MODE_POS,
> > >        [ADMV1013_QUAD_SE_NEG] = ADMV1013_SE_MODE_NEG,
> > > };
> > >
> > > Does this make sense?
> > 
> > Yes, if there is an explanation why we have 6,9,12 to begin with. Can
> > somebody
> > study the available datasheets and come up with the explanations?
> 
> There are the values accepted by the QUAD_SE_MODE bitfield.
> 
> https://www.analog.com/media/en/technical-documentation/data-sheets/admv1013.pdf
> Page 37 of 39, Table 15.  

Thanks!

So, looking at the code and the datasheet I think we need to do the following.

Apply the rule that each HW related enum has to be defined with explicit
value. But looking at the code, I think we need to do exactly the opposite,
id est treat all enums as Linux driver ones. With this

1. Drop values from the given enum

  enum {
        ADMV1013_SE_MODE_POS,
        ADMV1013_SE_MODE_NEG,
        ADMV1013_SE_MODE_DIFF,
  };

2. Use it directly as indices for string arrays.
3. Use plain numbers in the mapping switch when assigning the values in
the similar way like it's done for other registers in other functions,
exempli gratia quad filters.

This will make consistent code among different bitfields used in the driver
and makes easier to use string arrays without double enums, et cetera.

-- 
With Best Regards,
Andy Shevchenko