Re: [PATCH v24 12/34] ASoC: doc: Add documentation for SOC USB

[Wesley Cheng <quic_wcheng@quicinc.com>]

Hi Pierre,

On 8/1/2024 1:26 AM, Pierre-Louis Bossart wrote:
>> +
>> +::
>> +
>> +               USB                   |            ASoC
>> +                                     |  _________________________
>> +                                     | |   ASoC Platform card    |
>> +                                     | |_________________________|
>> +                                     |         |           |
>> +                                     |      ___V____   ____V____
>> +                                     |     |ASoC BE | |ASoC FE  |
>> +                                     |     |DAI LNK | |DAI LNK  |
>> +                                     |     |________| |_________|
>> +                                     |         ^  ^        ^
>> +                                     |         |  |________|
>> +                                     |      ___V____    |
>> +                                     |     |SOC-USB |   |
>> +     ________       ________               |        |   |
>> +    |USB SND |<--->|USBSND  |<------------>|________|   |
>> +    |(card.c)|     |offld   |<----------                |
>> +    |________|     |________|___     | |                |
>> +        ^               ^       |    | |    ____________V_________
>> +        |               |       |    | |   |IPC                   |
>> +     __ V_______________V_____  |    | |   |______________________|
>> +    |USB SND (endpoint.c)     | |    | |              ^
>> +    |_________________________| |    | |              |
>> +                ^               |    | |   ___________V___________
>> +                |               |    | |->|audio DSP              |
>> +     ___________V_____________  |    |    |_______________________|
>> +    |XHCI HCD                 |<-    |
>> +    |_________________________|      |
>> +
> It wouldn't hurt to describe what you mean by 'port' in this diagram...
Sure, as mentioned in my earlier comments, in the USB world, port and device is kind of interchangeable, at least IMO.  I'd like to stick with the "port" term, but if you see otherwise, let me know. 
>
>> +SOC USB driver
>> +==============
>> +Structures
>> +----------
>> +``struct snd_soc_usb``
>> +
>> +  - ``list``: list head for SND SOC struct list
>> +  - ``component``: reference to ASoC component
>> +  - ``num_supported_streams``: number of supported concurrent sessions
>> +  - ``connection_status_cb``: callback to notify connection events
>> +  - ``get_offload_dev``: callback to fetch selected USB sound card/PCM device
> I think you meant fetch offloaded sound card and PCM device information
> for a given USB card:device pair?
Correct, will change.
>
>> +Functions
>> +---------
>> +.. code-block:: rst
>> +
>> +	const char *snd_soc_usb_get_components_tag(bool playback);
>> +..
>> +
>> +  - ``playback``: direction of audio stream
> why not use the usual direction 0: playback and 1: capture?
>
>> +
>> +**snd_soc_usb_get_components_tag()** returns the tag used for describing if USB
>> +offloading is supported for appending to a sound card's components string.
> How does this work if the ASoC part is probe after the USB card? The
> component string would be modified after the creation of the card?
>
> A control is more dynamic by nature, not sure about this component
> string. Jaroslav?
Do we actually need to add this?  I think just having the kcontrol is sufficient.
>> +**snd_soc_usb_add_port()** add an allocated SOC USB device to the SOC USB framework.
>> +Once added, this device can be referenced by further operations.
>> +
>> +.. code-block:: rst
>> +
>> +	void snd_soc_usb_remove_port(struct snd_soc_usb *usb);
>> +..
>> +
>> +  - ``usb``: SOC USB device to remove
>> +
>> +**snd_soc_usb_remove_port()** removes a SOC USB device from the SOC USB framework.
>> +After removing a device, any SOC USB operations would not be able to reference the
>> +device removed.
> Not clear to me if the notion of 'port' helps, why not just
> snd_soc_usb_add_device() and remove_device()?
I'm open to either terms, since both mean the same to me :).
>
>> +
>> +USB Offload Related Kcontrols
>> +=============================
>> +Details
>> +-------
>> +A set of kcontrols can be utilized by applications to help select the proper sound
>> +devices to enable USB audio offloading.  SOC USB exposes the get_offload_dev()
>> +callback that designs can use to ensure that the proper indices are returned to the
>> +application.
>> +
>> +Implementation
>> +--------------
>> +
>> +**Example:**
>> +
>> +  **Sound Cards**:
>> +
>> +	::
>> +
>> +	  0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D
>> +						SM8250-MTP-WCD9380-WSA8810-VA-DMIC
>> +	  1 [Seri           ]: USB-Audio - Plantronics Blackwire 3225 Seri
>> +						Plantronics Plantronics Blackwire 3225 Seri at usb-xhci-hcd.1.auto-1.1, full sp
>> +	  2 [C320M          ]: USB-Audio - Plantronics C320-M
>> +                      Plantronics Plantronics C320-M at usb-xhci-hcd.1.auto-1.2, full speed
>> +
>> +  **USB Sound Card** - card#1:
>> +
>> +	::
>> +
>> +	  USB Offload Playback Route PCM#0        -1, -1 (range -1->255)
>> +
>> +  **USB Sound Card** - card#2:
>> +
>> +	::
>> +
>> +	  USB Offload Playback Route PCM#0        0, 1 (range -1->255)
>> +
>> +The above example shows a scenario where the system has one ASoC platform card
>> +(card#0) and two USB sound devices connected (card#1 and card#2).  When reading
>> +the available kcontrols for each USB audio device, the following kcontrol lists
>> +the mapped offload path for the specific device:
>> +
>> +	"USB Offload Playback Route#*"
>> +
>> +The kcontrol is indexed, because a USB audio device could potentially have
>> +several PCM devices.  The above kcontrols are defined as:
>> +
>> +  - ``USB Offload Playback Route PCM`` **(R)**: Returns the ASoC platform sound
>> +	card and PCM device index.  The output "0, 1" (card index, PCM device index)
>> +	signifies that there is an available offload path for the USB SND device
>> +	through card#0-PCM device#1.  If "-1, -1" is seen, then no offload path is
>> +	available for the USB SND device.
>> +
>> +USB Offload Playback Route Kcontrol
>> +-----------------------------------
>> +In order to allow for vendor specific implementations on audio offloading device
>> +selection, the SOC USB layer exposes the following:
>> +
>> +.. code-block:: rst
>> +
>> +	int (*get_offload_dev)(struct snd_kcontrol *kcontrol,
>> +			      struct snd_ctl_elem_value *ucontrol);
>> +..
>> +
>> +These are specific for the **USB Offload Playback Route PCM#** kcontrol.
>> +
>> +When users issue get calls to the kcontrol, the registered SOC USB callbacks will
>> +execute the registered function calls to the DPCM BE DAI link.
> Oh man, now I get what 'get_offload_dev" means: it really means
> "update_offload_info' or 'update_info_kcontrol".
> The 'get' routines usually provide a handle on something to another part
> of the kernel.
> Not here, it's an update of something to be looked-up by userspace...
>
I can change the naming for this if its not the right terms used.  As long as you understand how the concept works then the name changing isn't a problem.

Thanks

Wesley Cheng

>> +**Callback Registration:**
>> +
>> +.. code-block:: rst
>> +
>> +	static int q6usb_component_probe(struct snd_soc_component *component)
>> +	{
>> +	...
>> +	usb = snd_soc_usb_allocate_port(component, 1, &data->priv);
>> +	if (IS_ERR(usb))
>> +		return -ENOMEM;
>> +
>> +	usb->connection_status_cb = q6usb_alsa_connection_cb;
>> +	usb->get_offload_dev = q6usb_get_offload_dev;
>> +
>> +	ret = snd_soc_usb_add_port(usb);
>> +..