linux-kernel - Re: [PATCH v23 32/32] ASoC: doc: Add documentation for SOC USB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <408d9e8e-0f40-7e66-54be-2f8d2c0783a3@quicinc.com>
Date: Wed, 12 Jun 2024 12:28:40 -0700
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Amadeusz Sławiński
	<amadeuszx.slawinski@...ux.intel.com>,
        <srinivas.kandagatla@...aro.org>, <mathias.nyman@...el.com>,
        <perex@...ex.cz>, <conor+dt@...nel.org>, <corbet@....net>,
        <broonie@...nel.org>, <lgirdwood@...il.com>, <krzk+dt@...nel.org>,
        <Thinh.Nguyen@...opsys.com>, <bgoswami@...cinc.com>, <tiwai@...e.com>,
        <robh@...nel.org>, <gregkh@...uxfoundation.org>
CC: <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-sound@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>, <linux-doc@...r.kernel.org>,
        <alsa-devel@...a-project.org>
Subject: Re: [PATCH v23 32/32] ASoC: doc: Add documentation for SOC USB

Hi Amadeusz,

On 6/12/2024 7:47 AM, Amadeusz Sławiński wrote:
> On 6/11/2024 1:58 AM, Wesley Cheng wrote:
> 
> (...)
> 
>> +In the case where the USB offload driver is unbounded, while USB SND is
> 
> unbounded -> unbound
> 
> (...)
> 
>> +SOC USB and USB Sound Kcontrols
>> +===============================
>> +Details
>> +-------
>> +SOC USB and USB sound expose a set of SND kcontrols for applications 
>> to select
>> +and fetch the current offloading status for the ASoC platform sound 
>> card. Kcontrols
>> +are split between two layers:
>> +
>> +    - USB sound - Notifies the sound card number for the ASoC 
>> platform sound
>> +      card that it is registered to for supporting audio offload.
>> +
>> +    - SOC USB - Maintains the current status of the offload path, and 
>> device
>> +      (USB sound card and PCM device) information.  This would be the 
>> main
>> +      card that applications can read to determine offloading 
>> capabilities.
>> +
>> +Implementation
>> +--------------
>> +
>> +**Example:**
>> +
>> +  **Sound Cards**:
>> +
>> +    ::
>> +
>> +      0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D
>> +                     SM8250-MTP-WCD9380-WSA8810-VA-DMIC
>> +      1 [C320M          ]: USB-Audio - Plantronics C320-M
>> +                     Plantronics Plantronics C320-M at 
>> usb-xhci-hcd.1.auto-1, full speed
>> +
>> +
>> +  **Platform Sound Card** - card#0:
>> +
>> +    ::
>> +
>> +      USB Offload Playback Route Card Select  1 (range -1->32)
>> +      USB Offload Playback Route PCM Select   0 (range -1->255)
>> +      USB Offload Playback Route Card Status  -1 (range -1->32)
>> +      USB Offload Playback Route PCM Status   -1 (range -1->255)
>> +
>> +
>> +  **USB Sound Card** - card#1:
>> +
>> +    ::
>> +
>> +      USB Offload Playback Capable Card         0 (range -1->32)
>> +
>> +
>> +The platform sound card(card#0) kcontrols are created as part of 
>> adding the SOC
>> +USB device using **snd_soc_usb_add_port()**.  The following kcontrols 
>> are defined
>> +as:
>> +
>> +  - ``USB Offload Playback Route Card Status`` **(R)**: USB sound 
>> card device index
>> +    that defines which USB SND resources are currently offloaded.  If 
>> -1 is seen, it
>> +    signifies that offload is not active.
>> +  - ``USB Offload Playback Route PCM Status`` **(R)**: USB PCM device 
>> index
>> +    that defines which USB SND resources are currently offloaded.  If 
>> -1 is seen, it
>> +    signifies that offload is not active.
>> +  - ``USB Offload Playback Route Card Select`` **(R/W)**: USB sound 
>> card index which
>> +    selects the USB device to initiate offloading on.  If no value is 
>> written to the
>> +    kcontrol, then the last USB device discovered card index will be 
>> chosen.
> 
> I see only one kcontrol, what if hardware is capable of offloading on 
> more cards, is it possible to do offloading on more than one device?
> 
>> +  - ``USB Offload Playback Route PCM Select`` **(R/W)**: USB PCM 
>> index which selects
>> +    the USB device to initiate offloading on.  If no value is written 
>> to the
>> +    kcontrol, then the last USB device discovered PCM zero index will 
>> be chosen.
>> +
>> +The USB sound card(card#1) kcontrols are created as USB audio devices 
>> are plugged
>> +into the physical USB port and enumerated.  The kcontrols are defined 
>> as:
>> +
>> +  - ``USB Offload Playback Capable Card`` **(R)**: Provides the sound 
>> card
>> +    number/index that supports USB offloading.  Further/follow up 
>> queries about
>> +    the current offload state can be handled by reading the offload 
>> status
>> +    kcontrol exposed by the platform card.
>> +
> 
> 
> Why do we need to some magic between cards? I feel like whole kcontrol 
> thing is overengineered a bit - I'm not sure I understand the need to do 
> linking between cards. It would feel a lot simpler if USB card exposed 
> one "USB Offload" kcontrol on USB card if USB controller supports 
> offloading and allowed to set it to true/false to allow user to choose 
> if they want to do offloading on device.
> 
> (...)

Based on feedback from Pierre, what I understood is that for some 
applications, there won't be an order on which sound card is 
queried/opened first.

So the end use case example given was if an application opened the USB 
sound card first, it can see if there is an offload path available.  If 
there is then it can enable the offload path on the corresponding card 
if desired.

>> +Mixer Examples
>> +--------------
>> +
>> +    ::
>> +
>> +      tinymix -D 0 set 'USB Offload Playback Route Card Select' 2
>> +      tinymix -D 0 set 'USB Offload Playback Route PCM Select' 0
>> +
>> +
>> +    ::
>> +
>> +      tinymix -D 0 get 'USB Offload Playback Route Card Select'
>> +      --> 2 (range -1->32)
>> +      tinymix -D 0 get 'USB Offload Playback Route PCM Select'
>> +      --> 0 (range -1->255)
>> +
>> +    ::
>> +
>> +      tinymix -D 0 get 'USB Offload Playback Route Card Status'
>> +      --> 2 (range -1->32)   [OFFLD active]
>> +      --> -1 (range -1->32) [OFFLD idle]
>> +      tinymix -D 0 get 'USB Offload Playback Route PCM Status'
>> +      --> 0 (range -1->255)   [OFFLD active]
>> +      --> -1 (range -1->255) [OFFLD idle]
>> +
>> +    ::
>> +
>> +      tinymix -D 1 get 'USB Offload Playback Capable Card'
>> +      --> 0 (range -1->32)
>>
> 
> Yes, looking at examples again, I'm still not sure I understand. There 
> are two cards and you do linking between them, this feels broken by 
> design. From my point of view USB Offload should be property of USB card 
> and not involve any other card in a system.
> 

Main benefit to having two cards (keeping one for USB SND and another 
for the ASoC platform sound card) is that current applications won't 
break.  The behavior is the same, in that if something opens the USB 
sound card, it will go through the same non-offloaded path.  During 
initial reviews, I think this was a big point where folks wanted the USB 
PCM path to still be an option.

If applications want to add the offload capabilities to its environment, 
they can enable it as an additional feature.

Thanks
Wesley Cheng