diff mbox series

[v30,15/30] ASoC: usb: Fetch ASoC card and pcm device information

Message ID 20241106193413.1730413-16-quic_wcheng@quicinc.com (mailing list archive)
State New
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng Nov. 6, 2024, 7:33 p.m. UTC 
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@linux.intel.com>
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 include/sound/soc-usb.h | 16 ++++++++++++++++
 sound/soc/soc-usb.c     | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

Comments

Takashi Iwai Nov. 20, 2024, 12:23 p.m. UTC | #1 
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@linux.intel.com>
> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
> ---
>  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.


thanks,

Takashi
Wesley Cheng Nov. 20, 2024, 10:36 p.m. UTC | #2 
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@linux.intel.com>
>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
>> ---
>>  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
diff mbox series

Patch

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);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_usb_update_offload_route);
+
 /**
  * snd_soc_usb_find_priv_data() - Retrieve private data stored
  * @usbdev: device reference