Couldn't wait for my Pyra, so I bought a OMAP5432 devboard

[hns]

Ok, I have fixed the boot issue and now have a letux-6.1-rc2 kernel running with display

Yes, volume wheel needs the channel 2 and then it works:

root@letux:~/pyra_vol_mon# ./pyra_vol_mon 2 /bin/echo
Found IIO device with name palmas-gpadc with device number 4
666
704
736
775
815
850
884
922
951
871
818
800
781
744
710
686
651
627
610
580
^C
root@letux:~/pyra_vol_mon# uname -a
Linux letux 6.1.0-rc2-letux-lpae+ #11029 SMP PREEMPT Sun Oct 30 22:41:25 CET 2022 armv7l GNU/Linux
root@letux:~/pyra_vol_mon# cat /proc/device-tree/mode
model            modem/           modem_power_reg/ 
root@letux:~/pyra_vol_mon# cat /proc/device-tree/model 
Pyra-Handheld-V5.3
root@letux:~/pyra_vol_mon#

 

[Risca]

Nice! I'm working on improving the tool with support for a config file and sensible defaults for the pyra, and maybe some better cleanup on module unload so it resets the events. Unfortunately, my volume wheel came off so I'll have to resolder it to test. Hopefully, the PCB pad didn't come off this time

 

[hns]

Nice! I'm working on improving the tool with support for a config file and sensible defaults for the pyra, and maybe some better cleanup on module unload so it resets the events. Unfortunately, my volume wheel came off so I'll have to resolder it to test. Hopefully, the PCB pad didn't come off this time

One more idea (unless it is already there): the tool should call the subprocess once with the current value when the monitor daemon is started. The idea behind is that someone may have turned the wheel while the daemon wasn't running (e.g. power off/reboot) and expects the mechanical value to be active right from the beginning and not after the first movement.
Regarding a config file: I am more a friend of command line options... This makes no name of a config file hard-coded into the binary or adds no dependency on external libs.

 

[Risca]

One more idea (unless it is already there): the tool should call the subprocess once with the current value when the monitor daemon is started. The idea behind is that someone may have turned the wheel while the daemon wasn't running (e.g. power off/reboot) and expects the mechanical value to be active right from the beginning and not after the first movement.
Regarding a config file: I am more a friend of command line options... This makes no name of a config file hard-coded into the binary or adds no dependency on external libs.

Yeah, that makes a lot of sense. Time to dust off the old getopts() man page

 

[paralabamba]

Niiice ! One more thread who makes me loving waiting for the Pyra

 

[Risca]

One more idea (unless it is already there): the tool should call the subprocess once with the current value when the monitor daemon is started. The idea behind is that someone may have turned the wheel while the daemon wasn't running (e.g. power off/reboot) and expects the mechanical value to be active right from the beginning and not after the first movement.
Regarding a config file: I am more a friend of command line options... This makes no name of a config file hard-coded into the binary or adds no dependency on external libs.

Okay, I've updated the repo with your suggestions: https://github.com/Risca/pyra_vol_mon

It should work to just run

./pyra_vol_mon /bin/echo

The program takes the following options:

Usage: pyra_vol_mon [OPTION]... EXECUTABLE

Monitor palmas_gpadc ADC channel and run EXECUTABLE when input changes.

  -c, --channel     ADC channel to monitor. The Pyra volume wheel is
                    connected to ADC channel 2.
                    Defaults to 2.
  -l, --min         Lower limit to monitor. ADC values below this limit will
                    only trigger executable once, until the ADC channel goes
                    above the limit again.
                    Defaults to 0.
  -u, --max         Upper limit to monitor. ADC values above this limit will
                    only trigger executable once, until the ADC cannel goes
                    below the limit again.
                    Defaults to 0x7FF.
  -s, --step        How much the ADC input value is allowed to change before
                    calling the executable again.
                    Defaults to 25.
  -h, --help        Show this help text and exit
  -v, --verbose     Be a bit more verbose.

