Thread (32 messages) 32 messages, 4 authors, 2025-03-02

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

From: Jonathan Cameron <jic23@kernel.org>
Date: 2025-02-22 17:48:53
Also in: linux-devicetree, linux-iio, linux-renesas-soc, linux-sunxi, lkml

On Fri, 21 Feb 2025 18:41:05 +0200
Andy Shevchenko [off-list ref] wrote:
On Fri, Feb 21, 2025 at 12:10:23PM +0200, Matti Vaittinen wrote:
quoted
On 20/02/2025 16:56, Andy Shevchenko wrote:  
quoted
On Thu, Feb 20, 2025 at 04:21:37PM +0200, Matti Vaittinen wrote:  
quoted
On 20/02/2025 16:04, Andy Shevchenko wrote:  
quoted
On Thu, Feb 20, 2025 at 03:40:30PM +0200, Matti Vaittinen wrote:  
quoted
On 20/02/2025 14:41, Andy Shevchenko wrote:  
quoted
On Thu, Feb 20, 2025 at 09:13:00AM +0200, Matti Vaittinen wrote:  
quoted
On 19/02/2025 22:41, Andy Shevchenko wrote:  
quoted
On Wed, Feb 19, 2025 at 02:30:27PM +0200, Matti Vaittinen wrote:  
...
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
+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!
  
quoted
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.
quoted
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.
quoted
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.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
(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...  
...
quoted
quoted
quoted
quoted
quoted
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 :)
  
quoted
quoted
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:
  
quoted
quoted
quoted
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.
quoted
As an example,
  
quoted
quoted
quoted
quoted
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).  
quoted
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


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help