[PATCH 0/7] Add SDCA DAI ops helpers

[PATCH 0/7] Add SDCA DAI ops helpers

[Charles Keepax]

First, a couple of minor code fixups to already submitted code. Then
some patches to add new DAI ops helpers for the SDCA stuff, these allow
configuring things like the sample rate and finding out which SoundWire
port should be used for a specific SDCA streaming input/output terminal.
Still a few bits of outstanding work here (propogation of Cluster
information particularly) but his should be good enough to get some
basic use-cases working.

Hopefully we are getting fairly close to completing a first version of
the SDCA work now. Should be one more series to add FDL (firmware
downloading), then we should be able to send a first version of the
actual SDCA class driver itself.

Thanks,
Charles

Charles Keepax (7):
  ASoC: SDCA: Allow read-only controls to be deferrable
  ASoC: SDCA: Remove overly chatty input pin list warning
  ASoC: SDCA: Move SDCA search functions and export
  ASoC: soc-dai: Add private data to snd_soc_dai
  ASoC: SDCA: Add helper to add DAI constraints
  ASoC: SDCA: Add a helper to get the SoundWire port number
  ASoC: SDCA: Add hw_params() helper function

 include/sound/sdca_asoc.h       |  19 ++
 include/sound/sdca_function.h   |  31 +++
 include/sound/soc-dai.h         |   3 +
 sound/soc/sdca/sdca_asoc.c      | 427 +++++++++++++++++++++++++++-----
 sound/soc/sdca/sdca_functions.c |  72 +++++-
 5 files changed, 490 insertions(+), 62 deletions(-)

-- 
2.39.5

[PATCH 1/7] ASoC: SDCA: Allow read-only controls to be deferrable

[Charles Keepax]

The current SDCA Control parsing only checks the deferrable flag for
Read/Write and Dual Ranked controls. However, reads can defer as well as
writes so Read Only controls should also check for the deferrable flag.
Add the handling for this into find_sdca_entity_control().

Fixes: 42b144cb6a2d ("ASoC: SDCA: Add SDCA Control parsing")
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/sdca/sdca_functions.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index c34f3bf62983..164dc0e16a31 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -881,7 +881,8 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
 			control->value = tmp;
 			control->has_fixed = true;
 		}
-
+		fallthrough;
+	case SDCA_ACCESS_MODE_RO:
 		control->deferrable = fwnode_property_read_bool(control_node,
 								"mipi-sdca-control-deferrable");
 		break;
-- 
2.39.5

[PATCH 2/7] ASoC: SDCA: Remove overly chatty input pin list warning

[Charles Keepax]

