Re: [PATCH v3] ASoC: amd: acp: Initialize list to store acp_stream during pcm_open

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/29/2022 4:19 PM, Takashi Iwai wrote:
Thanks for your time.
[CAUTION: External Email]

On Fri, 29 Jul 2022 12:34:51 +0200,
Venkata Prasad Potturu wrote:
On 7/28/22 18:19, Mark Brown wrote:
Thanks for your time.

     On Thu, Jul 28, 2022 at 06:10:50PM +0530, Venkata Prasad Potturu wrote:

         @@ -104,14 +105,13 @@ static irqreturn_t i2s_irq_handler(int irq, void *data)

               ext_intr_stat = readl(ACP_EXTERNAL_INTR_STAT(adata, rsrc->irqp_used));

         -     for (i = 0; i < ACP_MAX_STREAM; i++) {
         -                           stream = adata->stream[i];
         +     spin_lock_irqsave(&adata->acp_lock, flags);
         +     list_for_each_entry(stream, &adata->stream_list, list) {

     If we're already in an interrupt handler here (presumably not a threaded
     one) why are we using irqsave?

Yes, your statement make sense, I have followed below statement in kernel
document. so used irqsave in interrupt context as well.

We will change it to spin_lock() and send it in the next version.

statement:- spin_lock_irqsave() will turn off interrupts if they are on,
otherwise does nothing (if we are already in an interrupt handler), hence
these functions are safe to call from any context.
Also the open and close callbacks are certainly non-irq context, hence
you can use spin_lock_irq() instead of irqsave(), too.

One doubt, if we use *spin_lock_irq()* it will do *local_irq_disable() *even if interrupts are already disabled, and when we call *spin_unlock_irq() *after critical section it  will forcibly re-enable interrupts in a potentially unwanted manner.

If we use *spin_lock_irqsave();* and *spin_unlock_irqrestore(); *will save and restore the interrupt state.

My understanding is *spin_lock_irqsave();* is better instead of improper interrupt enable and disable as like *spin_lock_irq();*

Could you please let me know is there any other thing to use *spin_lock_irq(); *particularly here.

Takashi



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux