Re: [PATCH v5 14/21] ASoC: qdsp6: audioreach: add basic pkt alloc support

Thanks Pierre for taking time to review the patches.

On 03/09/2021 15:23, Pierre-Louis Bossart wrote:

quoted

+struct apm_sub_graph_params  {
+	struct apm_module_param_data param_data;
+	uint32_t num_sub_graphs;
+	struct apm_sub_graph_data sg_cfg[0];

I thought the use of zero-length arrays [0] was deprecated in favor of
flexible arrays []?

Thanks for the hints, I will fix such instances, typos and along with missing new lines before sending out next version.

quoted

+} __packed;
+

...

quoted

+struct audioreach_module {
+	uint32_t module_id;
+	uint32_t instance_id;
+
+	uint32_t max_ip_port;
+	uint32_t max_op_port;
+
+	uint32_t in_port;
+	uint32_t out_port;
+
+	/* Connections */
+	uint32_t src_mod_inst_id;
+	uint32_t src_mod_op_port_id;
+	uint32_t dst_mod_inst_id;
+	uint32_t dst_mod_ip_port_id;
+
+	/* Format specifics */
+	uint32_t ch_fmt;
+	uint32_t rate;
+	uint32_t bit_depth;
+
+	/* I2S module */
+	uint32_t hw_interface_idx;
+	uint32_t sd_line_idx;
+	uint32_t ws_src;
+	uint32_t frame_size_factor;
+	uint32_t data_format;
+	uint32_t hw_interface_type;
+
+	/* PCM module specific */
+	uint32_t interleave_type;
+
+	/* GAIN/Vol Control Module */
+	uint16_t gain;

newline for consistency?

quoted

+	/* Logging */
+	uint32_t log_code;
+	uint32_t log_tap_point_id;
+	uint32_t mode;

probably not related to logging, comment missing?

This is actually a log mode to to indicate whether to log immediately (1) or wait until log buffer is completely filled (0)

I will rename this to log_mode in next version and also add an extra new line to split some member used for book keeping.

quoted

+	struct list_head node;
+	struct audioreach_container *container;
+	struct snd_soc_dapm_widget *widget;
+};
+

--srini