An input pin list is not generally required, so a warning on the
absence of one is a little extreme, remove this warning message.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 sound/soc/sdca/sdca_functions.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 164dc0e16a31..1113f8d91b3a 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -1635,7 +1635,6 @@ static int find_sdca_entity_connection(struct device *dev,
 	ret = fwnode_property_read_u64(entity_node, "mipi-sdca-input-pin-list", &pin_list);
 	if (ret == -EINVAL) {
 		/* Allow missing pin lists, assume no pins. */
-		dev_warn(dev, "%s: missing pin list\n", entity->label);
 		return 0;
 	} else if (ret) {
 		dev_err(dev, "%s: failed to read pin list: %d\n", entity->label, ret);
-- 
2.39.5

[PATCH 3/7] ASoC: SDCA: Move SDCA search functions and export

[Charles Keepax]

The ASoC code for SDCA contains several helper functions that search for
controls/ranges/etc. As the code evolves these helpers are likely to be
useful to anything interacting with the stored DisCo data. Move the
helpers into sdca_function.c and export them so other modules can also
use them.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/sound/sdca_function.h   | 11 +++++
 sound/soc/sdca/sdca_asoc.c      | 73 ++++++---------------------------
 sound/soc/sdca/sdca_functions.c | 50 ++++++++++++++++++++++
 3 files changed, 74 insertions(+), 60 deletions(-)

diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index b4a97ff08729..543c09e99ab1 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -1316,4 +1316,15 @@ int sdca_parse_function(struct device *dev,
 			struct sdca_function_desc *desc,
 			struct sdca_function_data *function);
 
+struct sdca_control *sdca_selector_find_control(struct device *dev,
+						struct sdca_entity *entity,
+						const int sel);
+struct sdca_control_range *sdca_control_find_range(struct device *dev,
+						   struct sdca_entity *entity,
+						   struct sdca_control *control,
+						   int cols, int rows);
+struct sdca_control_range *sdca_selector_find_range(struct device *dev,
+						    struct sdca_entity *entity,
+						    int sel, int cols, int rows);
+
 #endif
diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
index dd7b19083c85..11c9b3b935fc 100644
--- a/sound/soc/sdca/sdca_asoc.c
+++ b/sound/soc/sdca/sdca_asoc.c
@@ -26,53 +26,6 @@
 #include <sound/soc-dapm.h>
 #include <sound/tlv.h>
 
-static struct sdca_control *selector_find_control(struct device *dev,
-						  struct sdca_entity *entity,
-						  const int sel)
-{
-	int i;
-
-	for (i = 0; i < entity->num_controls; i++) {
-		struct sdca_control *control = &entity->controls[i];
-
-		if (control->sel == sel)
-			return control;
-	}
-
-	dev_err(dev, "%s: control %#x: missing\n", entity->label, sel);
-	return NULL;
-}
-
-static struct sdca_control_range *control_find_range(struct device *dev,
-						     struct sdca_entity *entity,
-						     struct sdca_control *control,
-						     int cols, int rows)
-{
-	struct sdca_control_range *range = &control->range;
-
-	if ((cols && range->cols != cols) || (rows && range->rows != rows) ||
-	    !range->data) {
-		dev_err(dev, "%s: control %#x: ranges invalid (%d,%d)\n",
-			entity->label, control->sel, range->cols, range->rows);
-		return NULL;
-	}
-
-	return range;
-}
-
-static struct sdca_control_range *selector_find_range(struct device *dev,
-						      struct sdca_entity *entity,
-						      int sel, int cols, int rows)
-{
-	struct sdca_control *control;
-
-	control = selector_find_control(dev, entity, sel);
-	if (!control)
-		return NULL;
-
-	return control_find_range(dev, entity, control, cols, rows);
-}
-
 static bool exported_control(struct sdca_entity *entity, struct sdca_control *control)
 {
 	switch (SDCA_CTL_TYPE(entity->type, control->sel)) {
@@ -213,7 +166,7 @@ static int entity_early_parse_ge(struct device *dev,
 	const char **texts;
 	int i;
 
-	control = selector_find_control(dev, entity, SDCA_CTL_GE_SELECTED_MODE);
+	control = sdca_selector_find_control(dev, entity, SDCA_CTL_GE_SELECTED_MODE);
 	if (!control)
 		return -EINVAL;
 
@@ -221,7 +174,7 @@ static int entity_early_parse_ge(struct device *dev,
 		dev_warn(dev, "%s: unexpected access layer: %x\n",
 			 entity->label, control->layers);
 
-	range = control_find_range(dev, entity, control, SDCA_SELECTED_MODE_NCOLS, 0);
+	range = sdca_control_find_range(dev, entity, control, SDCA_SELECTED_MODE_NCOLS, 0);
 	if (!range)
 		return -EINVAL;
 
@@ -443,7 +396,7 @@ static int entity_parse_pde(struct device *dev,
 	unsigned int mask = 0;
 	int i;
 
-	control = selector_find_control(dev, entity, SDCA_CTL_PDE_REQUESTED_PS);
+	control = sdca_selector_find_control(dev, entity, SDCA_CTL_PDE_REQUESTED_PS);
 	if (!control)
 		return -EINVAL;
 
@@ -452,7 +405,7 @@ static int entity_parse_pde(struct device *dev,
 		dev_warn(dev, "%s: unexpected access layer: %x\n",
 			 entity->label, control->layers);
 
-	range = control_find_range(dev, entity, control, SDCA_REQUESTED_PS_NCOLS, 0);
+	range = sdca_control_find_range(dev, entity, control, SDCA_REQUESTED_PS_NCOLS, 0);
 	if (!range)
 		return -EINVAL;
 
@@ -499,8 +452,8 @@ static int entity_parse_su_device(struct device *dev,
 		return -EINVAL;
 	}
 
-	range = selector_find_range(dev, entity->group, SDCA_CTL_GE_SELECTED_MODE,
-				    SDCA_SELECTED_MODE_NCOLS, 0);
+	range = sdca_selector_find_range(dev, entity->group, SDCA_CTL_GE_SELECTED_MODE,
+					 SDCA_SELECTED_MODE_NCOLS, 0);
 	if (!range)
 		return -EINVAL;
 
@@ -613,7 +566,7 @@ static int entity_parse_su(struct device *dev,
 		return -EINVAL;
 	}
 
-	control = selector_find_control(dev, entity, SDCA_CTL_SU_SELECTOR);
+	control = sdca_selector_find_control(dev, entity, SDCA_CTL_SU_SELECTOR);
 	if (!control)
 		return -EINVAL;
 
@@ -643,7 +596,7 @@ static int entity_parse_mu(struct device *dev,
 		return -EINVAL;
 	}
 
-	control = selector_find_control(dev, entity, SDCA_CTL_MU_MIXER);
+	control = sdca_selector_find_control(dev, entity, SDCA_CTL_MU_MIXER);
 	if (!control)
 		return -EINVAL;
 
@@ -853,7 +806,7 @@ static int control_limit_kctl(struct device *dev,
 	/*
 	 * FIXME: For now only handle the simple case of a single linear range
 	 */
-	range = control_find_range(dev, entity, control, SDCA_VOLUME_LINEAR_NCOLS, 1);
+	range = sdca_control_find_range(dev, entity, control, SDCA_VOLUME_LINEAR_NCOLS, 1);
 	if (!range)
 		return -EINVAL;
 
@@ -1140,9 +1093,9 @@ static int populate_rate_format(struct device *dev,
 	}
 
 	if (entity->iot.clock) {
-		range = selector_find_range(dev, entity->iot.clock,
-					    SDCA_CTL_CS_SAMPLERATEINDEX,
-					    SDCA_SAMPLERATEINDEX_NCOLS, 0);
+		range = sdca_selector_find_range(dev, entity->iot.clock,
+						 SDCA_CTL_CS_SAMPLERATEINDEX,
+						 SDCA_SAMPLERATEINDEX_NCOLS, 0);
 		if (!range)
 			return -EINVAL;
 
@@ -1154,7 +1107,7 @@ static int populate_rate_format(struct device *dev,
 		clock_rates = UINT_MAX;
 	}
 
-	range = selector_find_range(dev, entity, sel, SDCA_USAGE_NCOLS, 0);
+	range = sdca_selector_find_range(dev, entity, sel, SDCA_USAGE_NCOLS, 0);
 	if (!range)
 		return -EINVAL;
 
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index 1113f8d91b3a..ca965e8a4c46 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -1941,6 +1941,56 @@ int sdca_parse_function(struct device *dev,
 }
 EXPORT_SYMBOL_NS(sdca_parse_function, "SND_SOC_SDCA");
 
+struct sdca_control *sdca_selector_find_control(struct device *dev,
+						struct sdca_entity *entity,
+						const int sel)
+{
+	int i;
+
+	for (i = 0; i < entity->num_controls; i++) {
+		struct sdca_control *control = &entity->controls[i];
+
+		if (control->sel == sel)
+			return control;
+	}
+
+	dev_err(dev, "%s: control %#x: missing\n", entity->label, sel);
+	return NULL;
+}
+EXPORT_SYMBOL_NS(sdca_selector_find_control, "SND_SOC_SDCA");
+
+struct sdca_control_range *sdca_control_find_range(struct device *dev,
+						   struct sdca_entity *entity,
+						   struct sdca_control *control,
+						   int cols, int rows)
+{
+	struct sdca_control_range *range = &control->range;
+
+	if ((cols && range->cols != cols) || (rows && range->rows != rows) ||
+	    !range->data) {
+		dev_err(dev, "%s: control %#x: ranges invalid (%d,%d)\n",
+			entity->label, control->sel, range->cols, range->rows);
+		return NULL;
+	}
+
+	return range;
+}
+EXPORT_SYMBOL_NS(sdca_control_find_range, "SND_SOC_SDCA");
+
+struct sdca_control_range *sdca_selector_find_range(struct device *dev,
+						    struct sdca_entity *entity,
+						    int sel, int cols, int rows)
+{
+	struct sdca_control *control;
+
+	control = sdca_selector_find_control(dev, entity, sel);
+	if (!control)
+		return NULL;
+
+	return sdca_control_find_range(dev, entity, control, cols, rows);
+}
+EXPORT_SYMBOL_NS(sdca_selector_find_range, "SND_SOC_SDCA");
+
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("SDCA library");
 MODULE_IMPORT_NS("SND_SOC_SDCA_HID");
-- 
2.39.5

[PATCH 4/7] ASoC: soc-dai: Add private data to snd_soc_dai

[Charles Keepax]

Add a private data pointer that can be used to store context along
with the DAI. This will be useful to allow the SDCA class library to
store data separately from the CODEC driver itself.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/sound/soc-dai.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index d19ab5572d2b..166c29557e9d 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -463,6 +463,9 @@ struct snd_soc_dai {
 
 	/* bit field */
 	unsigned int probed:1;
+
+	/* DAI private data */
+	void *priv;
 };
 
 static inline const struct snd_soc_pcm_stream *
-- 
2.39.5

[PATCH 5/7] ASoC: SDCA: Add helper to add DAI constraints

[Charles Keepax]

Currently the core SDCA code simply creates a place holder available
channels from 1 to SDCA_MAX_CHANNEL_COUNT. Add a helper function
that will constrain the number of channels based on the actual
available SDCA Clusters in DisCo. Currently this code only handles
Input Terminal Entities as they directly specify the Cluster. More
work will be required later for Output Terminals which inherit their
Cluster. Typically this new helper would be called from the DAIs
startup callback.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/sound/sdca_asoc.h       | 10 ++++
 include/sound/sdca_function.h   | 12 ++++
 sound/soc/sdca/sdca_asoc.c      | 99 +++++++++++++++++++++++++++++++++
 sound/soc/sdca/sdca_functions.c | 18 ++++++
 4 files changed, 139 insertions(+)

diff --git a/include/sound/sdca_asoc.h b/include/sound/sdca_asoc.h
index 9121531f0826..bbf146e4fcea 100644
--- a/include/sound/sdca_asoc.h
+++ b/include/sound/sdca_asoc.h
@@ -11,9 +11,12 @@
 #define __SDCA_ASOC_H__
 
 struct device;
+struct regmap;
 struct sdca_function_data;
 struct snd_kcontrol_new;
+struct snd_pcm_substream;
 struct snd_soc_component_driver;
+struct snd_soc_dai;
 struct snd_soc_dai_driver;
 struct snd_soc_dai_ops;
 struct snd_soc_dapm_route;
@@ -39,4 +42,11 @@ int sdca_asoc_populate_component(struct device *dev,
 				 struct snd_soc_dai_driver **dai_drv, int *num_dai_drv,
 				 const struct snd_soc_dai_ops *ops);
 
+int sdca_asoc_set_constraints(struct device *dev, struct regmap *regmap,
+			      struct sdca_function_data *function,
+			      struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai);
+void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai);
+
 #endif // __SDCA_ASOC_H__
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index 543c09e99ab1..3bde07409bf3 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -1268,6 +1268,15 @@ struct sdca_cluster {
 	struct sdca_channel *channels;
 };
 
+/**
+ * enum sdca_cluster_range - SDCA Range column definitions for ClusterIndex
+ */
+enum sdca_cluster_range {
+	SDCA_CLUSTER_BYTEINDEX				= 0,
+	SDCA_CLUSTER_CLUSTERID				= 1,
+	SDCA_CLUSTER_NCOLS				= 2,
+};
+
 /**
  * struct sdca_function_data - top-level information for one SDCA function
  * @desc: Pointer to short descriptor from initial parsing.
@@ -1326,5 +1335,8 @@ struct sdca_control_range *sdca_control_find_range(struct device *dev,
 struct sdca_control_range *sdca_selector_find_range(struct device *dev,
 						    struct sdca_entity *entity,
 						    int sel, int cols, int rows);
+struct sdca_cluster *sdca_id_find_cluster(struct device *dev,
+					  struct sdca_function_data *function,
+					  const int id);
 
 #endif
diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
index 11c9b3b935fc..1a0149287584 100644
--- a/sound/soc/sdca/sdca_asoc.c
+++ b/sound/soc/sdca/sdca_asoc.c
@@ -7,16 +7,20 @@
  * https://www.mipi.org/mipi-sdca-v1-0-download
  */
 
+#include <linux/bits.h>
 #include <linux/bitmap.h>
+#include <linux/build_bug.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
 #include <linux/device.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/overflow.h>
+#include <linux/regmap.h>
 #include <linux/soundwire/sdw_registers.h>
 #include <linux/string_helpers.h>
 #include <sound/control.h>
+#include <sound/pcm.h>
 #include <sound/sdca.h>
 #include <sound/sdca_asoc.h>
 #include <sound/sdca_function.h>
@@ -1269,3 +1273,98 @@ int sdca_asoc_populate_component(struct device *dev,
 	return 0;
 }
 EXPORT_SYMBOL_NS(sdca_asoc_populate_component, "SND_SOC_SDCA");
+
+/**
+ * sdca_asoc_set_constraints - constrain channels available on a DAI
+ * @dev: Pointer to the device, used for error messages.
+ * @regmap: Pointer to the Function register map.
+ * @function: Pointer to the Function information.
+ * @substream: Pointer to the PCM substream.
+ * @dai: Pointer to the ASoC DAI.
+ *
+ * Typically called from startup().
+ *
+ * Return: Returns zero on success, and a negative error code on failure.
+ */
+int sdca_asoc_set_constraints(struct device *dev, struct regmap *regmap,
+			      struct sdca_function_data *function,
+			      struct snd_pcm_substream *substream,
+			      struct snd_soc_dai *dai)
+{
+	static const unsigned int channel_list[] = {
+		 1,  2,  3,  4,  5,  6,  7,  8,  9, 10, 11, 12, 13, 14, 15, 16,
+		17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32,
+	};
+	struct sdca_entity *entity = &function->entities[dai->id];
+	struct snd_pcm_hw_constraint_list *constraint;
+	struct sdca_control_range *range;
+	struct sdca_control *control;
+	unsigned int channel_mask = 0;
+	int i, ret;
+
+	static_assert(ARRAY_SIZE(channel_list) == SDCA_MAX_CHANNEL_COUNT);
+	static_assert(sizeof(channel_mask) * BITS_PER_BYTE >= SDCA_MAX_CHANNEL_COUNT);
+
+	if (entity->type != SDCA_ENTITY_TYPE_IT)
+		return 0;
+
+	control = sdca_selector_find_control(dev, entity, SDCA_CTL_IT_CLUSTERINDEX);
+	if (!control)
+		return -EINVAL;
+
+	range = sdca_control_find_range(dev, entity, control, SDCA_CLUSTER_NCOLS, 0);
+	if (!range)
+		return -EINVAL;
+
+	for (i = 0; i < range->rows; i++) {
+		int clusterid = sdca_range(range, SDCA_CLUSTER_CLUSTERID, i);
+		struct sdca_cluster *cluster;
+
+		cluster = sdca_id_find_cluster(dev, function, clusterid);
+		if (!cluster)
+			return -ENODEV;
+
+		channel_mask |= (1 << (cluster->num_channels - 1));
+	}
+
+	dev_dbg(dev, "%s: set channel constraint mask: %#x\n",
+		entity->label, channel_mask);
+
+	constraint = kzalloc(sizeof(*constraint), GFP_KERNEL);
+	if (!constraint)
+		return -ENOMEM;
+
+	constraint->count = ARRAY_SIZE(channel_list);
+	constraint->list = channel_list;
+	constraint->mask = channel_mask;
+
+	ret = snd_pcm_hw_constraint_list(substream->runtime, 0,
+					 SNDRV_PCM_HW_PARAM_CHANNELS,
+					 constraint);
+	if (ret) {
+		dev_err(dev, "%s: failed to add constraint: %d\n", entity->label, ret);
+		kfree(constraint);
+		return ret;
+	}
+
+	dai->priv = constraint;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS(sdca_asoc_set_constraints, "SND_SOC_SDCA");
+
+/**
+ * sdca_asoc_free_constraints - free constraint allocations
+ * @substream: Pointer to the PCM substream.
+ * @dai: Pointer to the ASoC DAI.
+ *
+ * Typically called from shutdown().
+ */
+void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct snd_pcm_hw_constraint_list *constraint = dai->priv;
+
+	kfree(constraint);
+}
+EXPORT_SYMBOL_NS(sdca_asoc_free_constraints, "SND_SOC_SDCA");
diff --git a/sound/soc/sdca/sdca_functions.c b/sound/soc/sdca/sdca_functions.c
index ca965e8a4c46..0019c72b4904 100644
--- a/sound/soc/sdca/sdca_functions.c
+++ b/sound/soc/sdca/sdca_functions.c
@@ -1991,6 +1991,24 @@ struct sdca_control_range *sdca_selector_find_range(struct device *dev,
 }
 EXPORT_SYMBOL_NS(sdca_selector_find_range, "SND_SOC_SDCA");
 
+struct sdca_cluster *sdca_id_find_cluster(struct device *dev,
+					  struct sdca_function_data *function,
+					  const int id)
+{
+	int i;
+
+	for (i = 0; i < function->num_clusters; i++) {
+		struct sdca_cluster *cluster = &function->clusters[i];
+
+		if (cluster->id == id)
+			return cluster;
+	}
+
+	dev_err(dev, "%s: cluster %#x: missing\n", function->desc->name, id);
+	return NULL;
+}
+EXPORT_SYMBOL_NS(sdca_id_find_cluster, "SND_SOC_SDCA");
+
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_DESCRIPTION("SDCA library");
 MODULE_IMPORT_NS("SND_SOC_SDCA_HID");
-- 
2.39.5

[PATCH 6/7] ASoC: SDCA: Add a helper to get the SoundWire port number

[Charles Keepax]

Add a helper function to extract the SoundWire hardware port number
from the SDCA DataPort Selector Control. Typically this would be
called from hw_params() and used to call sdw_stream_add_slave().

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/sound/sdca_asoc.h     |  3 ++
 include/sound/sdca_function.h |  8 ++++
 sound/soc/sdca/sdca_asoc.c    | 75 +++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+)

diff --git a/include/sound/sdca_asoc.h b/include/sound/sdca_asoc.h
index bbf146e4fcea..800a26adcd8e 100644
--- a/include/sound/sdca_asoc.h
+++ b/include/sound/sdca_asoc.h
@@ -48,5 +48,8 @@ int sdca_asoc_set_constraints(struct device *dev, struct regmap *regmap,
 			      struct snd_soc_dai *dai);
 void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai);
+int sdca_asoc_get_port(struct device *dev, struct regmap *regmap,
+		       struct sdca_function_data *function,
+		       struct snd_soc_dai *dai);
 
 #endif // __SDCA_ASOC_H__
diff --git a/include/sound/sdca_function.h b/include/sound/sdca_function.h
index 3bde07409bf3..90d77fc46416 100644
--- a/include/sound/sdca_function.h
+++ b/include/sound/sdca_function.h
@@ -185,6 +185,14 @@ enum sdca_usage_range {
 	SDCA_USAGE_NCOLS				= 7,
 };
 
+/**
+ * enum sdca_dataport_selector_range - Column definitions for DataPort_Selector
+ */
+enum sdca_dataport_selector_range {
+	SDCA_DATAPORT_SELECTOR_NCOLS			= 16,
+	SDCA_DATAPORT_SELECTOR_NROWS			= 4,
+};
+
 /**
  * enum sdca_mu_controls - SDCA Controls for Mixer Unit
  *
diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
index 1a0149287584..03c663413cc9 100644
--- a/sound/soc/sdca/sdca_asoc.c
+++ b/sound/soc/sdca/sdca_asoc.c
@@ -19,6 +19,7 @@
 #include <linux/regmap.h>
 #include <linux/soundwire/sdw_registers.h>
 #include <linux/string_helpers.h>
+#include <linux/types.h>
 #include <sound/control.h>
 #include <sound/pcm.h>
 #include <sound/sdca.h>
@@ -1368,3 +1369,77 @@ void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
 	kfree(constraint);
 }
 EXPORT_SYMBOL_NS(sdca_asoc_free_constraints, "SND_SOC_SDCA");
+
+/**
+ * sdca_asoc_get_port - return SoundWire port for a DAI
+ * @dev: Pointer to the device, used for error messages.
+ * @regmap: Pointer to the Function register map.
+ * @function: Pointer to the Function information.
+ * @dai: Pointer to the ASoC DAI.
+ *
+ * Typically called from hw_params().
+ *
+ * Return: Returns a positive port number on success, and a negative error
+ * code on failure.
+ */
+int sdca_asoc_get_port(struct device *dev, struct regmap *regmap,
+		       struct sdca_function_data *function,
+		       struct snd_soc_dai *dai)
+{
+	struct sdca_entity *entity = &function->entities[dai->id];
+	struct sdca_control_range *range;
+	unsigned int reg, val;
+	int sel = -EINVAL;
+	int i, ret;
+
+	switch (entity->type) {
+	case SDCA_ENTITY_TYPE_IT:
+		sel = SDCA_CTL_IT_DATAPORT_SELECTOR;
+		break;
+	case SDCA_ENTITY_TYPE_OT:
+		sel = SDCA_CTL_OT_DATAPORT_SELECTOR;
+		break;
+	default:
+		break;
+	}
+
+	if (sel < 0 || !entity->iot.is_dataport) {
+		dev_err(dev, "%s: port number only available for dataports\n",
+			entity->label);
+		return -EINVAL;
+	}
+
+	range = sdca_selector_find_range(dev, entity, sel, SDCA_DATAPORT_SELECTOR_NCOLS,
+					 SDCA_DATAPORT_SELECTOR_NROWS);
+	if (!range)
+		return -EINVAL;
+
+	reg = SDW_SDCA_CTL(function->desc->adr, entity->id, sel, 0);
+
+	ret = regmap_read(regmap, reg, &val);
+	if (ret) {
+		dev_err(dev, "%s: failed to read dataport selector: %d\n",
+			entity->label, ret);
+		return ret;
+	}
+
+	for (i = 0; i < range->rows; i++) {
+		static const u8 port_mask = 0xF;
+
+		sel = sdca_range(range, val & port_mask, i);
+
+		/*
+		 * FIXME: Currently only a single dataport is supported, so
+		 * return the first one found, technically up to 4 dataports
+		 * could be linked, but this is not yet supported.
+		 */
+		if (sel != 0xFF)
+			return sel;
+
+		val >>= hweight8(port_mask);
+	}
+
+	dev_err(dev, "%s: no dataport found\n", entity->label);
+	return -ENODEV;
+}
+EXPORT_SYMBOL_NS(sdca_asoc_get_port, "SND_SOC_SDCA");
-- 
2.39.5

[PATCH 7/7] ASoC: SDCA: Add hw_params() helper function

[Charles Keepax]

Add a helper function that can be called from hw_params() in the DAI ops
to configure the SDCA Cluster, Clock and Usage controls. These setup the
channels, sample rate, and bit depths that will be used by the Terminal.

Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
---
 include/sound/sdca_asoc.h  |   6 ++
 sound/soc/sdca/sdca_asoc.c | 180 +++++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+)

diff --git a/include/sound/sdca_asoc.h b/include/sound/sdca_asoc.h
index 800a26adcd8e..aa9124f93218 100644
--- a/include/sound/sdca_asoc.h
+++ b/include/sound/sdca_asoc.h
@@ -14,6 +14,7 @@ struct device;
 struct regmap;
 struct sdca_function_data;
 struct snd_kcontrol_new;
+struct snd_pcm_hw_params;
 struct snd_pcm_substream;
 struct snd_soc_component_driver;
 struct snd_soc_dai;
@@ -51,5 +52,10 @@ void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
 int sdca_asoc_get_port(struct device *dev, struct regmap *regmap,
 		       struct sdca_function_data *function,
 		       struct snd_soc_dai *dai);
+int sdca_asoc_hw_params(struct device *dev, struct regmap *regmap,
+			struct sdca_function_data *function,
+			struct snd_pcm_substream *substream,
+			struct snd_pcm_hw_params *params,
+			struct snd_soc_dai *dai);
 
 #endif // __SDCA_ASOC_H__
diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
index 03c663413cc9..252d72377091 100644
--- a/sound/soc/sdca/sdca_asoc.c
+++ b/sound/soc/sdca/sdca_asoc.c
@@ -22,6 +22,7 @@
 #include <linux/types.h>
 #include <sound/control.h>
 #include <sound/pcm.h>
+#include <sound/pcm_params.h>
 #include <sound/sdca.h>
 #include <sound/sdca_asoc.h>
 #include <sound/sdca_function.h>
@@ -1443,3 +1444,182 @@ int sdca_asoc_get_port(struct device *dev, struct regmap *regmap,
 	return -ENODEV;
 }
 EXPORT_SYMBOL_NS(sdca_asoc_get_port, "SND_SOC_SDCA");
+
+static int set_cluster(struct device *dev, struct regmap *regmap,
+		       struct sdca_function_data *function,
+		       struct sdca_entity *entity, unsigned int channels)
+{
+	int sel = SDCA_CTL_IT_CLUSTERINDEX;
+	struct sdca_control_range *range;
+	int i, ret;
+
+	range = sdca_selector_find_range(dev, entity, sel, SDCA_CLUSTER_NCOLS, 0);
+	if (!range)
+		return -EINVAL;
+
+	for (i = 0; i < range->rows; i++) {
+		int cluster_id = sdca_range(range, SDCA_CLUSTER_CLUSTERID, i);
+		struct sdca_cluster *cluster;
+
+		cluster = sdca_id_find_cluster(dev, function, cluster_id);
+		if (!cluster)
+			return -ENODEV;
+
+		if (cluster->num_channels == channels) {
+			int index = sdca_range(range, SDCA_CLUSTER_BYTEINDEX, i);
+			unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
+							entity->id, sel, 0);
+
+			ret = regmap_update_bits(regmap, reg, 0xFF, index);
+			if (ret) {
+				dev_err(dev, "%s: failed to write cluster index: %d\n",
+					entity->label, ret);
+				return ret;
+			}
+
+			dev_dbg(dev, "%s: set cluster to %d (%d channels)\n",
+				entity->label, index, channels);
+
+			return 0;
+		}
+	}
+
+	dev_err(dev, "%s: no cluster for %d channels\n", entity->label, channels);
+	return -EINVAL;
+}
+
+static int set_clock(struct device *dev, struct regmap *regmap,
+		     struct sdca_function_data *function,
+		     struct sdca_entity *entity, int target_rate)
+{
+	int sel = SDCA_CTL_CS_SAMPLERATEINDEX;
+	struct sdca_control_range *range;
+	int i, ret;
+
+	range = sdca_selector_find_range(dev, entity, sel, SDCA_SAMPLERATEINDEX_NCOLS, 0);
+	if (!range)
+		return -EINVAL;
+
+	for (i = 0; i < range->rows; i++) {
+		unsigned int rate = sdca_range(range, SDCA_SAMPLERATEINDEX_RATE, i);
+
+		if (rate == target_rate) {
+			unsigned int index = sdca_range(range,
+							SDCA_SAMPLERATEINDEX_INDEX,
+							i);
+			unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
+							entity->id, sel, 0);
+
+			ret = regmap_update_bits(regmap, reg, 0xFF, index);
+			if (ret) {
+				dev_err(dev, "%s: failed to write clock rate: %d\n",
+					entity->label, ret);
+				return ret;
+			}
+
+			dev_dbg(dev, "%s: set clock rate to %d (%dHz)\n",
+				entity->label, index, rate);
+
+			return 0;
+		}
+	}
+
+	dev_err(dev, "%s: no clock rate for %dHz\n", entity->label, target_rate);
+	return -EINVAL;
+}
+
+static int set_usage(struct device *dev, struct regmap *regmap,
+		     struct sdca_function_data *function,
+		     struct sdca_entity *entity, int sel,
+		     int target_rate, int target_width)
+{
+	struct sdca_control_range *range;
+	int i, ret;
+
+	range = sdca_selector_find_range(dev, entity, sel, SDCA_USAGE_NCOLS, 0);
+	if (!range)
+		return -EINVAL;
+
+	for (i = 0; i < range->rows; i++) {
+		unsigned int rate = sdca_range(range, SDCA_USAGE_SAMPLE_RATE, i);
+		unsigned int width = sdca_range(range, SDCA_USAGE_SAMPLE_WIDTH, i);
+
+		if ((!rate || rate == target_rate) && width == target_width) {
+			unsigned int usage = sdca_range(range, SDCA_USAGE_NUMBER, i);
+			unsigned int reg = SDW_SDCA_CTL(function->desc->adr,
+							entity->id, sel, 0);
+
+			ret = regmap_update_bits(regmap, reg, 0xFF, usage);
+			if (ret) {
+				dev_err(dev, "%s: failed to write usage: %d\n",
+					entity->label, ret);
+				return ret;
+			}
+
+			dev_dbg(dev, "%s: set usage to %#x (%dHz, %d bits)\n",
+				entity->label, usage, target_rate, target_width);
+
+			return 0;
+		}
+	}
+
+	dev_err(dev, "%s: no usage for %dHz, %dbits\n",
+		entity->label, target_rate, target_width);
+	return -EINVAL;
+}
+
+/**
+ * sdca_asoc_hw_params - set SDCA channels, sample rate and bit depth
+ * @dev: Pointer to the device, used for error messages.
+ * @regmap: Pointer to the Function register map.
+ * @function: Pointer to the Function information.
+ * @substream: Pointer to the PCM substream.
+ * @params: Pointer to the hardware parameters.
+ * @dai: Pointer to the ASoC DAI.
+ *
+ * Typically called from hw_params().
+ *
+ * Return: Returns zero on success, and a negative error code on failure.
+ */
+int sdca_asoc_hw_params(struct device *dev, struct regmap *regmap,
+			struct sdca_function_data *function,
+			struct snd_pcm_substream *substream,
+			struct snd_pcm_hw_params *params,
+			struct snd_soc_dai *dai)
+{
+	struct sdca_entity *entity = &function->entities[dai->id];
+	int channels = params_channels(params);
+	int width = params_width(params);
+	int rate = params_rate(params);
+	int usage_sel;
+	int ret;
+
+	switch (entity->type) {
+	case SDCA_ENTITY_TYPE_IT:
+		ret = set_cluster(dev, regmap, function, entity, channels);
+		if (ret)
+			return ret;
+
+		usage_sel = SDCA_CTL_IT_USAGE;
+		break;
+	case SDCA_ENTITY_TYPE_OT:
+		usage_sel = SDCA_CTL_OT_USAGE;
+		break;
+	default:
+		dev_err(dev, "%s: hw_params on non-terminal entity\n", entity->label);
+		return -EINVAL;
+	}
+
+	if (entity->iot.clock) {
+		ret = set_clock(dev, regmap, function, entity->iot.clock, rate);
+		if (ret)
+			return ret;
+	}
+
+	ret = set_usage(dev, regmap, function, entity, usage_sel, rate, width);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_NS(sdca_asoc_hw_params, "SND_SOC_SDCA");
-- 
2.39.5

Re: [PATCH 0/7] Add SDCA DAI ops helpers

[Pierre-Louis Bossart]

On 7/7/25 14:41, Charles Keepax wrote:
> First, a couple of minor code fixups to already submitted code. Then
> some patches to add new DAI ops helpers for the SDCA stuff, these allow
> configuring things like the sample rate and finding out which SoundWire
> port should be used for a specific SDCA streaming input/output terminal.
> Still a few bits of outstanding work here (propogation of Cluster
> information particularly) but his should be good enough to get some
> basic use-cases working.

For the patchset:

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>

> Hopefully we are getting fairly close to completing a first version of
> the SDCA work now. Should be one more series to add FDL (firmware
> downloading), then we should be able to send a first version of the
> actual SDCA class driver itself.

FDL is a very desirable feature, but shouldn't there be an UMP (Universal Message Passing) layer first? HID and FDL rely on UMP.
I would have expected the ownership changes to be handled in a shared UMP library or set of helpers, no?

Re: [PATCH 0/7] Add SDCA DAI ops helpers

[Charles Keepax]

On Tue, Jul 15, 2025 at 07:58:32PM +0200, Pierre-Louis Bossart wrote:
> On 7/7/25 14:41, Charles Keepax wrote:
> > First, a couple of minor code fixups to already submitted code. Then
> > some patches to add new DAI ops helpers for the SDCA stuff, these allow
> > configuring things like the sample rate and finding out which SoundWire
> > port should be used for a specific SDCA streaming input/output terminal.
> > Still a few bits of outstanding work here (propogation of Cluster
> > information particularly) but his should be good enough to get some
> > basic use-cases working.
> 
> For the patchset:
> 
> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
> 
> > Hopefully we are getting fairly close to completing a first version of
> > the SDCA work now. Should be one more series to add FDL (firmware
> > downloading), then we should be able to send a first version of the
> > actual SDCA class driver itself.
> 
> FDL is a very desirable feature, but shouldn't there be an UMP
> (Universal Message Passing) layer first? HID and FDL rely on UMP.
> I would have expected the ownership changes to be handled in
> a shared UMP library or set of helpers, no?

Yeah we are still working through the last iterations internally
on the FDL stuff. I think it probably does make sense to pull
out some UMP helpers (indeed I was have a look at it yesterday)
although they should be pretty simple so I imagine we will just
send them as part of the FDL set (subject to change as we work
through it).

Thanks,
Charles

Re: [PATCH 0/7] Add SDCA DAI ops helpers

[Mark Brown]

On Mon, 07 Jul 2025 13:41:48 +0100, Charles Keepax wrote:
> First, a couple of minor code fixups to already submitted code. Then
> some patches to add new DAI ops helpers for the SDCA stuff, these allow
> configuring things like the sample rate and finding out which SoundWire
> port should be used for a specific SDCA streaming input/output terminal.
> Still a few bits of outstanding work here (propogation of Cluster
> information particularly) but his should be good enough to get some
> basic use-cases working.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/7] ASoC: SDCA: Allow read-only controls to be deferrable
      commit: 4eb6ad5d2080681b531db2c1764246f9a868062f
[2/7] ASoC: SDCA: Remove overly chatty input pin list warning
      commit: cbcb5f5c2be523ef0908df290b3033138bd4c185
[3/7] ASoC: SDCA: Move SDCA search functions and export
      commit: c57ad862462f064c0bd943a5828f5e0eca469ca5
[4/7] ASoC: soc-dai: Add private data to snd_soc_dai
      commit: 5f86d41d0410b072b5f4875ef5d38bf8d18eed55
[5/7] ASoC: SDCA: Add helper to add DAI constraints
      commit: 7b0d60dbb468fa82e9053292cdc8a5436400bfaf
[6/7] ASoC: SDCA: Add a helper to get the SoundWire port number
      commit: 264d3d776fb1a428706b0ca0f679bbed876fe7c9
[7/7] ASoC: SDCA: Add hw_params() helper function
      commit: 4ed357f72a0e0a691304e5f14a3323811c8ce862

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH 0/7] Add SDCA DAI ops helpers

[Pierre-Louis Bossart]

On 7/16/25 10:28, Charles Keepax wrote:
> On Tue, Jul 15, 2025 at 07:58:32PM +0200, Pierre-Louis Bossart wrote:
>> On 7/7/25 14:41, Charles Keepax wrote:
>>> First, a couple of minor code fixups to already submitted code. Then
>>> some patches to add new DAI ops helpers for the SDCA stuff, these allow
>>> configuring things like the sample rate and finding out which SoundWire
>>> port should be used for a specific SDCA streaming input/output terminal.
>>> Still a few bits of outstanding work here (propogation of Cluster
>>> information particularly) but his should be good enough to get some
>>> basic use-cases working.
>>
>> For the patchset:
>>
>> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>
>>
>>> Hopefully we are getting fairly close to completing a first version of
>>> the SDCA work now. Should be one more series to add FDL (firmware
>>> downloading), then we should be able to send a first version of the
>>> actual SDCA class driver itself.
>>
>> FDL is a very desirable feature, but shouldn't there be an UMP
>> (Universal Message Passing) layer first? HID and FDL rely on UMP.
>> I would have expected the ownership changes to be handled in
>> a shared UMP library or set of helpers, no?
> 
> Yeah we are still working through the last iterations internally
> on the FDL stuff. I think it probably does make sense to pull
> out some UMP helpers (indeed I was have a look at it yesterday)
> although they should be pretty simple so I imagine we will just
> send them as part of the FDL set (subject to change as we work
> through it).

Indeed UMP is pretty simple on paper, but as usual there will be corner cases of the ownership change never happening. I vaguely remember there was a sequence required for the HID used by Realtek.
FDL is another order of magnitude in terms of error handling, the state machine is rather complicated and it'll fun to see how implementations map to the spec :-)