Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse

[Jonathan Cameron]

On Fri, 19 Jun 2026 10:16:31 +0100
Nuno Sá <noname.nuno@xxxxxxxxx> wrote:

> On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote:
> > On 18/06/26 16:06, Nuno Sá wrote:  
> > > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote:  
> > > > From: Rodrigo Alencar <rodrigo.alencar@xxxxxxxxxx>
> > > > 
> > > > Move logic to create a channel prefix for naming attribute files into a
> > > > separate __iio_chan_prefix_emit() function for reuse.  
> > 
> > ...
> >   
> > > > +static int __iio_chan_prefix_emit(const struct iio_chan_spec *chan,
> > > > +				  enum iio_shared_by shared_by,
> > > > +				  char *buf, size_t len)
> > > > +{
> > > > +	const char *dir = iio_direction[chan->output];
> > > > +	const char *type = iio_chan_type_name_spec[chan->type];
> > > > +	int n = 0;
> > > > +
> > > > +	switch (shared_by) {
> > > > +	case IIO_SHARED_BY_ALL:
> > > > +		buf[0] = '\0'; /* empty channel prefix */
> > > > +		break;
> > > > +	case IIO_SHARED_BY_DIR:
> > > > +		n = scnprintf(buf, len, "%s", dir);
> > > > +		break;
> > > > +	case IIO_SHARED_BY_TYPE:
> > > > +		n = scnprintf(buf, len, "%s_%s", dir, type);
> > > > +		if (chan->differential)
> > > > +			n += scnprintf(buf + n, len - n, "-%s", type);
> > > > +		break;
> > > > +	case IIO_SEPARATE:
> > > > +		if (chan->indexed) {
> > > > +			n = scnprintf(buf, len, "%s_%s%d", dir, type,
> > > > +				      chan->channel);
> > > > +			if (chan->differential)
> > > > +				n += scnprintf(buf + n, len - n, "-%s%d", type,
> > > > +					       chan->channel2);
> > > > +		} else {
> > > > +			if (chan->differential) {
> > > > +				WARN(1, "Differential channels must be indexed\n");
> > > > +				return -EINVAL;
> > > > +			}
> > > > +			n = scnprintf(buf, len, "%s_%s", dir, type);
> > > > +		}
> > > > +
> > > > +		if (chan->modified) {
> > > > +			if (chan->differential) {
> > > > +				WARN(1, "Differential channels can not have modifier\n");
> > > > +				return -EINVAL;  
> > > 
> > > WARN() looks too much to me. dev_error() as we're treating it as such. I
> > > guess you don't want to pass struct device but not really an issue IMHO.  
> > 
> > __iio_device_attr_init() also used WARN(), probably because it didnt have
> > access to a dev pointer. It would not be a problem to add an extra param.  
> 
> Hmm, fair enough. Maybe a chance to change it. Not sure how others feel
> about it.
I was young and knew less back then (either as reviewer or author).
WARN() on a lot of systems actually means panic, so for stuff like this
it isn't appropriate.  I'd definitely support a series flipping any
WARN() to dev_err() or similar.

Thanks,

Jonathan