EXECUTABLE will be called like this:

    EXECUTABLE <adc value>

NOTE:
I wrote these changes on a train without access to my devboard so I haven't actually tested it on real hardware, but it should work

 

[hns]

Hi,
have pulled the files and they compile fine (there were just some whitespace errors).
Unfortunately it does not run:

root@letux:~# /bin/sh -c 'cd /root/pyra_vol_mon && make && ./pyra_vol_mon -c 2 /root/pyra_vol_mon/pyra_vol_ctl'
gcc -MT .objs/pyra_vol_mon.o -MMD -MP -MF .deps/pyra_vol_mon.d  -D_GNU_SOURCE -c -o .objs/pyra_vol_mon.o pyra_vol_mon.c
gcc -MT .objs/iio_utils.o -MMD -MP -MF .deps/iio_utils.d  -D_GNU_SOURCE -c -o .objs/iio_utils.o iio_utils.c
gcc -MT .objs/config.o -MMD -MP -MF .deps/config.d  -D_GNU_SOURCE -c -o .objs/config.o config.c
gcc -MT .objs/iio_event.o -MMD -MP -MF .deps/iio_event.d  -D_GNU_SOURCE -c -o .objs/iio_event.o iio_event.c
gcc -MT .objs/main.o -MMD -MP -MF .deps/main.d  -D_GNU_SOURCE -c -o .objs/main.o main.c
gcc -lc .objs/pyra_vol_mon.o .objs/iio_utils.o .objs/config.o .objs/iio_event.o .objs/main.o -o pyra_vol_mon
Found IIO device with name palmas-gpadc with device number 2
*** Error in `./pyra_vol_mon': malloc(): memory corruption: 0x00415420 ***
Aborted
root@letux:~#

If I find time I can compile with debugging info and run gdb...
Note: pyra_vol_ctl is something I have added to the Letux setup. This script looks up the sound card and calls some amixer commands to control volume.

 

[hns]

Ok, here is the backtrace:

(gdb) r -c 2 /bin/echo
Starting program: /root/pyra_vol_mon/pyra_vol_mon -c 2 /bin/echo
Found IIO device with name palmas-gpadc with device number 2
*** Error in `/root/pyra_vol_mon/pyra_vol_mon': malloc(): memory corruption: 0x00415420 ***

Program received signal SIGABRT, Aborted.
__libc_do_syscall () at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47
47    ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S: No such file or directory.
(gdb) bt
#0  __libc_do_syscall () at ../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47
#1  0xb6f0d3cc in __libc_signal_restore_set (set=0xbefff730) at ../sysdeps/unix/sysv/linux/nptl-signals.h:79
#2  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:48
#3  0xb6f0e0ba in __GI_abort () at abort.c:89
#4  0xb6f34cda in __libc_message (do_abort=do_abort@entry=2, fmt=<optimized out>) at ../sysdeps/posix/libc_fatal.c:175
#5  0xb6f391ca in malloc_printerr (action=<optimized out>, str=0xb6fb785c "malloc(): memory corruption", ptr=<optimized out>, ar_ptr=<optimized out>) at malloc.c:5049
#6  0xb6f3ab88 in _int_malloc (av=av@entry=0xb6fd37a4 <main_arena>, bytes=bytes@entry=51) at malloc.c:3512
#7  0xb6f3c342 in __GI___libc_malloc (bytes=51) at malloc.c:2928
#8  0x004022e0 in read_sysfs_posint (filename=0x4154a0 "in_voltage2_input", basedir=0x415478 "/sys/bus/iio/devices/iio:device2") at iio_utils.c:864
#9  0x00402d8e in pyra_iio_get_value (handle=0x415410) at iio_event.c:201
#10 0x00400d28 in read_value_and_update_thresholds (config=0xbefffbac, iio=0x415410) at pyra_vol_mon.c:14
#11 0x00402f76 in main (argc=4, argv=0xbefffd44) at main.c:62
(gdb)

Note that here the memory is already corrupted so the bug occurs earlier.

 

