andy-shev/linux

[Feature Request] Edison: Battery level reporting

vpelletier opened this issue · 21 comments

I see you are working on the power button lately. If I may suggest a likely-related (on which-chip-does-what level) feature I would like to see working on the edison, it is getting battery level and power source information. I believe these come from the TI-developed, Intel-specific, specs-unavailable PMIC.

It's out of my scope, but what we can do is a collaboration in the way you port forward a cleaned version of the feature from official repository. I would answer to (some) questions related to PMIC. Would it be suitable for you?

BTW, I have SparkFun battery block I can run some tests on.

Yep, fine for me, thanks for the collaboration offer ! I should have time to work on this this week-end.

@vpelletier : this has been done under Yocto as well, see https://github.com/cjo20/EdisonVoltage
Might be usefull for you.

Okay, from that we can conclude we need to have ADC driver for PMIC.

I started reading drivers/power/intel_mid_battery.c from 01org's edison-linux, but I now see this was removed here, with a commit message stating it was a leftover from Morestown.

This, plus the (very interesting) report from @PieterGit makes me less sure of what I actually have to move around: I kind of expected something like the code I read from above driver, plus maybe some standard way of reporting charge level which userland apps could pick up more easily than the start measure/read result dance done in EdisonVoltage (though they obviously had little choice on how kernel exposed the feature, of course).

So... adc only for now, it is ?

Porting commits, in the original initialisation code I find register_rpmsg_service (comming from intel_mid_remoteproc). There does not seem to be other uses of this in current code. How should I proceed for this ?

[edit] Actually, reading more of the code, it seems the rpmsg is only used as a way to load the device, the only (?) messaging function only complaining when called. So I'm getting confident this should be dropped.

I pushed a first patch stack.

On the revision tracking side of things, I cherry-picked the commits from 01org/edison-linux branch edison-3.10.17, removed the pieces which were irrelevant but kept the original commit messages & sign-offs. Then I started my changes in a separate commit.

On the code side of things, the module does not get automatically loaded, but once manually loaded it appears in sysfs and seems working:

root@tesla:/sys/devices/platform/bcove_adc.0/basincove_gpadc# echo 0x1ff > channel
root@tesla:/sys/devices/platform/bcove_adc.0/basincove_gpadc# echo 1 > sample
root@tesla:/sys/devices/platform/bcove_adc.0/basincove_gpadc# cat result
sample_result[0] = 937
sample_result[1] = 1023
sample_result[2] = 511
sample_result[3] = 0
sample_result[4] = 0
sample_result[5] = 0
sample_result[6] = 0
sample_result[7] = 0
sample_result[8] = 0

