On 07/09/2025 14:16, Jonathan Cameron wrote:

quoted

quoted

quoted

+/* ADC channels as named in the data-sheet */
+static const char * const bd79112_chan_names[] = {
+	"AGIO0A", "AGIO1A", "AGIO2A", "AGIO3A", "AGIO4A",	/* 0 - 4 */
+	"AGIO5A", "AGIO6A", "AGIO7A", "AGIO8A", "AGIO9A",	/* 5 - 9 */
+	"AGIO10A", "AGIO11A", "AGIO12A", "AGIO13A", "AGIO14A",	/* 10 - 14 */
+	"AGIO15A", "AGIO0B", "AGIO1B", "AGIO2B", "AGIO3B",	/* 15 - 19 */
+	"AGIO4B", "AGIO5B", "AGIO6B", "AGIO7B", "AGIO8B",	/* 20 - 24 */
+	"AGIO9B", "AGIO10B", "AGIO11B", "AGIO12B", "AGIO13B",	/* 25 - 29 */
+	"AGIO14B", "AGIO15B",					/* 30 - 31 */

O-o-key, but why not power-of-two per line (esp. taking into account
the whole size)? (Whatever, it's not something I would fight for.)

I just filled the rows to the maximum width, while keeping the item
count same for each row and adding the comment.

I'm not really sure having 4 items / row, and adding one row more would
be much better, but not much worse either. I can do this if you think
it's better. (No need to even fight for that).

In this case I'd do it in 4s purely because then the B ones start on
a new line and that looks nicer ;)

Ok.

quoted

quoted

quoted

+};

...
   

quoted

+	data->vref_mv = ret / 1000;

Yeah, mV, (MICRO / MILLI) and other things I leave to other people to discuss.

Thanks Andy. I've a feeling we were somewhat stuck on this discussion
anyways. I suppose it's nice to hear other's opinions if someone else
really cares. This was approaching bikeshedding.

I'll just throw a quick comment in here.

I'm absolutely in favour of the defines as they help in several ways.
- Stop counting 0s when there are lots of them.

I do agree on this when there is more than 3 zeroes. Although, for 
example 100 * 1000 is still clear to me. 100 * MILLI is not quite as 
obvious (again, to me).

- Provide information on the unit conversion not otherwise visible
   where they happen to be used (often avoids need for local variables etc).

Speaking only on my behalf but I'd rather see the local variables 
instead of constructs like (MICRO / MILLI) - which does not really feel 
right to me.

- Consistency across a code base.

This probably means much more to a subsystem maintainer than to me! :) 
I'd still claim that consistency could be also achieved by consistently 
using dividers like / 1000 :)

In this particular case none apply strongly enough to force the issue.
Even so I'd prefer the units.h macros are used for the consistency reason
but I won't refuse to merge the driver over this.

One of those perfect (to me) being the enemy of good cases.

p.s. Matti, 3 versions in 4 days?  Perhaps slow down a little.

I should've indeed throttled sending the versions a bit. I just did 
fixes suggested by Andy/David immediately - and then I thought I could 
just do a decent change-log for reviewers, and send them while at it.

I didn't consider someone like You who will probably skim through all 
the versions and discussion. Sorry!

Thanks for the input!

Yours,
	-- Matti