Re: [PATCH] usb: gadget: u_audio: initialize spinlocks

From: Michael Grzeschik <mgr@pengutronix.de>
To: Ruslan Bilovol <ruslan.bilovol@gmail.com>
Cc: laurent.pinchart@ideasonboard.com, linux-usb@vger.kernel.org,
	linux-media@vger.kernel.org, gregkh@linuxfoundation.org,
	john@keeping.me.uk, alsi@bang-olufsen.dk, kernel@pengutronix.de
Subject: Re: [PATCH] usb: gadget: u_audio: initialize spinlocks
Date: Tue, 3 Oct 2023 13:12:57 +0200	[thread overview]
Message-ID: <ZRv3Ocwc0c/sKaDk@pengutronix.de> (raw)
In-Reply-To: <CAB=otbTBNDNpg0B_WdCbCuj7qcoOdPCP2QE4az0J=KrHOTgfLg@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3161 bytes --]

On Mon, Oct 02, 2023 at 04:39:42AM -0400, Ruslan Bilovol wrote:
>On Mon, Oct 2, 2023 at 2:42 AM Michael Grzeschik
><m.grzeschik@pengutronix.de> wrote:
>>
>> When using uac2 with lockdep enabled, the kernel is throwing this
>> message, due to uninitialized spinlocks. We initialize them now.
>>
>> [   24.668867] The code is fine but needs lockdep annotation, or maybe
>> [   24.675878] you didn't initialize this object before use?
>> [   24.681910] turning off the locking correctness validator.
>> [   24.688038] CPU: 0 PID: 348 Comm: irq/43-dwc3 Tainted: G         C         6.5.0-20230919-1+ #14
>> [   24.697866] Hardware name: WolfVision PF5 (DT)
>> [   24.702831] Call trace:
>> [   24.705559]  dump_backtrace+0xac/0x130
>> [   24.709755]  show_stack+0x30/0x48
>> [   24.713456]  dump_stack_lvl+0x60/0xb0
>> [   24.717552]  dump_stack+0x18/0x28
>> [   24.721254]  register_lock_class+0x4e8/0x4f8
>> [   24.726029]  __lock_acquire+0x88/0x2130
>> [   24.730314]  lock_acquire+0x17c/0x338
>> [   24.734403]  _raw_spin_lock_irqsave+0x60/0x90
>> [   24.739274]  u_audio_get_capture_srate+0x34/0x68
>> [   24.744436]  afunc_setup+0x2d8/0x538
>> [   24.748431]  composite_setup+0x1a8/0x1ba8
>> [   24.752913]  configfs_composite_setup+0x88/0xc0
>> [   24.757974]  dwc3_ep0_interrupt+0x5e8/0xab8
>> [   24.762648]  dwc3_process_event_buf+0x424/0xbb0
>> [   24.767717]  dwc3_thread_interrupt+0x4c/0x90
>> [   24.772488]  irq_thread_fn+0x34/0xb8
>> [   24.776484]  irq_thread+0x1a0/0x290
>> [   24.780379]  kthread+0x10c/0x120
>> [   24.783985]  ret_from_fork+0x10/0x20
>>
>> Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
>> ---
>>  drivers/usb/gadget/function/u_audio.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
>> index 4a42574b4a7feb..9d9f906adf7c00 100644
>> --- a/drivers/usb/gadget/function/u_audio.c
>> +++ b/drivers/usb/gadget/function/u_audio.c
>> @@ -1172,6 +1172,9 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
>>         g_audio->uac = uac;
>>         uac->audio_dev = g_audio;
>>
>> +       spin_lock_init(&uac->c_prm.lock);
>> +       spin_lock_init(&uac->p_prm.lock);
>
>These locks are already initialized just a few lines below in this function.

It was probably to obvious, so I missed them.

>It seems the issue is because uac2 does u_audio_get_capture_srate() while
>capture isn't enabled? What is your UAC2 gadget configuration?

Yes, so probably the c_chmask is not setup while we still try to
work/configure the capture path.

There is actually no application of ours involved in setting the srate.

It might be possible that pipewire is changing things that should
left alone. I will check that assumption.

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]