[hns]

Seems as if it should be

h = (struct pyra_iio_event_handle*)malloc(sizeof(*h));

 

[Risca]

Seems as if it should be

h = (struct pyra_iio_event_handle*)malloc(sizeof(*h));

Ah, yes, of course! Fix pushed to the repo now.

I'll see if I can add any more unit tests for this, and test it out myself when I get home again tomorrow

 

[Risca]

Note: pyra_vol_ctl is something I have added to the Letux setup. This script looks up the sound card and calls some amixer commands to control volume.

I'll take a look at this script!

 

[hns]

Here is the script (and bug fixes) included: https://git.goldelico.com/?p=letux-...0369eaa24e078d64a;hb=refs/heads/letux-6.1-rc4

A minor weakness is that the gpadc rarely reports the volume of exactly "$VOL" = 0. Especially if the previous value was only slightly above 0. Then moving the wheel to 0 seems to not leave the threshold window. The result is that we do not switch off the Handsfree DACs and this draws more energy from the battery.

 

[Risca]

Here is the script (and bug fixes) included: https://git.goldelico.com/?p=letux-...0369eaa24e078d64a;hb=refs/heads/letux-6.1-rc4

CARD="$($SCRIPTPATH/findsoundcard -a | head -1)"    # first channel
CARD=L15    # this is the sound card...

A minor weakness is that the gpadc rarely reports the volume of exactly "$VOL" = 0. Especially if the previous value was only slightly above 0. Then moving the wheel to 0 seems to not leave the threshold window. The result is that we do not switch off the Handsfree DACs and this draws more energy from the battery.

Yeah, that one is tricky. Unless I disabled the lower threshold trigger, it would repeatedly wake me up again. I believe there was some kind of unit conversion between raw ADC values and mVs That could possibly cause some issue. The threshold values all work on mV and I think the gpadc chipset expects raw threshold values. Maybe I could change the userspace program to report anything close to zero as 0? Or maybe anything below min?
I could of course change the code to only work with raw values, but that's not great either. Every ADC is different, and the conversion code takes factory calibration data into account so the result should be more consistent across devices.

On another note, can't the driver be changed so that DACs gets turned off as soon as no application is using audio? Isn't that the whole purpose of the Linux DAPM subsystem?

 

[hns]

CARD="$($SCRIPTPATH/findsoundcard -a | head -1)"    # first channel
CARD=L15    # this is the sound card...

Yes... some boards also have HDMI audio an there is no reliable way to find the headset/speaker connected sound card.
BTW: it is something I have not yet understood: why the card name is "L15". There is no "L15" in the whole kernel tree... Maybe some magic algorithm.

Yeah, that one is tricky. Unless I disabled the lower threshold trigger, it would repeatedly wake me up again. I believe there was some kind of unit conversion between raw ADC values and mVs That could possibly cause some issue. The threshold values all work on mV and I think the gpadc chipset expects raw threshold values. Maybe I could change the userspace program to report anything close to zero as 0? Or maybe anything below min?
I could of course change the code to only work with raw values, but that's not great either. Every ADC is different, and the conversion code takes factory calibration data into account so the result should be more consistent across devices.

Well, my old volumed was fine by polling to get a "0".
Usually the factory calibration plus noise made the input voltage even report negative values an complain in kernel log. Fortunately a patch was finally accepted (a8e1f0ba138e0).
But the issue is not conversion from ADC values to mV. It is that if the user turns the wheel down it may be that it reports 50 - 30 - 10 - but not finally 0, like the polling volumed did.
Maybe it is that the logic of read_value_and_update_thresholds() is not yet perfect for this corner case.

On another note, can't the driver be changed so that DACs gets turned off as soon as no application is using audio? Isn't that the whole purpose of the Linux DAPM subsystem?

I think this is done in addition, but they should also be turned off while playing music and the user moves the wheel to 0.
The real issue behind this is that (unless we get AESS working one day) there is a DC offset of the H-bridge amplifiers which makes a constant current flowing through the speakers even for 0 level audio. Only AESS can handle this DC offset. Or turning off the DACs.

 

