Minor SDCA Fixes

Merge series from Charles Keepax <ckeepax@opensource.cirrus.com>:

A bit of a mixed bag of minor misc fixes, improve handling
of volatile SDCA Controls, make some minor bug fixes to jack
detect, improve the cache syncing by adding some more defaults,
and improve some FDL error messages.

Author: broonie

include/sound/sdca_function.h

@@ -798,6 +798,7 @@ struct sdca_control_range {
  * @sel: Identifier used for addressing.
  * @nbits: Number of bits used in the Control.
  * @values: Holds the Control value for constants and defaults.
+ * @reset: Defined reset value for the Control.
  * @cn_list: A bitmask showing the valid Control Numbers within this Control,
  * Control Numbers typically represent channels.
  * @interrupt_position: SCDA interrupt line that will alert to changes on this
@@ -808,6 +809,7 @@ struct sdca_control_range {
  * @layers: Bitmask of access layers of the Control.
  * @deferrable: Indicates if the access to the Control can be deferred.
  * @has_default: Indicates the Control has a default value to be written.
+ * @has_reset: Indicates the Control has a defined reset value.
  * @has_fixed: Indicates the Control only supports a single value.
  */
 struct sdca_control {
@@ -816,6 +818,7 @@ struct sdca_control {
 
 	int nbits;
 	int *values;
+	int reset;
 	u64 cn_list;
 	int interrupt_position;
 
@@ -827,6 +830,7 @@ struct sdca_control {
 	bool deferrable;
 	bool is_volatile;
 	bool has_default;
+	bool has_reset;
 	bool has_fixed;
 };
 

sound/soc/sdca/sdca_asoc.c

@@ -16,6 +16,7 @@
 #include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/overflow.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/soundwire/sdw_registers.h>
 #include <linux/string_helpers.h>
@@ -115,6 +116,41 @@ int sdca_asoc_count_component(struct device *dev, struct sdca_function_data *fun
 }
 EXPORT_SYMBOL_NS(sdca_asoc_count_component, "SND_SOC_SDCA");
 
+static int ge_put_enum_double(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_dapm_kcontrol_to_dapm(kcontrol);
+	struct snd_soc_component *component = snd_soc_dapm_to_component(dapm);
+	struct device *dev = component->dev;
+	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
+	unsigned int *item = ucontrol->value.enumerated.item;
+	unsigned int reg = e->reg;
+	int ret;
+
+	reg &= ~SDW_SDCA_CTL_CSEL(0x3F);
+	reg |= SDW_SDCA_CTL_CSEL(SDCA_CTL_GE_DETECTED_MODE);
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to resume writing %s: %d\n",
+			kcontrol->id.name, ret);
+		return ret;
+	}
+
+	ret = snd_soc_component_read(component, reg);
+	pm_runtime_put(dev);
+	if (ret < 0)
+		return ret;
+	else if (ret <= SDCA_DETECTED_MODE_DETECTION_IN_PROGRESS)
+		return -EBUSY;
+
+	ret = snd_soc_enum_item_to_val(e, item[0]);
+	if (ret <= SDCA_DETECTED_MODE_DETECTION_IN_PROGRESS)
+		return -EINVAL;
+
+	return snd_soc_dapm_put_enum_double(kcontrol, ucontrol);
+}
+
 static int entity_early_parse_ge(struct device *dev,
 				 struct sdca_function_data *function,
 				 struct sdca_entity *entity)
@@ -191,7 +227,7 @@ static int entity_early_parse_ge(struct device *dev,
 	kctl->name = control_name;
 	kctl->info = snd_soc_info_enum_double;
 	kctl->get = snd_soc_dapm_get_enum_double;
-	kctl->put = snd_soc_dapm_put_enum_double;
+	kctl->put = ge_put_enum_double;
 	kctl->private_value = (unsigned long)soc_enum;
 
 	entity->ge.kctl = kctl;
@@ -792,6 +828,48 @@ static int control_limit_kctl(struct device *dev,
 	return 0;
 }
 
+static int volatile_get_volsw(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct device *dev = component->dev;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to resume reading %s: %d\n",
+			kcontrol->id.name, ret);
+		return ret;
+	}
+
+	ret = snd_soc_get_volsw(kcontrol, ucontrol);
+
+	pm_runtime_put(dev);
+
+	return ret;
+}
+
+static int volatile_put_volsw(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct device *dev = component->dev;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret < 0) {
+		dev_err(dev, "failed to resume writing %s: %d\n",
+			kcontrol->id.name, ret);
+		return ret;
+	}
+
+	ret = snd_soc_put_volsw(kcontrol, ucontrol);
+
+	pm_runtime_put(dev);
+
+	return ret;
+}
+
 static int populate_control(struct device *dev,
 			    struct sdca_function_data *function,
 			    struct sdca_entity *entity,
@@ -849,8 +927,13 @@ static int populate_control(struct device *dev,
 	(*kctl)->private_value = (unsigned long)mc;
 	(*kctl)->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
 	(*kctl)->info = snd_soc_info_volsw;
-	(*kctl)->get = snd_soc_get_volsw;
-	(*kctl)->put = snd_soc_put_volsw;
+	if (control->is_volatile) {
+		(*kctl)->get = volatile_get_volsw;
+		(*kctl)->put = volatile_put_volsw;
+	} else {
+		(*kctl)->get = snd_soc_get_volsw;
+		(*kctl)->put = snd_soc_put_volsw;
+	}
 
 	if (readonly_control(control))
 		(*kctl)->access = SNDRV_CTL_ELEM_ACCESS_READ;

sound/soc/sdca/sdca_fdl.c

@@ -256,8 +256,7 @@ static int fdl_load_file(struct sdca_interrupt *interrupt,
 		    tmp->file_length != firmware->size) {
 			dev_err(dev, "bad disk SWF size\n");
 		} else if (!swf || swf->file_version <= tmp->file_version) {
-			dev_dbg(dev, "using SWF from disk: %x-%x-%x\n",
-				tmp->vendor_id, tmp->file_id, tmp->file_version);
+			dev_dbg(dev, "using SWF from disk\n");
 			swf = tmp;
 		}
 	}
@@ -267,6 +266,9 @@ static int fdl_load_file(struct sdca_interrupt *interrupt,
 		return -ENOENT;
 	}
 
+	dev_info(dev, "loading SWF: %x-%x-%x\n",
+		 swf->vendor_id, swf->file_id, swf->file_version);
+
 	ret = sdca_ump_write_message(dev, interrupt->device_regmap,
 				     interrupt->function_regmap,
 				     interrupt->function, interrupt->entity,

sound/soc/sdca/sdca_functions.c

@@ -911,10 +911,38 @@ static int find_sdca_control_value(struct device *dev, struct sdca_entity *entit
 	return 0;
 }
 
-/*
- * TODO: Add support for -cn- properties, allowing different channels to have
- * different defaults etc.
- */
+static int find_sdca_control_reset(const struct sdca_entity *entity,
+				   struct sdca_control *control)
+{
+	switch (SDCA_CTL_TYPE(entity->type, control->sel)) {
+	case SDCA_CTL_TYPE_S(FU, AGC):
+	case SDCA_CTL_TYPE_S(FU, BASS_BOOST):
+	case SDCA_CTL_TYPE_S(FU, LOUDNESS):
+	case SDCA_CTL_TYPE_S(SMPU, TRIGGER_ENABLE):
+	case SDCA_CTL_TYPE_S(GE, SELECTED_MODE):
+	case SDCA_CTL_TYPE_S(TG, TONE_DIVIDER):
+	case SDCA_CTL_TYPE_S(ENTITY_0, COMMIT_GROUP_MASK):
+		control->has_reset = true;
+		control->reset = 0;
+		break;
+	case SDCA_CTL_TYPE_S(XU, BYPASS):
+	case SDCA_CTL_TYPE_S(MFPU, BYPASS):
+	case SDCA_CTL_TYPE_S(FU, MUTE):
+	case SDCA_CTL_TYPE_S(CX, CLOCK_SELECT):
+		control->has_reset = true;
+		control->reset = 1;
+		break;
+	case SDCA_CTL_TYPE_S(PDE, REQUESTED_PS):
+		control->has_reset = true;
+		control->reset = 3;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static int find_sdca_entity_control(struct device *dev, struct sdca_entity *entity,
 				    struct fwnode_handle *control_node,
 				    struct sdca_control *control)
@@ -990,6 +1018,10 @@ static int find_sdca_entity_control(struct device *dev, struct sdca_entity *enti
 
 	control->is_volatile = find_sdca_control_volatile(entity, control);
 
+	ret = find_sdca_control_reset(entity, control);
+	if (ret)
+		return ret;
+
 	ret = find_sdca_control_range(dev, control_node, &control->range);
 	if (ret) {
 		dev_err(dev, "%s: control %#x: range missing: %d\n",
@@ -2033,6 +2065,7 @@ static int find_sdca_filesets(struct device *dev, struct sdw_slave *sdw,
 	num_sets = fwnode_property_count_u32(function_node,
 					     "mipi-sdca-file-set-id-list");
 	if (num_sets == 0 || num_sets == -EINVAL) {
+		dev_dbg(dev, "%pfwP: file set id list missing\n", function_node);
 		return 0;
 	} else if (num_sets < 0) {
 		dev_err(dev, "%pfwP: failed to read file set list: %d\n",

sound/soc/sdca/sdca_jack.c

@@ -41,10 +41,11 @@ int sdca_jack_process(struct sdca_interrupt *interrupt)
 	struct jack_state *state = interrupt->priv;
 	struct snd_kcontrol *kctl = state->kctl;
 	struct snd_ctl_elem_value *ucontrol __free(kfree) = NULL;
-	struct soc_enum *soc_enum;
 	unsigned int reg, val;
 	int ret;
 
+	guard(rwsem_write)(rwsem);
+
 	if (!kctl) {
 		const char *name __free(kfree) = kasprintf(GFP_KERNEL, "%s %s",
 							   interrupt->entity->label,
@@ -54,16 +55,12 @@ int sdca_jack_process(struct sdca_interrupt *interrupt)
 			return -ENOMEM;
 
 		kctl = snd_soc_component_get_kcontrol(component, name);
-		if (!kctl) {
+		if (!kctl)
 			dev_dbg(dev, "control not found: %s\n", name);
-			return -ENOENT;
-		}
-
-		state->kctl = kctl;
+		else
+			state->kctl = kctl;
 	}
 
-	soc_enum = (struct soc_enum *)kctl->private_value;
-
 	reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
 			   interrupt->control->sel, 0);
 
@@ -73,13 +70,12 @@ int sdca_jack_process(struct sdca_interrupt *interrupt)
 		return ret;
 	}
 
+	reg = SDW_SDCA_CTL(interrupt->function->desc->adr, interrupt->entity->id,
+			   SDCA_CTL_GE_SELECTED_MODE, 0);
+
 	switch (val) {
 	case SDCA_DETECTED_MODE_DETECTION_IN_PROGRESS:
 	case SDCA_DETECTED_MODE_JACK_UNKNOWN:
-		reg = SDW_SDCA_CTL(interrupt->function->desc->adr,
-				   interrupt->entity->id,
-				   SDCA_CTL_GE_SELECTED_MODE, 0);
-
 		/*
 		 * Selected mode is not normally marked as volatile register
 		 * (RW), but here force a read from the hardware. If the
@@ -100,21 +96,29 @@ int sdca_jack_process(struct sdca_interrupt *interrupt)
 
 	dev_dbg(dev, "%s: %#x\n", interrupt->name, val);
 
-	ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL);
-	if (!ucontrol)
-		return -ENOMEM;
+	if (kctl) {
+		struct soc_enum *soc_enum = (struct soc_enum *)kctl->private_value;
+
+		ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL);
+		if (!ucontrol)
+			return -ENOMEM;
 
-	ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(soc_enum, val);
+		ucontrol->value.enumerated.item[0] = snd_soc_enum_val_to_item(soc_enum, val);
 
-	down_write(rwsem);
-	ret = kctl->put(kctl, ucontrol);
-	up_write(rwsem);
-	if (ret < 0) {
-		dev_err(dev, "failed to update selected mode: %d\n", ret);
-		return ret;
-	}
+		ret = snd_soc_dapm_put_enum_double(kctl, ucontrol);
+		if (ret < 0) {
+			dev_err(dev, "failed to update selected mode: %d\n", ret);
+			return ret;
+		}
 
-	snd_ctl_notify(card->snd_card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
+		snd_ctl_notify(card->snd_card, SNDRV_CTL_EVENT_MASK_VALUE, &kctl->id);
+	} else {
+		ret = regmap_write(interrupt->function_regmap, reg, val);
+		if (ret) {
+			dev_err(dev, "failed to write selected mode: %d\n", ret);
+			return ret;
+		}
+	}
 
 	return sdca_jack_report(interrupt);
 }

sound/soc/sdca/sdca_regmap.c

@@ -218,7 +218,8 @@ int sdca_regmap_count_constants(struct device *dev,
 		struct sdca_entity *entity = &function->entities[i];
 
 		for (j = 0; j < entity->num_controls; j++) {
-			if (entity->controls[j].mode == SDCA_ACCESS_MODE_DC)
+			if (entity->controls[j].mode == SDCA_ACCESS_MODE_DC ||
+			    entity->controls[j].has_reset)
 				nconsts += hweight64(entity->controls[j].cn_list);
 		}
 	}
@@ -255,7 +256,8 @@ int sdca_regmap_populate_constants(struct device *dev,
 			struct sdca_control *control = &entity->controls[j];
 			int cn;
 
-			if (control->mode != SDCA_ACCESS_MODE_DC)
+			if (control->mode != SDCA_ACCESS_MODE_DC &&
+			    !control->has_reset)
 				continue;
 
 			l = 0;
@@ -264,7 +266,10 @@ int sdca_regmap_populate_constants(struct device *dev,
 				consts[k].reg = SDW_SDCA_CTL(function->desc->adr,
 							     entity->id,
 							     control->sel, cn);
-				consts[k].def = control->values[l];
+				if (control->mode == SDCA_ACCESS_MODE_DC)
+					consts[k].def = control->values[l];
+				else
+					consts[k].def = control->reset;
 				k++;
 				l++;
 			}
@@ -306,6 +311,9 @@ static int populate_control_defaults(struct device *dev, struct regmap *regmap,
 
 			i++;
 		} else if (!control->is_volatile) {
+			if (control->has_reset)
+				regcache_drop_region(regmap, reg, reg);
+
 			ret = regmap_read(regmap, reg, &val);
 			if (ret) {
 				dev_err(dev, "Failed to read initial %#x: %d\n",