More minor SDCA changes

Merge series from Charles Keepax <[email protected]>:

A small chain with some patches that seem to have got lost in the
process. One small additional bug fix for the regmap callbacks, and one
small feature addition for the control parsing.

Charles Keepax (2):
  ASoC: SDCA: Fix some holes in the regmap readable/writeable helpers
  ASoC: SDCA: Add support for -cn- value properties

 include/sound/sdca_function.h   | 14 ++---
 sound/soc/sdca/sdca_functions.c | 99 +++++++++++++++++++++------------
 sound/soc/sdca/sdca_regmap.c    | 29 ++++++++--
 3 files changed, 92 insertions(+), 50 deletions(-)

--
2.39.5

Author: broonie

include/sound/sdca_function.h

@@ -742,14 +742,14 @@ struct sdca_control_range {
  * struct sdca_control - information for one SDCA Control
  * @label: Name for the Control, from SDCA Specification v1.0, section 7.1.7.
  * @sel: Identifier used for addressing.
- * @value: Holds the Control value for constants and defaults.
  * @nbits: Number of bits used in the Control.
- * @interrupt_position: SCDA interrupt line that will alert to changes on this
- * Control.
+ * @values: Holds the Control value for constants and defaults.
  * @cn_list: A bitmask showing the valid Control Numbers within this Control,
  * Control Numbers typically represent channels.
- * @range: Buffer describing valid range of values for the Control.
+ * @interrupt_position: SCDA interrupt line that will alert to changes on this
+ * Control.
  * @type: Format of the data in the Control.
+ * @range: Buffer describing valid range of values for the Control.
  * @mode: Access mode of the Control.
  * @layers: Bitmask of access layers of the Control.
  * @deferrable: Indicates if the access to the Control can be deferred.
@@ -760,13 +760,13 @@ struct sdca_control {
 	const char *label;
 	int sel;
 
-	int value;
 	int nbits;
-	int interrupt_position;
+	int *values;
 	u64 cn_list;
+	int interrupt_position;
 
-	struct sdca_control_range range;
 	enum sdca_control_datatype type;
+	struct sdca_control_range range;
 	enum sdca_access_mode mode;
 	u8 layers;
 

sound/soc/sdca/sdca_functions.c

@@ -814,6 +814,43 @@ static int find_sdca_control_range(struct device *dev,
 	return 0;
 }
 
+static int find_sdca_control_value(struct device *dev, struct sdca_entity *entity,
+				   struct fwnode_handle *control_node,
+				   struct sdca_control *control,
+				   const char * const label)
+{
+	char property[SDCA_PROPERTY_LENGTH];
+	bool global = true;
+	int ret, cn, i;
+	u32 tmp;
+
+	snprintf(property, sizeof(property), "mipi-sdca-control-%s", label);
+
+	ret = fwnode_property_read_u32(control_node, property, &tmp);
+	if (ret == -EINVAL)
+		global = false;
+	else if (ret)
+		return ret;
+
+	i = 0;
+	for_each_set_bit(cn, (unsigned long *)&control->cn_list,
+			 BITS_PER_TYPE(control->cn_list)) {
+		if (!global) {
+			snprintf(property, sizeof(property),
+				 "mipi-sdca-control-cn-%d-%s", cn, label);
+
+			ret = fwnode_property_read_u32(control_node, property, &tmp);
+			if (ret)
+				return ret;
+		}
+
+		control->values[i] = tmp;
+		i++;
+	}
+
+	return 0;
+}
+
 /*
  * TODO: Add support for -cn- properties, allowing different channels to have
  * different defaults etc.
@@ -843,44 +880,44 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
 
 	control->layers = tmp;
 
+	ret = fwnode_property_read_u64(control_node, "mipi-sdca-control-cn-list",
+				       &control->cn_list);
+	if (ret == -EINVAL) {
+		/* Spec allows not specifying cn-list if only the first number is used */
+		control->cn_list = 0x1;
+	} else if (ret || !control->cn_list) {
+		dev_err(dev, "%s: control %#x: cn list missing: %d\n",
+			entity->label, control->sel, ret);
+		return ret;
+	}
+
+	control->values = devm_kzalloc(dev, hweight64(control->cn_list), GFP_KERNEL);
+	if (!control->values)
+		return -ENOMEM;
+
 	switch (control->mode) {
 	case SDCA_ACCESS_MODE_DC:
-		ret = fwnode_property_read_u32(control_node,
-					       "mipi-sdca-control-dc-value",
-					       &tmp);
+		ret = find_sdca_control_value(dev, entity, control_node, control,
+					      "dc-value");
 		if (ret) {
 			dev_err(dev, "%s: control %#x: dc value missing: %d\n",
 				entity->label, control->sel, ret);
 			return ret;
 		}
 
-		control->value = tmp;
 		control->has_fixed = true;
 		break;
 	case SDCA_ACCESS_MODE_RW:
 	case SDCA_ACCESS_MODE_DUAL:
-		ret = fwnode_property_read_u32(control_node,
-					       "mipi-sdca-control-default-value",
-					       &tmp);
-		if (!ret) {
-			control->value = tmp;
+		ret = find_sdca_control_value(dev, entity, control_node, control,
+					      "default-value");
+		if (!ret)
 			control->has_default = true;
-		}
-
-		ret = fwnode_property_read_u32(control_node,
-					       "mipi-sdca-control-fixed-value",
-					       &tmp);
-		if (!ret) {
-			if (control->has_default && control->value != tmp) {
-				dev_err(dev,
-					"%s: control %#x: default and fixed value don't match\n",
-					entity->label, control->sel);
-				return -EINVAL;
-			}
 
-			control->value = tmp;
+		ret = find_sdca_control_value(dev, entity, control_node, control,
+					      "fixed-value");
+		if (!ret)
 			control->has_fixed = true;
-		}
 		fallthrough;
 	case SDCA_ACCESS_MODE_RO:
 		control->deferrable = fwnode_property_read_bool(control_node,
@@ -897,17 +934,6 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
 		return ret;
 	}
 
-	ret = fwnode_property_read_u64(control_node, "mipi-sdca-control-cn-list",
-				       &control->cn_list);
-	if (ret == -EINVAL) {
-		/* Spec allows not specifying cn-list if only the first number is used */
-		control->cn_list = 0x1;
-	} else if (ret || !control->cn_list) {
-		dev_err(dev, "%s: control %#x: cn list missing: %d\n",
-			entity->label, control->sel, ret);
-		return ret;
-	}
-
 	ret = fwnode_property_read_u32(control_node,
 				       "mipi-sdca-control-interrupt-position",
 				       &tmp);
@@ -923,11 +949,10 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
 	control->type = find_sdca_control_datatype(entity, control);
 	control->nbits = find_sdca_control_bits(entity, control);
 
-	dev_info(dev, "%s: %s: control %#x mode %#x layers %#x cn %#llx int %d value %#x %s\n",
+	dev_info(dev, "%s: %s: control %#x mode %#x layers %#x cn %#llx int %d %s\n",
 		 entity->label, control->label, control->sel,
 		 control->mode, control->layers, control->cn_list,
-		 control->interrupt_position, control->value,
-		 control->deferrable ? "deferrable" : "");
+		 control->interrupt_position, control->deferrable ? "deferrable" : "");
 
 	return 0;
 }

sound/soc/sdca/sdca_regmap.c

@@ -72,12 +72,18 @@ bool sdca_regmap_readable(struct sdca_function_data *function, unsigned int reg)
 	if (!control)
 		return false;
 
+	if (!(BIT(SDW_SDCA_CTL_CNUM(reg)) & control->cn_list))
+		return false;
+
 	switch (control->mode) {
 	case SDCA_ACCESS_MODE_RW:
 	case SDCA_ACCESS_MODE_RO:
-	case SDCA_ACCESS_MODE_DUAL:
 	case SDCA_ACCESS_MODE_RW1S:
 	case SDCA_ACCESS_MODE_RW1C:
+		if (SDW_SDCA_NEXT_CTL(0) & reg)
+			return false;
+		fallthrough;
+	case SDCA_ACCESS_MODE_DUAL:
 		/* No access to registers marked solely for device use */
 		return control->layers & ~SDCA_ACCESS_LAYER_DEVICE;
 	default:
@@ -104,11 +110,17 @@ bool sdca_regmap_writeable(struct sdca_function_data *function, unsigned int reg
 	if (!control)
 		return false;
 
+	if (!(BIT(SDW_SDCA_CTL_CNUM(reg)) & control->cn_list))
+		return false;
+
 	switch (control->mode) {
 	case SDCA_ACCESS_MODE_RW:
-	case SDCA_ACCESS_MODE_DUAL:
 	case SDCA_ACCESS_MODE_RW1S:
 	case SDCA_ACCESS_MODE_RW1C:
+		if (SDW_SDCA_NEXT_CTL(0) & reg)
+			return false;
+		fallthrough;
+	case SDCA_ACCESS_MODE_DUAL:
 		/* No access to registers marked solely for device use */
 		return control->layers & ~SDCA_ACCESS_LAYER_DEVICE;
 	default:
@@ -241,7 +253,7 @@ int sdca_regmap_populate_constants(struct device *dev,
 				   struct sdca_function_data *function,
 				   struct reg_default *consts)
 {
-	int i, j, k;
+	int i, j, k, l;
 
 	for (i = 0, k = 0; i < function->num_entities; i++) {
 		struct sdca_entity *entity = &function->entities[i];
@@ -253,13 +265,15 @@ int sdca_regmap_populate_constants(struct device *dev,
 			if (control->mode != SDCA_ACCESS_MODE_DC)
 				continue;
 
+			l = 0;
 			for_each_set_bit(cn, (unsigned long *)&control->cn_list,
 					 BITS_PER_TYPE(control->cn_list)) {
 				consts[k].reg = SDW_SDCA_CTL(function->desc->adr,
 							     entity->id,
 							     control->sel, cn);
-				consts[k].def = control->value;
+				consts[k].def = control->values[l];
 				k++;
+				l++;
 			}
 		}
 	}
@@ -283,7 +297,7 @@ EXPORT_SYMBOL_NS(sdca_regmap_populate_constants, "SND_SOC_SDCA");
 int sdca_regmap_write_defaults(struct device *dev, struct regmap *regmap,
 			       struct sdca_function_data *function)
 {
-	int i, j;
+	int i, j, k;
 	int ret;
 
 	for (i = 0; i < function->num_entities; i++) {
@@ -299,16 +313,19 @@ int sdca_regmap_write_defaults(struct device *dev, struct regmap *regmap,
 			if (!control->has_default && !control->has_fixed)
 				continue;
 
+			k = 0;
 			for_each_set_bit(cn, (unsigned long *)&control->cn_list,
 					 BITS_PER_TYPE(control->cn_list)) {
 				unsigned int reg;
 
 				reg = SDW_SDCA_CTL(function->desc->adr, entity->id,
 						   control->sel, cn);
 
-				ret = regmap_write(regmap, reg, control->value);
+				ret = regmap_write(regmap, reg, control->values[k]);
 				if (ret)
 					return ret;
+
+				k++;
 			}
 		}
 	}