Re: [PATCH v4 08/32] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
  	default:
  		dev_err(dev, "Invalid port id 0x%x\n", port_id);
  		return ERR_PTR(-EINVAL);
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



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux