Re: [PATCH 1/3] iio: accel: adxl372: introduce chip_info structure

[Jonathan Cameron]

On Mon, 2 Mar 2026 14:59:37 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxx> wrote:

> On Mon, Mar 02, 2026 at 02:20:57PM +0200, Antoniu Miclaus wrote:
> > Introduce a chip_info structure to parameterize device-specific
> > properties such as ODR/bandwidth frequency tables, activity/inactivity
> > timer scale factors, and the maximum ODR value. This refactors the
> > driver to use chip_info lookups instead of hardcoded values, preparing
> > the driver to support multiple device variants.
> > 
> > The sampling_frequency_available sysfs attribute is changed from a
> > static const string to a dynamic callback that reads from chip_info,
> > and the SPI/I2C probe functions are updated to pass a chip_info
> > pointer instead of a device name string.
> > 
> > No functional change intended.  
> 
> ...
> 
> > -static IIO_CONST_ATTR_SAMP_FREQ_AVAIL("400 800 1600 3200 6400");
> > +static ssize_t adxl372_show_samp_freq_avail(struct device *dev,
> > +					    struct device_attribute *attr,
> > +					    char *buf)
> > +{
> > +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > +	struct adxl372_state *st = iio_priv(indio_dev);
> > +	int i;
> > +	size_t len = 0;
> > +
> > +	for (i = 0; i < st->chip_info->num_freqs; i++)  
> 
> > +		len += scnprintf(buf + len, PAGE_SIZE - len,
> > +				 "%d ", st->chip_info->samp_freq_tbl[i]);  
> 
> This is part of sysfs, use sysfs_emit_at().

Instead can we switch this to read_avail() based handling?
That means setting info_mask_shared_by_all_available
bit for sampling frequency.

Looks superficially easy to do the same for the
filter_low_pass_3db_frequency_available.

The attributes go away entirely.
Advantage of this is it both enforces the formatting without us having
to read the functions carefully and it makes them available to in kernel
consumers.

Jonathan



> 
> > +	buf[len - 1] = '\n';
> > +
> > +	return len;
> > +}
> > +
> > +static IIO_DEVICE_ATTR(sampling_frequency_available,
> > +		       0444, adxl372_show_samp_freq_avail, NULL, 0);  
> 
> What's wrong with IIO_DEVICE_ATTR_RO()?
> 
> >  static IIO_DEVICE_ATTR(in_accel_filter_low_pass_3db_frequency_available,
> >  		       0444, adxl372_show_filter_freq_avail, NULL, 0);  
> 
> Make sure this is closer to its callback(s).
> 
> ...
> 
> >  static struct attribute *adxl372_attributes[] = {
> > -	&iio_const_attr_sampling_frequency_available.dev_attr.attr,
> > +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> >  	&iio_dev_attr_in_accel_filter_low_pass_3db_frequency_available.dev_attr.attr,  
> 
> >  	NULL,  
> 
> Side note: At some point consider dropping trailing comma in the terminator.
> 
> >  };  
>