I think most of the platform data is useless currently, as channel names are not exposed (...or I just couldn't identify how to retrieve them ?).

Actually v3.10.98 is latest official base. Does it work there? If so, we need best from them (either v3.10.17, or v3.10.98).

@vpelletier : did you manage to get any further with this issue?

@PieterGit : My work so far is on my fork, where I create new branches based on @andy-shev's eds branch from time to time to work on this.

The work is not finished yet. It is basically functioning, but is not at the right quality level to be upstreamed, and some channels are not always refreshed (there is a suspicion around IRQ handling beyond just the ADC, IIRC, which I did not look into). I would also like to get better understanding of what exactly is being measured. For example, "current2" input (see below) is always siting very near 512, so I suspect it is the charge/discharge current of the short-lived battery which can be connected to the edison allowing a power source change. I suspect the voltage input is similarly measured. There are also 2 temperature and the resistance inputs which have values which I did not investigate at all so far.

I have started a wiki page to document what I understand from the chip through code and measured values.

There was a long hiatus as I was/am traveling, so progress is slow at the moment.

FWIW, I intend to not keep the sysfs interface used in the original module. Instead, I fixed the IndusIO interfacing, and iio_info command (from libiio-utils package on a debian-ish) now show stuff:

vincent@tesla:~$ iio_info 
Library version: 0.7 (git tag: v0.7)
IIO context created with local backend.
Backend version: 0.7 (git tag: v0.7)
Backend description string: Linux tesla 4.11.0-rc3-edison+ #24 SMP Sat Mar 25 07:16:53 JST 2017 i686
IIO context has 1 devices:
        iio:device0: basin_cove_adc
                9 channels found:
                        temp6:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 263
                        voltage0:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 1002
                        temp3:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 468
                        temp7:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 0
                        temp4:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 0
                        temp8:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 0
                        resistance1:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 1023
                        temp5:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 0
                        current2:  (input)
                        1 channel-specific attributes found:
                                attr 0: raw value: 510

Besides above Hans de Goede is doing a lot of work regarding to CherryTrail PMICs, such as Whiskey Cove and he also pointed out to design flaw in support of that PMIC for Broxton. I suspect that all upstreamed PMIC drivers are affected and need to be redesigned. Thus, @vpelletier 's approach needs to be reviewed more carefully.

@andy-shev and @vpelletier also the jubilinux efforts might be usefull for getting the voltage stuff working. See https://github.com/jubilinux/edison-linux-helper (@esialb is working on that)

@PieterGit thanks for information, unfortunately it is unrelated to newer kernels.

@PieterGit many thanks for the plug, but I'm no kernel developer. Most of what I've done regarding the edison-linux kernel is make it easy to build, including updating dfu images.

Latest branch (with ACPI support enabled) has PMIC initial driver and its GPADC. Can you try it?

I will try to get to it this week. Do you have a sample kernel config? Or can I just reuse the Jubilinux default?

@esialb, I think better to ask @htot for this. I have two branches: edison-acpi in U-Boot and eds-acpi in Linux kernel repositories. That's what I'm working with on daily basis. As kernel configuration I'm using x86_64_defconfg.

htot commented

This with kernel 5.0.0-rc8 and Yocto Thud, with spidev and arduino tables (looks like I need to enable ads7951 too):

root@edison:~# iio_info 
Library version: 0.14 (git tag: v0.14)
Compiled with backends: local xml ip usb
IIO context created with local backend.
Backend version: 0.14 (git tag: v0.14)
Backend description string: Linux edison 5.0.0-rc8-edison-acpi-standard #1 SMP Sat Mar 2 21:07:00 UTC 2019 i686
IIO context has 1 attributes:
        local,kernel: 5.0.0-rc8-edison-acpi-standard
IIO context has 3 devices:
        iio:device1: ads7951 (buffer capable)
                9 channels found:
                        voltage0:  (input, index: 0, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw value: 8
                                attr  1: scale value: 2.442002442
                        voltage1:  (input, index: 1, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage2:  (input, index: 2, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage3:  (input, index: 3, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage4:  (input, index: 4, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage5:  (input, index: 5, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage6:  (input, index: 6, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        voltage7:  (input, index: 7, format: le:u12/16>>0)
                        2 channel-specific attributes found:
                                attr  0: raw ERROR: Input/output error (-5)
                                attr  1: scale value: 2.442002442
                        timestamp:  (input, index: 8, format: le:S64/64>>0)
                1 device-specific attributes found:
                                attr  0: current_timestamp_clock value: realtime

                2 buffer-specific attributes found:
                                attr  0: watermark value: 1
                                attr  1: data_available value: 0
        iio:device0: mrfld_bcove_adc
                9 channels found:
                        temp6:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 355
                        voltage0:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 979
                        temp3:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 475
                        temp7:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 1
                        temp4:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 1
                        temp8:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 1
                        resistance1:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 1022
                        temp5:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 1
                        current2:  (input)
                        1 channel-specific attributes found:
                                attr  0: raw value: 509
        iio_sysfs_trigger:
                0 channels found:
                2 device-specific attributes found:
                                attr  0: add_trigger ERROR: Permission denied (-13)
                                attr  1: remove_trigger ERROR: Permission denied (-13)
htot commented

Close this one? I think it's resolved.

To me also. In case it's not, feel free to reopen with an additional information.