Hi,

On Thu, Sep 20, 2018 at 06:20:38PM -0700, Nicolin Chen wrote:

quoted

So if there are no devicetree entries, the user now gets a sequence of
"unknown" sensors ? I don't think so. Please keep in mind that there are
users of this chip who don't have devicetree systems, and other users
may not want to specify any specific name properties (or they use sensors3.conf
to do it).

Being enlightened by your comments below, maybe adding a
separate group for channel names by attaching is_visible
to it could be acceptable? Then, name nodes can hide from
those who don't want to specify.

quoted

quoted

+		/* Log connected channels */
+		ina->attr_group[g++] = &ina3221_group[i];
+		ina->channel_name[i] = str;
+		ina->enable[i] = true;

I also don't see the need for defining the group dynamically. The
is_visible callback is very well suited for handling optional sysfs
attributes.

I tried is_visible but it looks like it won't be that neat to
implement as some attributes of this driver don't really pass
the channel index to the store()/show() but some other indexes.

If you are very against the dynamical group, I can drop it to
leave the sysfs node as it was.

And for the name nodes that I was talking about above, I will
add an sysfs store() function so non-DT users can set them,
and I also removed the confusing "unknown" default name.

I have been rewriting the DT binding and it should make a lot
of sense now comparing to this version. Will send v2 tomrrow.

Thanks
Nicolin