Re: [PATCH v3 2/9] iio: adc: add helpers for parsing ADC nodes

[Jonathan Cameron]

On Fri, 21 Feb 2025 18:41:05 +0200
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:

> On Fri, Feb 21, 2025 at 12:10:23PM +0200, Matti Vaittinen wrote:
> > On 20/02/2025 16:56, Andy Shevchenko wrote:  
> > > On Thu, Feb 20, 2025 at 04:21:37PM +0200, Matti Vaittinen wrote:  
> > > > On 20/02/2025 16:04, Andy Shevchenko wrote:  
> > > > > On Thu, Feb 20, 2025 at 03:40:30PM +0200, Matti Vaittinen wrote:  
> > > > > > On 20/02/2025 14:41, Andy Shevchenko wrote:  
> > > > > > > On Thu, Feb 20, 2025 at 09:13:00AM +0200, Matti Vaittinen wrote:  
> > > > > > > > On 19/02/2025 22:41, Andy Shevchenko wrote:  
> > > > > > > > > On Wed, Feb 19, 2025 at 02:30:27PM +0200, Matti Vaittinen wrote:  
> 
> ...
> 
> > > > > > > > > > +EXPORT_SYMBOL_GPL(iio_adc_device_num_channels);  
> > > > > > > > > 
> > > > > > > > > No namespace?  
> > > > > > > > 
> > > > > > > > I was considering also this. The IIO core functions don't belong into a
> > > > > > > > namespace - so I followed the convention to keep these similar to other IIO
> > > > > > > > core stuff.  
> > > > > > > 
> > > > > > > But it's historically. We have already started using namespaces
> > > > > > > in the parts of IIO, haven't we?  
> > > > > > 
> > > > > > Yes. But as I wrote, I don't think adding new namespaces for every helper
> > > > > > file with a function or two exported will scale. We either need something
> > > > > > common for IIO (or IIO "subsystems" like "adc", "accel", "light", ... ), or
> > > > > > then we just keep these small helpers same as most of the IIO core.  
> > > > > 
> > > > > It can be still pushed to IIO_CORE namespace. Do you see an issue with that?  
> > > > 
> > > > No. I've missed the fact we have IIO_CORE O_o. Thanks for pointing it out!
> > > >   
> > > > > Or a new opaque namespace for the mentioned cases, something like IIO_HELPERS.  
> > > > 
> > > > I am unsure if it really benefits to split this out of the IIO_CORE. I've a
> > > > feeling it falls into the category of making things harder for user with no
> > > > apparent reason. But yes, the IIO_CORE makes sense.  
> > > 
> > > Probably I was not clear, I mean to put this under a given namespace. There is
> > > no a such, we have currently:
> > > 
> > > IIO_BACKEND
> > > IIO_DMA_BUFFER
> > > IIO_DMAENGINE_BUFFER
> > > IIO_GTS_HELPER
> > > IIO_RESCALE  
> > 
> > Ah. So, the IIO core stuff is still not in a namespace. Those listed above
> > are all too specific (I believe, in general, and definitely to carry ADC
> > helpers).

Not yet. On todo list... Trick is working out what the correct break up is.

> > 
> > Adding 'ADC_HELPERS' would just add yet another way too specific one. So,
> > currently there is no suitable namespace for these helpers, and I still
> > believe they fit best to where the rest of the IIO-core stuff is.

Just add an IIO namespace.  That would be the one I'd expect a typical
driver to use.  We can move things into it later. The ones above are
more obscure functionality.  Avoid the IIO_CORE naming as I'd expect
that to be stuff the core alone should call and we shouldn't see in drivers.

> > 
> > If we want really play the namespace game, then the existing IIO stuff
> > should be put in a IIO_CORE-namespace instead of creating more new small
> > ones. I am afraid that adding all existing IIO core to a IIO_CORE namespace
> > and converting all existing users to use the IIO_CORE is not a reasonable
> > request for a person trying to:
> > 
> > 1. Write a driver
> > 2. Add a small helper to aid others (instead of just melding it all in the
> > given new driver - which does not benefit anyone else and just leads to code
> > duplication in the long run...)  
> 
> That's why more specific, but also a bit general might work, like IIO_HELPERS,
> considering that they may be used by many drivers.
> 
> While it may be not your call, somebody should do the job. Jonathan? :-)

It's on the list.  Not that near the top of it, but there to do at somepoint.

> 
> > > > > > > > (Sometimes I have a feeling that the trend today is to try make things
> > > > > > > > intentionally difficult in the name of the safety. Like, "more difficult I
> > > > > > > > make this, more experience points I gain in the name of the safety".)
> > > > > > > > 
> > > > > > > > Well, I suppose I could add a namespace for these functions - if this
> > > > > > > > approach stays - but I'd really prefer having all IIO core stuff in some
> > > > > > > > global IIO namespace and not to have dozens of fine-grained namespaces for
> > > > > > > > an IIO driver to use...  
> 
> ...
> 
> > > > > > foo &= (~bar);
> > > > > > 
> > > > > > is _much_ faster than seeing:  
> > > > > 
> > > > > Strongly disagree. One need to parse an additional pair of parentheses,
> > > > > and especially when it's a big statement inside with nested ones along
> > > > > with understanding what the heck is going on that you need them in the
> > > > > first place.
> > > > > 
> > > > > On top of that, we have a common practices in the LK project and
> > > > > with our history of communication it seems you are trying to do differently
> > > > > from time to time. Sounds like a rebellion to me :-)  
> > > > 
> > > > I only rebel when I (in my opinion) have a solid reason :)
> > > >   
> > > > > > foo &= ~bar;
> > > > > > 
> > > > > > and having to google the priorities.  
> > > > > 
> > > > > Again, this is something a (regular) kernel developer keeps refreshed.
> > > > > Or even wider, C-language developer.  
> > > > 
> > > > Ha. As I mentioned, I've been writing C on a daily bases for almost 25
> > > > years. I wonder if you intent to say I am not a kernel/C-language developer?
> > > > Bold claim.  
> > > 
> > > I'm just surprised by seeing that style from a 25y experienced C developer,
> > > that's all.  
> > 
> > I am not. If something, these 25 years have taught me to understand that
> > even if something is simple and obvious to me, it may not be simple and
> > obvious to someone else. Similarly, something obvious to someone else, is
> > not obvious to me. Hence, I am very careful when telling people that:
> >   
> > >>> Again, this is something a (regular) kernel developer keeps refreshed.
> > >>> Or even wider, C-language developer.  
> > 
> > I may however say that "this is something _I_ keep refreshed (as a
> > kernel/C-developer)".  
> 
> True.
> 
> > As an example,
> >   
> > >>>> foo &= (~bar);  
> > 
> > This is something _I_ find very clear and exact, with zero doubt if negation
> > is applied before &=. For _me_ the parenthesis there _help_, and for _me_
> > the parenthesis cause no confusion when reading the code.
> > 
> > I won't go and tell that I'd expect any C or kernel developer to be able to
> > fluently parse "foo &= (~bar)". (Whether I think they should is another
> > matter).  
> 
> > Oh well, let's wait and see what Jonathan thinks of these helpers in
> > general. We can continue the parenthesis discussion when we know whether the
> > code is going to stay.  
> 
> Sure, but it's not only about these helpers, it's about the style in general.
> Spreading unneeded characters in the code seems to me as an attempt to put
> _your_ rules over the subsytem's ones. Whatever, let's Jonathan to judge, we
> will never agree on a keep growing list of things anyway...
> 
I need to find some time to consider it a bit more and look at other users.
I'm not keen on this being 'general' if we only have one user in the short
term.

Jonathan