On Fri, 19 Jun 2026 10:16:31 +0100
Nuno Sá <[email protected]> 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 <[email protected]>
> > > > 
> > > > 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

Reply via email to