[Risca]

Yes... some boards also have HDMI audio an there is no reliable way to find the headset/speaker connected sound card.
BTW: it is something I have not yet understood: why the card name is "L15". There is no "L15" in the whole kernel tree... Maybe some magic algorithm.

Well, my old volumed was fine by polling. Usually the factory calibration plus noise made the input voltage even report negative values. Fortunately my patch was finally accepted (a8e1f0ba138e0).
But the issue is not conversion from ADC values to mV. It is that if the user turns the wheel down it may be that it reports 50 - 30 - 10 - but not finally 0, like the polling volumed did.
Maybe it is that the logic of read_value_and_update_thresholds() is not yet perfect for this corner case.

Yes, I had an issue when mV reached below the step (defaults to 25) value. The low threshold value is calculated as:

low_threshold = MAX(0, current_mv - step)

This effectively clamps the value to 0 or higher. However, setting the lower threshold to 0 led to continuous interrupts so I just disabled the lower threshold if it's below the step value. You can adjust the step value with --step <mV>. I found 25 to be stable on my board, but my volume wheel might be less precise than the Pyra. I guess it would kind of make sense to report 0 mV if the real reading is "close to 0". Or maybe introduce a new variable called --cutoff <mV>?

I think this is done in addition, but they should also be turned off while playing music and the user moves the wheel to 0.
The real issue behind this is that (unless we get AESS working one day) there is a DC offset of the H-bridge amplifiers which makes a constant current flowing through the speakers even for 0 level audio. Only AESS can handle this DC offset. Or turning off the DACs.

Got it!

I was thinking of having the application be able to adjust the volume itself, so it doesn't need an external executable. With the risk of starting a holy war: which sound system should it support? Pure ALSA? Pulseaudio? PipeWire? Jack? No?

 

[hns]

I am a friend of a modular approach...
So one daemon decoding the wheel and calling another tool to do something. And a third mechanism to set this up either on boot or on demand. This seems to be the most flexible approach and would already be reached

 

[Djoga'Ro]

Do deciding the lower threshold and deciding the value you output exist in the same component/process? Then disable the threshold, iff you get to output 0. ?

 

[Risca]

I am a friend of a modular approach...
So one daemon decoding the wheel and calling another tool to do something. And a third mechanism to set this up either on boot or on demand. This seems to be the most flexible approach and would already be reached

Yeah, I guess you're right. I'm a sucker for premature optimization and sometimes I need to get a hold of myself.

Do deciding the lower threshold and deciding the value you output exist in the same component/process? Then disable the threshold, iff you get to output 0. ?

Yes, both reading the value and calculating the new lower threshold is part of the same function. The problem was that for values close to 0, the calculated lower threshold would become 0 as well. This cause the gpadc to automatically trigger another interrupt as fast as possible. But maybe this would work?

int value = pyra_iio_get_value();
if (value <= 0)
    pyra_iio_disable_lower_threshold();
else {
    int threshold = value - step;
    if (threshold < min)
        threshold = min;
    if (threshold <= 0)
        threshold = 1;
    pyra_iio_enable_lower_threshold(threshold);
}

(code has been simplified a bit for this example)

Unfortunately, my volume wheel has come off my PCB (again) so I would need someone else to test it for me. @hns, would you mind?
I've updated https://github.com/Risca/pyra_vol_mon

I'll fix my own volume wheel eventually

 

[Djoga'Ro]

/* update lower threshold */
    if (value == 0 || value <= config->min) {
        ret = pyra_iio_disable_lower_threshold(iio);

Here you should also set value to 0. ?

 

[Risca]

/* update lower threshold */
    if (value == 0 || value <= config->min) {
        ret = pyra_iio_disable_lower_threshold(iio);

Here you should also set value to 0. ?

Hmm... Maybe. Might be odd if values jump from near min to 0. The output value could also be scaled between min and max, but then you loose what real mV it was