On 11/16/21 23:31, Linus Walleij wrote:

On Tue, Nov 16, 2021 at 4:19 PM Vaittinen, Matti
[off-list ref] wrote:

quoted

[Me]

quoted

here I add a table to interpolate the temperature from an NTC
resistance to struct power_supply_battery_info.

Yep. Thanks for sharing this. I'd just hoped there was Kelvins used
instead of Celsius. Would also allow using negative temperatures as
errors in functions like the *resist2temp*().

It's not really an issue with these functions since they can't fail
and never return errors anyway.

It'd be a bit thick to start to using Kelvin somewhere in the kernel
when everyone and it's dog is using Celsius, including the
sysfs ABI to IIO. Also the datasheets I've seen are using Celsius
for operating conditions.

I suppose you know the saying: "If everyone else in the world thinks 
this should be done differently, it only means everyone else is wrong" ;)

I know Celsius are superior in everyday life (at least for us who have 
never used F. And for our dogs). But in code the Kelvin is superior - 
even if I am not saying you should be the odd one using it - it's still 
superior. For example, your res2temp() function did check the 
parameters. Good old -EINVAL suits almost all functions.

Yes yes. I know Kelvins are rarely used and break the existing style. I 
used Celsius myself with the degradation tables. So I am probably just 
rattling the chains :) But how cool would it be to use unsigned types 
without worrying about the negative values?

As for clarity of Kelvin - it's true it's more difficult to instantly 
know whether to wear the T-Shirt or a jacket at 293K - compared to 20C. 
But take an average engineer and he/she still figures it out in a no 
time. I'd guess even the average joe (who may not be an engineer) can 
figure it out if he has the motivation.

I'll make a bold claim for you - take that average engineer and ask 
him/her to put the negative temperature value in device-tree - and it'll 
take far longer from him to get it right than it takes to do the 
conversion to K ;)

If we need an error code it is better to pass the temperature
out in a *pointer argument and add a proper return code.

yes yes. There are ways around it I know. It's just that the Kelvins 
suit computing quite naturally :)

quoted

quoted

Apart from this I want to add:
maintainance charging A and B so essentially two voltage+current
levels after CV charging has finished, each with a safety timer so
2 * 3 new properties to struct power_supply_battery_info.
But I haven't coded it yet.

Ok. This does not sound like a complete rework :) I'll keep on eye what
you cook up here ;)

Nah. I was thinking of breaking out all the CC/CV charging params
to its own struct, but it is better to do that when someone needs
it.

I like this idea. I've not given much of thinking to it but it'd might 
also be clearer to also split the capacity estimation related stuff like 
the OCV tables in own struct.

All the best
	-- Matti

-- 
The Linux Kernel guy at ROHM Semiconductors

Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~ this year is the year of a signature writers block ~~