usb controls for level/volume and mute now get applied

[mryndzionek]

I think than access to usb_volume and usb_mute might need locking.

[penfold42]

I think than access to usb_volume and usb_mute might need locking.

How come ? are usb and rx_dsp on separate cores ?

[mryndzionek]

I think than access to usb_volume and usb_mute might need locking.

How come ? are usb and rx_dsp on separate cores ?

No, but on_usb_set_mutevol will, most likely, be called from usb_audio_device_task, which is run from an interrupt. on_usb_audio_tx_ready only communicates via a ring buffer, which has locking.

[penfold42]

There's only 1 writer/1 reader and they're each less than 32 bit objects. Can i be lazy and mark them volatile?

[mryndzionek]

There's only 1 writer/1 reader and they're each less than 32 bit objects. Can i be lazy and mark them volatile?

Yes, sure, we can try, should be atomic.

[penfold42]

It seemed to work fine and continues to work fine with volatile.

[mryndzionek]

It seemed to work fine and continues to work fine with volatile.

With concurrency issues it always seems to work fine at the beginning :wink:

[penfold42]

If you still want me to add the locking, is this approach IRQ safe ?

sem_acquire_blocking(&usbcontrol_semaphore);
sem_release(&usbcontrol_semaphore);

[mryndzionek]

If you still want me to add the locking, is this approach IRQ safe ?

sem_acquire_blocking(&usbcontrol_semaphore);
sem_release(&usbcontrol_semaphore);

I think a critical section is appropriate in this case: https://www.raspberrypi.com/documentation/pico-sdk/high_level.html#group_critical_section

[penfold42]

Thanks - please have a peek if you have a moment.

[mryndzionek]

Looks good and works okay on Linux :+1:

[dawsonjon]

Cheers

[penfold42]

oh excellent - thanks for checking that. Does alsamixer behave sensibly?

[mryndzionek]

oh excellent - thanks for checking that. Does alsamixer behave sensibly?

Yes.