Hi Takashi,
On 11/20/2024 4:23 AM, Takashi Iwai wrote:
> On Wed, 06 Nov 2024 20:33:58 +0100,
> Wesley Cheng wrote:
>> USB SND needs to know how the USB offload path is being routed. This would
>> allow for applications to open the corresponding sound card and pcm device
>> when it wants to take the audio offload path. This callback should return
>> the mapped indexes based on the USB SND device information.
>>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
>> Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
>> ---
>> include/sound/soc-usb.h | 16 ++++++++++++++++
>> sound/soc/soc-usb.c | 34 ++++++++++++++++++++++++++++++++++
>> 2 files changed, 50 insertions(+)
>>
>> diff --git a/include/sound/soc-usb.h b/include/sound/soc-usb.h
>> index 587ea07a8cf5..c3d3e8d62ac5 100644
>> --- a/include/sound/soc-usb.h
>> +++ b/include/sound/soc-usb.h
>> @@ -36,6 +36,11 @@ struct snd_soc_usb_device {
>> * @list - list head for SND SOC struct list
>> * @component - reference to ASoC component
>> * @connection_status_cb - callback to notify connection events
>> + * @update_offload_route_info - callback to fetch mapped ASoC card and pcm
>> + * device pair. This is unrelated to the concept
>> + * of DAPM route. The "route" argument carries
>> + * an array used for a kcontrol output and should
>> + * contain two integers, card and pcm device index
>> * @priv_data - driver data
>> **/
>> struct snd_soc_usb {
>> @@ -44,6 +49,9 @@ struct snd_soc_usb {
>> int (*connection_status_cb)(struct snd_soc_usb *usb,
>> struct snd_soc_usb_device *sdev,
>> bool connected);
>> + int (*update_offload_route_info)(struct snd_soc_component *component,
>> + int card, int pcm, int direction,
>> + long *route);
>> void *priv_data;
>> };
>>
>> @@ -61,6 +69,8 @@ int snd_soc_usb_setup_offload_jack(struct snd_soc_component *component,
>> int snd_soc_usb_disable_offload_jack(struct snd_soc_component *component);
>> int snd_soc_usb_enable_offload_jack(struct snd_soc_component *component,
>> struct snd_soc_jack *jack);
>> +int snd_soc_usb_update_offload_route(struct device *dev, int card, int pcm,
>> + int direction, long *route);
>>
>> struct snd_soc_usb *snd_soc_usb_allocate_port(struct snd_soc_component *component,
>> void *data);
>> @@ -109,6 +119,12 @@ static inline int snd_soc_usb_enable_offload_jack(struct snd_soc_component *comp
>> return 0;
>> }
>>
>> +static int snd_soc_usb_update_offload_route(struct device *dev, int card, int pcm,
>> + int direction, long *route)
>> +{
>> + return -ENODEV;
>> +}
>> +
>> static inline struct snd_soc_usb *
>> snd_soc_usb_allocate_port(struct snd_soc_component *component, void *data)
>> {
>> diff --git a/sound/soc/soc-usb.c b/sound/soc/soc-usb.c
>> index ab914878e101..e56826f1df71 100644
>> --- a/sound/soc/soc-usb.c
>> +++ b/sound/soc/soc-usb.c
>> @@ -145,6 +145,40 @@ int snd_soc_usb_enable_offload_jack(struct snd_soc_component *component,
>> }
>> EXPORT_SYMBOL_GPL(snd_soc_usb_enable_offload_jack);
>>
>> +/**
>> + * snd_soc_usb_update_offload_route - Find active USB offload path
>> + * @dev - USB device to get offload status
>> + * @card - USB card index
>> + * @pcm - USB PCM device index
>> + * @direction - playback or capture direction
>> + * @route - pointer to route output array
>> + *
>> + * Fetch the current status for the USB SND card and PCM device indexes
>> + * specified. The "route" argument should be an array of integers being
>> + * used for a kcontrol output. The first element should have the selected
>> + * card index, and the second element should have the selected pcm device
>> + * index.
>> + */
>> +int snd_soc_usb_update_offload_route(struct device *dev, int card, int pcm,
>> + int direction, long *route)
>> +{
>> + struct snd_soc_usb *ctx;
>> + int ret = -EINVAL;
>> +
>> + ctx = snd_soc_find_usb_ctx(dev);
>> + if (!ctx)
>> + return -ENODEV;
>> +
>> + mutex_lock(&ctx_mutex);
>> + if (ctx && ctx->update_offload_route_info)
>> + ret = ctx->update_offload_route_info(ctx->component, card, pcm,
>> + direction, route);
>> + mutex_unlock(&ctx_mutex);
> The second ctx check is redundant. And the locking scheme looks
> dubious -- as ctx isn't protected by ctx_mutex after its retrieval via
> snd_soc_find_usb_ctx(), even if you reacquire ctx_mutex, it may point
> to an already released object (in theory).
>
> IOW, for a safer protection, you'd need to cover the whole
> find-and-exec procedure via a single ctx_mutex lock action.
>
That's fair, will make the change to move the mutexes around.
Thanks
Wesley Cheng
