svn commit: r308424 - head/sys/arm/broadcom/bcm2835

[Oleksandr Tymoshenko]

> On Nov 7, 2016, at 11:51 AM, Hans Petter Selasky <hps at selasky.org> wrote:
> 
> On 11/07/16 20:32, Hans Petter Selasky wrote:
>> On 11/07/16 20:23, Oleksandr Tymoshenko wrote:
>>> 
>>>> On Nov 7, 2016, at 10:27 AM, Hans Petter Selasky <hps at selasky.org>
>>>> wrote:
>>>> 
>>>> On 11/07/16 18:38, Oleksandr Tymoshenko wrote:
>>>>> +        bcm2835_audio_unlock(sc);
>>>>> +        cv_signal(&sc->worker_cv);
>>>> 
>>>> 
>>>> Shouldn't cv_signal() be done locked, so that you don't loose any
>>>> transactions? CV's only wakeup the treads that are sleeping right
>>>> there and then.
>>> 
>>> Hi Hans,
>>> 
>>> In this case it doesn’t matter. bcm2835_audio_xxx lock functions are
>>> used to keep channel state consistent. The actual audio hw
>>> reprogramming happens in worker thread which only picks up latest
>>> state of the virtual channel, there is no need to run every
>>> transaction in sequence.
>>> 
>> 
>> Hi,
>> 
>> It is not about running in sequence, but that if the worker thread is
>> not sleeping, but on the way to sleep, it will never get woken up unless
>> you use proper locks here!
>> 
>> --HPS
> 
> Hi,
> 
> Also the teardown sequence for the worker thread looks a bit broken, that it doesn't wait for the thread to exit.
> 
> I've made a patch, attached which I think is the right way to do it.
> 
> Try opening and closing /dev/dsp in a loop with some DSP ioctls and see what happens.

Thanks for patch Hans. Looks good to me.  I’ll test and commit it.