ASoC: Intel: catpt: IPC log improvements and code

Merge series from Cezary Rojewski <cezary.rojewski@intel.com>:

Entire patchset provides no new features and does not alter the code
from functional (user) perspective.

The first two improve IPC-error logging 'mechanism' and align the
catpt-driver with what's done in another Intel's driver: the avs-driver.
In essence, no need to log the error in every function, let the common
handler do so instead.

The last three simplify the code, and fix some spacing issues.  All in
all, we get better readability with lower LOC.

sound/soc/intel/catpt/core.h

@@ -62,6 +62,7 @@ struct catpt_module_type {
 struct catpt_spec {
 	struct snd_soc_acpi_mach *machines;
 	u8 core_id;
+	const char *fw_name;
 	u32 host_dram_offset;
 	u32 host_iram_offset;
 	u32 host_shim_offset;
@@ -129,13 +130,13 @@ irqreturn_t catpt_dsp_irq_thread(int irq, void *dev_id);
  * HOST <-> DSP communication yet failure to process specific request.
  * Use below macro to convert returned non-zero values appropriately
  */
-#define CATPT_IPC_ERROR(err) (((err) < 0) ? (err) : -EREMOTEIO)
+#define CATPT_IPC_RET(ret)	(((ret) <= 0) ? (ret) : -EREMOTEIO)
 
 int catpt_dsp_send_msg_timeout(struct catpt_dev *cdev,
 			       struct catpt_ipc_msg request,
-			       struct catpt_ipc_msg *reply, int timeout);
+			       struct catpt_ipc_msg *reply, int timeout, const char *name);
 int catpt_dsp_send_msg(struct catpt_dev *cdev, struct catpt_ipc_msg request,
-		       struct catpt_ipc_msg *reply);
+		       struct catpt_ipc_msg *reply, const char *name);
 
 int catpt_first_boot_firmware(struct catpt_dev *cdev);
 int catpt_boot_firmware(struct catpt_dev *cdev, bool restore);

sound/soc/intel/catpt/device.c

@@ -41,7 +41,7 @@ static int catpt_do_suspend(struct device *dev)
 	memset(&cdev->dx_ctx, 0, sizeof(cdev->dx_ctx));
 	ret = catpt_ipc_enter_dxstate(cdev, CATPT_DX_STATE_D3, &cdev->dx_ctx);
 	if (ret) {
-		ret = CATPT_IPC_ERROR(ret);
+		ret = CATPT_IPC_RET(ret);
 		goto release_dma_chan;
 	}
 
@@ -107,7 +107,7 @@ static int catpt_resume(struct device *dev)
 
 		ret = catpt_ipc_set_device_format(cdev, &cdev->devfmt[i]);
 		if (ret)
-			return CATPT_IPC_ERROR(ret);
+			return CATPT_IPC_RET(ret);
 	}
 
 	return 0;
