Hi Pierre,
On 7/25/2023 1:27 AM, Pierre-Louis Bossart wrote:
+static const struct snd_soc_dai_ops q6usb_ops = {
- .prepare = q6afe_dai_prepare,
- .hw_params = q6usb_hw_params,
- .shutdown = q6afe_dai_shutdown,
it's a bit odd to see a .shutdown without a .startup?
Is this intentional and should a comment be added?
That is correct, because the Q6AFE port start command needs to also pass in the PCM params, so we wait for the hw_params() callback before actually starting the port. I will add a comment.
+/* device token of actual end USB aduio device */
audio
- u32 dev_token;
+/* endianness of this interface */
- u32 endian;
+/* service interval */
- u32 service_interval;
+} __packed;
+/**
- struct afe_param_id_usb_audio_dev_params
- @cfg_minor_version: Minor version used for tracking USB audio device
- configuration.
- Supported values:
AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG
- @dev_token: device token of actual end USB aduio device
audio. please run a spell-checker.
Will fix the typos.
- svc_int.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
- svc_int.svc_interval = pcfg->usb_cfg.service_interval;
- ret = q6afe_port_set_param_v2(port, &svc_int,
AFE_PARAM_ID_USB_AUDIO_SVC_INTERVAL,AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(svc_int));- if (ret) {
dev_err(port->afe->dev, "%s: AFE device param cmd svc_interval failed %d\n",__func__, ret);ret = -EINVAL;why do you override the return value?
goto exit;not necessary, this is a jump to the next line. Looks like copy-paste ...
Thanks, will fix.
- }
+exit:
- return ret;
+}
+/**
- q6afe_usb_port_prepare() - Prepare usb afe port.
- @port: Instance of afe port
- @cfg: USB configuration for the afe port
- */
+void q6afe_usb_port_prepare(struct q6afe_port *port,
struct q6afe_usb_cfg *cfg)+{
- union afe_port_config *pcfg = &port->port_cfg;
- pcfg->usb_cfg.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
- pcfg->usb_cfg.sample_rate = cfg->sample_rate;
- pcfg->usb_cfg.num_channels = cfg->num_channels;
- pcfg->usb_cfg.bit_width = cfg->bit_width;
- afe_port_send_usb_dev_param(port, cfg);
+} +EXPORT_SYMBOL_GPL(q6afe_usb_port_prepare);
- /**
- q6afe_hdmi_port_prepare() - Prepare hdmi afe port.
@@ -1611,7 +1791,10 @@ struct q6afe_port *q6afe_port_get_from_id(struct device *dev, int id) break; case AFE_PORT_ID_WSA_CODEC_DMA_RX_0 ... AFE_PORT_ID_RX_CODEC_DMA_RX_7: cfg_type = AFE_PARAM_ID_CODEC_DMA_CONFIG;
- break;
break;- case AFE_PORT_ID_USB_RX:
cfg_type = AFE_PARAM_ID_USB_AUDIO_CONFIG;break;diff --git a/sound/soc/qcom/qdsp6/q6afe.h b/sound/soc/qcom/qdsp6/q6afe.h index 30fd77e2f458..e098a3e15135 100644 --- a/sound/soc/qcom/qdsp6/q6afe.h +++ b/sound/soc/qcom/qdsp6/q6afe.h @@ -5,7 +5,7 @@
#include <dt-bindings/sound/qcom,q6afe.h>
-#define AFE_PORT_MAX 129 +#define AFE_PORT_MAX 130
#define MSM_AFE_PORT_TYPE_RX 0 #define MSM_AFE_PORT_TYPE_TX 1 @@ -205,6 +205,47 @@ struct q6afe_cdc_dma_cfg { u16 active_channels_mask; };
+/**
- struct q6afe_usb_cfg
- @cfg_minor_version: Minor version used for tracking USB audio device
- configuration.
- Supported values:
AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG
- @sample_rate: Sampling rate of the port
- Supported values:
AFE_PORT_SAMPLE_RATE_8K
AFE_PORT_SAMPLE_RATE_11025
AFE_PORT_SAMPLE_RATE_12K
AFE_PORT_SAMPLE_RATE_16K
AFE_PORT_SAMPLE_RATE_22050
AFE_PORT_SAMPLE_RATE_24K
AFE_PORT_SAMPLE_RATE_32K
AFE_PORT_SAMPLE_RATE_44P1K
AFE_PORT_SAMPLE_RATE_48K
AFE_PORT_SAMPLE_RATE_96K
AFE_PORT_SAMPLE_RATE_192K
- @bit_width: Bit width of the sample.
- Supported values: 16, 24
- @num_channels: Number of channels
- Supported values: 1, 2
- @data_format: Data format supported by the USB
- Supported values: 0
- @reserved: this field must be 0
- @dev_token: device token of actual end USB audio device
- @endian: endianness of this interface
- @service_interval: service interval
- **/
+struct q6afe_usb_cfg {
- u32 cfg_minor_version;
- u32 sample_rate;
- u16 bit_width;
- u16 num_channels;
- u16 data_format;
- u16 reserved;
- u32 dev_token;
- u32 endian;
- u32 service_interval;
+};
this definition looks exactly the same as struct afe_param_id_usb_cfg ??
I'll remove some of the params that we aren't utilizing.
Thanks Wesley Cheng