@@ -348,6 +348,7 @@ static struct snd_soc_acpi_mach wpt_machines[] = {
 static struct catpt_spec lpt_desc = {
 	.machines = lpt_machines,
 	.core_id = 0x01,
+	.fw_name = "intel/IntcSST1.bin",
 	.host_dram_offset = 0x000000,
 	.host_iram_offset = 0x080000,
 	.host_shim_offset = 0x0E7000,
@@ -363,6 +364,7 @@ static struct catpt_spec lpt_desc = {
 static struct catpt_spec wpt_desc = {
 	.machines = wpt_machines,
 	.core_id = 0x02,
+	.fw_name = "intel/IntcSST2.bin",
 	.host_dram_offset = 0x000000,
 	.host_iram_offset = 0x0A0000,
 	.host_shim_offset = 0x0FB000,

sound/soc/intel/catpt/ipc.c

@@ -84,7 +84,7 @@ static int catpt_wait_msg_completion(struct catpt_dev *cdev, int timeout)
 
 static int catpt_dsp_do_send_msg(struct catpt_dev *cdev,
 				 struct catpt_ipc_msg request,
-				 struct catpt_ipc_msg *reply, int timeout)
+				 struct catpt_ipc_msg *reply, int timeout, const char *name)
 {
 	struct catpt_ipc *ipc = &cdev->ipc;
 	unsigned long flags;
@@ -111,6 +111,8 @@ static int catpt_dsp_do_send_msg(struct catpt_dev *cdev,
 	}
 
 	ret = ipc->rx.rsp.status;
+	if (ret)
+		dev_err(cdev->dev, "%s (0x%08x) failed: %d\n", name, request.header, ret);
 	if (reply) {
 		reply->header = ipc->rx.header;
 
@@ -123,23 +125,23 @@ static int catpt_dsp_do_send_msg(struct catpt_dev *cdev,
 
 int catpt_dsp_send_msg_timeout(struct catpt_dev *cdev,
 			       struct catpt_ipc_msg request,
-			       struct catpt_ipc_msg *reply, int timeout)
+			       struct catpt_ipc_msg *reply, int timeout, const char *name)
 {
 	struct catpt_ipc *ipc = &cdev->ipc;
 	int ret;
 
 	mutex_lock(&ipc->mutex);
-	ret = catpt_dsp_do_send_msg(cdev, request, reply, timeout);
+	ret = catpt_dsp_do_send_msg(cdev, request, reply, timeout, name);
 	mutex_unlock(&ipc->mutex);
 
 	return ret;
 }
 
 int catpt_dsp_send_msg(struct catpt_dev *cdev, struct catpt_ipc_msg request,
-		       struct catpt_ipc_msg *reply)
+		       struct catpt_ipc_msg *reply, const char *name)
 {
 	return catpt_dsp_send_msg_timeout(cdev, request, reply,
-					  cdev->ipc.default_timeout);
+					  cdev->ipc.default_timeout, name);
 }
 
 static void

sound/soc/intel/catpt/loader.c

@@ -580,10 +580,6 @@ static int catpt_load_image(struct catpt_dev *cdev, struct dma_chan *chan,
 
 static int catpt_load_images(struct catpt_dev *cdev, bool restore)
 {
-	static const char *const names[] = {
-		"intel/IntcSST1.bin",
-		"intel/IntcSST2.bin",
-	};
 	struct dma_chan *chan;
 	int ret;
 
@@ -591,7 +587,7 @@ static int catpt_load_images(struct catpt_dev *cdev, bool restore)
 	if (IS_ERR(chan))
 		return PTR_ERR(chan);
 
-	ret = catpt_load_image(cdev, chan, names[cdev->spec->core_id - 1],
+	ret = catpt_load_image(cdev, chan, cdev->spec->fw_name,
 			       FW_SIGNATURE, restore);
 	if (ret)
 		goto release_dma_chan;
@@ -656,7 +652,7 @@ int catpt_first_boot_firmware(struct catpt_dev *cdev)
 
 	ret = catpt_ipc_get_mixer_stream_info(cdev, &cdev->mixer);
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	ret = catpt_arm_stream_templates(cdev);
 	if (ret) {

sound/soc/intel/catpt/messages.c

@@ -15,17 +15,12 @@ int catpt_ipc_get_fw_version(struct catpt_dev *cdev,
 {
 	union catpt_global_msg msg = CATPT_GLOBAL_MSG(GET_FW_VERSION);
 	struct catpt_ipc_msg request = {{0}}, reply;
-	int ret;
 
 	request.header = msg.val;
 	reply.size = sizeof(*version);
 	reply.data = version;
 
-	ret = catpt_dsp_send_msg(cdev, request, &reply);
-	if (ret)
-		dev_err(cdev->dev, "get fw version failed: %d\n", ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, &reply, "get fw version");
 }
 
 struct catpt_alloc_stream_input {
@@ -94,11 +89,7 @@ int catpt_ipc_alloc_stream(struct catpt_dev *cdev,
 	reply.size = sizeof(*sinfo);
 	reply.data = sinfo;
 
-	ret = catpt_dsp_send_msg(cdev, request, &reply);
-	if (ret)
-		dev_err(cdev->dev, "alloc stream type %d failed: %d\n",
-			type, ret);
-
+	ret = catpt_dsp_send_msg(cdev, request, &reply, "alloc stream");
 	kfree(payload);
 	return ret;
 }
@@ -107,18 +98,12 @@ int catpt_ipc_free_stream(struct catpt_dev *cdev, u8 stream_hw_id)
 {
 	union catpt_global_msg msg = CATPT_GLOBAL_MSG(FREE_STREAM);
 	struct catpt_ipc_msg request;
-	int ret;
 
 	request.header = msg.val;
 	request.size = sizeof(stream_hw_id);
 	request.data = &stream_hw_id;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "free stream %d failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "free stream");
 }
 
 int catpt_ipc_set_device_format(struct catpt_dev *cdev,
@@ -126,17 +111,12 @@ int catpt_ipc_set_device_format(struct catpt_dev *cdev,
 {
 	union catpt_global_msg msg = CATPT_GLOBAL_MSG(SET_DEVICE_FORMATS);
 	struct catpt_ipc_msg request;
-	int ret;
 
 	request.header = msg.val;
 	request.size = sizeof(*devfmt);
 	request.data = devfmt;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "set device format failed: %d\n", ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "set device format");
 }
 
 int catpt_ipc_enter_dxstate(struct catpt_dev *cdev, enum catpt_dx_state state,
@@ -144,7 +124,6 @@ int catpt_ipc_enter_dxstate(struct catpt_dev *cdev, enum catpt_dx_state state,
 {
 	union catpt_global_msg msg = CATPT_GLOBAL_MSG(ENTER_DX_STATE);
 	struct catpt_ipc_msg request, reply;
-	int ret;
 
 	request.header = msg.val;
 	request.size = sizeof(state);
@@ -152,11 +131,7 @@ int catpt_ipc_enter_dxstate(struct catpt_dev *cdev, enum catpt_dx_state state,
 	reply.size = sizeof(*context);
 	reply.data = context;
 
-	ret = catpt_dsp_send_msg(cdev, request, &reply);
-	if (ret)
-		dev_err(cdev->dev, "enter dx state failed: %d\n", ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, &reply, "enter dx state");
 }
 
 int catpt_ipc_get_mixer_stream_info(struct catpt_dev *cdev,
@@ -164,68 +139,45 @@ int catpt_ipc_get_mixer_stream_info(struct catpt_dev *cdev,
 {
 	union catpt_global_msg msg = CATPT_GLOBAL_MSG(GET_MIXER_STREAM_INFO);
 	struct catpt_ipc_msg request = {{0}}, reply;
-	int ret;
 
 	request.header = msg.val;
 	reply.size = sizeof(*info);
 	reply.data = info;
 
-	ret = catpt_dsp_send_msg(cdev, request, &reply);
-	if (ret)
-		dev_err(cdev->dev, "get mixer info failed: %d\n", ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, &reply, "get mixer info");
 }
 
 int catpt_ipc_reset_stream(struct catpt_dev *cdev, u8 stream_hw_id)
 {
 	union catpt_stream_msg msg = CATPT_STREAM_MSG(RESET_STREAM);
 	struct catpt_ipc_msg request = {{0}};
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	request.header = msg.val;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "reset stream %d failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "reset stream");
 }
 
 int catpt_ipc_pause_stream(struct catpt_dev *cdev, u8 stream_hw_id)
 {
 	union catpt_stream_msg msg = CATPT_STREAM_MSG(PAUSE_STREAM);
 	struct catpt_ipc_msg request = {{0}};
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	request.header = msg.val;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "pause stream %d failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "pause stream");
 }
 
 int catpt_ipc_resume_stream(struct catpt_dev *cdev, u8 stream_hw_id)
 {
 	union catpt_stream_msg msg = CATPT_STREAM_MSG(RESUME_STREAM);
 	struct catpt_ipc_msg request = {{0}};
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	request.header = msg.val;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "resume stream %d failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "resume stream");
 }
 
 struct catpt_set_volume_input {
@@ -243,7 +195,6 @@ int catpt_ipc_set_volume(struct catpt_dev *cdev, u8 stream_hw_id,
 	union catpt_stream_msg msg = CATPT_STAGE_MSG(SET_VOLUME);
 	struct catpt_ipc_msg request;
 	struct catpt_set_volume_input input;
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	input.channel = channel;
@@ -255,12 +206,7 @@ int catpt_ipc_set_volume(struct catpt_dev *cdev, u8 stream_hw_id,
 	request.size = sizeof(input);
 	request.data = &input;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "set stream %d volume failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "set stream volume");
 }
 
 struct catpt_set_write_pos_input {
@@ -275,7 +221,6 @@ int catpt_ipc_set_write_pos(struct catpt_dev *cdev, u8 stream_hw_id,
 	union catpt_stream_msg msg = CATPT_STAGE_MSG(SET_WRITE_POSITION);
 	struct catpt_ipc_msg request;
 	struct catpt_set_write_pos_input input;
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	input.new_write_pos = pos;
@@ -286,28 +231,18 @@ int catpt_ipc_set_write_pos(struct catpt_dev *cdev, u8 stream_hw_id,
 	request.size = sizeof(input);
 	request.data = &input;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "set stream %d write pos failed: %d\n",
-			stream_hw_id, ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "set stream write pos");
 }
 
 int catpt_ipc_mute_loopback(struct catpt_dev *cdev, u8 stream_hw_id, bool mute)
 {
 	union catpt_stream_msg msg = CATPT_STAGE_MSG(MUTE_LOOPBACK);
 	struct catpt_ipc_msg request;
-	int ret;
 
 	msg.stream_hw_id = stream_hw_id;
 	request.header = msg.val;
 	request.size = sizeof(mute);
 	request.data = &mute;
 
-	ret = catpt_dsp_send_msg(cdev, request, NULL);
-	if (ret)
-		dev_err(cdev->dev, "mute loopback failed: %d\n", ret);
-
-	return ret;
+	return catpt_dsp_send_msg(cdev, request, NULL, "mute loopback");
 }

sound/soc/intel/catpt/pcm.c

@@ -114,14 +114,10 @@ catpt_stream_find(struct catpt_dev *cdev, u8 stream_hw_id)
 	return result;
 }
 
-static u32 catpt_stream_read_position(struct catpt_dev *cdev,
-				      struct catpt_stream_runtime *stream)
+static void catpt_stream_read_position(struct catpt_dev *cdev,
+				       struct catpt_stream_runtime *stream, u32 *pos)
 {
-	u32 pos;
-
-	memcpy_fromio(&pos, cdev->lpe_ba + stream->info.read_pos_regaddr,
-		      sizeof(pos));
-	return pos;
+	memcpy_fromio(pos, cdev->lpe_ba + stream->info.read_pos_regaddr, sizeof(*pos));
 }
 
 static u32 catpt_stream_volume(struct catpt_dev *cdev,
@@ -365,9 +361,7 @@ static int catpt_dai_apply_usettings(struct snd_soc_dai *dai,
 	if (stream->template->type != CATPT_STRM_TYPE_LOOPBACK)
 		return catpt_set_dspvol(cdev, id, (long *)pos->private_value);
 	ret = catpt_ipc_mute_loopback(cdev, id, *(bool *)pos->private_value);
-	if (ret)
-		return CATPT_IPC_ERROR(ret);
-	return 0;
+	return CATPT_IPC_RET(ret);
 }
 
 static int catpt_dai_hw_params(struct snd_pcm_substream *substream,
@@ -414,7 +408,7 @@ static int catpt_dai_hw_params(struct snd_pcm_substream *substream,
 				     cdev->scratch,
 				     &stream->info);
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	ret = catpt_dai_apply_usettings(dai, stream);
 	if (ret) {
@@ -456,11 +450,11 @@ static int catpt_dai_prepare(struct snd_pcm_substream *substream,
 
 	ret = catpt_ipc_reset_stream(cdev, stream->info.stream_hw_id);
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	ret = catpt_ipc_pause_stream(cdev, stream->info.stream_hw_id);
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	stream->prepared = true;
 	return 0;
@@ -491,7 +485,7 @@ static int catpt_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		ret = catpt_ipc_set_write_pos(cdev, stream->info.stream_hw_id,
 					      pos, false, false);
 		if (ret)
-			return CATPT_IPC_ERROR(ret);
+			return CATPT_IPC_RET(ret);
 		fallthrough;
 	case SNDRV_PCM_TRIGGER_RESUME:
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
@@ -499,7 +493,7 @@ static int catpt_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		catpt_dsp_update_lpclock(cdev);
 		ret = catpt_ipc_resume_stream(cdev, stream->info.stream_hw_id);
 		if (ret)
-			return CATPT_IPC_ERROR(ret);
+			return CATPT_IPC_RET(ret);
 		break;
 
 	case SNDRV_PCM_TRIGGER_STOP:
@@ -510,7 +504,7 @@ static int catpt_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 		ret = catpt_ipc_pause_stream(cdev, stream->info.stream_hw_id);
 		catpt_dsp_update_lpclock(cdev);
 		if (ret)
-			return CATPT_IPC_ERROR(ret);
+			return CATPT_IPC_RET(ret);
 		break;
 
 	default:
@@ -617,7 +611,7 @@ catpt_component_pointer(struct snd_soc_component *component,
 		return 0;
 
 	stream = snd_soc_dai_get_dma_data(cpu_dai, substream);
-	pos = catpt_stream_read_position(cdev, stream);
+	catpt_stream_read_position(cdev, stream, &pos);
 
 	return bytes_to_frames(substream->runtime, pos);
 }
@@ -679,7 +673,7 @@ static int catpt_dai_pcm_new(struct snd_soc_pcm_runtime *rtm,
 	pm_runtime_put_autosuspend(cdev->dev);
 
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	/* store device format set for given SSP */
 	memcpy(&cdev->devfmt[devfmt.iface], &devfmt, sizeof(devfmt));
@@ -693,7 +687,7 @@ static const struct snd_soc_dai_ops catpt_dai_ops = {
 static struct snd_soc_dai_driver dai_drivers[] = {
 /* FE DAIs */
 {
-	.name  = "System Pin",
+	.name = "System Pin",
 	.id = CATPT_STRM_TYPE_SYSTEM,
 	.ops = &catpt_fe_dai_ops,
 	.playback = {
@@ -716,7 +710,7 @@ static struct snd_soc_dai_driver dai_drivers[] = {
 	},
 },
 {
-	.name  = "Offload0 Pin",
+	.name = "Offload0 Pin",
 	.id = CATPT_STRM_TYPE_RENDER,
 	.ops = &catpt_fe_dai_ops,
 	.playback = {
@@ -730,7 +724,7 @@ static struct snd_soc_dai_driver dai_drivers[] = {
 	},
 },
 {
-	.name  = "Offload1 Pin",
+	.name = "Offload1 Pin",
 	.id = CATPT_STRM_TYPE_RENDER,
 	.ops = &catpt_fe_dai_ops,
 	.playback = {
@@ -744,7 +738,7 @@ static struct snd_soc_dai_driver dai_drivers[] = {
 	},
 },
 {
-	.name  = "Loopback Pin",
+	.name = "Loopback Pin",
 	.id = CATPT_STRM_TYPE_LOOPBACK,
 	.ops = &catpt_fe_dai_ops,
 	.capture = {
@@ -758,7 +752,7 @@ static struct snd_soc_dai_driver dai_drivers[] = {
 	},
 },
 {
-	.name  = "Bluetooth Pin",
+	.name = "Bluetooth Pin",
 	.id = CATPT_STRM_TYPE_BLUETOOTH_RENDER,
 	.ops = &catpt_fe_dai_ops,
 	.playback = {
@@ -849,9 +843,7 @@ static int catpt_set_dspvol(struct catpt_dev *cdev, u8 stream_id, long *ctlvol)
 		}
 	}
 
-	if (ret)
-		return CATPT_IPC_ERROR(ret);
-	return 0;
+	return CATPT_IPC_RET(ret);
 }
 
 static int catpt_volume_info(struct snd_kcontrol *kcontrol,
@@ -1041,7 +1033,7 @@ static int catpt_loopback_switch_put(struct snd_kcontrol *kcontrol,
 	pm_runtime_put_autosuspend(cdev->dev);
 
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	*(bool *)kcontrol->private_value = mute;
 	return 0;

sound/soc/intel/catpt/sysfs.c

@@ -24,7 +24,7 @@ static ssize_t fw_version_show(struct device *dev,
 	pm_runtime_put_autosuspend(cdev->dev);
 
 	if (ret)
-		return CATPT_IPC_ERROR(ret);
+		return CATPT_IPC_RET(ret);
 
 	return sysfs_emit(buf, "%d.%d.%d.%d\n", version.type, version.major,
 			  version.minor, version.build);