[PATCH 00/12] ASoC: SOF: Abstract away the 'bar' usage (mostly)

[PATCH 00/12] ASoC: SOF: Abstract away the 'bar' usage (mostly)

[Peter Ujfalusi]

Hi,

The direct use of 'bar' as address regions in SOF can limit portability and
makes assumption that all platforms are using memory mapped regions or BARs
to communicate with the DSP.

The core API should not use 'bar' in it's parameter lists or make assumption
that the platform needs to comply with this system integration.

Use the region type as an ID in core and translate that in platform dependent
code to a way to access registers, mailbox or DSP memory.

Regards,
Peter
---
Peter Ujfalusi (12):
  ASoC: SOF: Intel: bdw: Set the mailbox offset directly in bdw_probe
  ASoC: SOF: ipc: Remove snd_sof_dsp_mailbox_init()
  ASoC: SOF: imx: Do not initialize the snd_sof_dsp_ops.read64
  ASoC: SOF: imx: imx8: Bar index is only valid for IRAM and SRAM types
  ASoC: SOF: imx: imx8m: Bar index is only valid for IRAM and SRAM types
  ASoC: SOF: loader: No need to export snd_sof_fw_parse_ext_data()
  ASoC: SOF: core: Do not use 'bar' as parameter for block_read/write
  ASoC: SOF: debug: Add generic API and ops for DSP regions
  ASoC: SOF: imx: Provide debugfs_add_region_item ops for core
  ASoC: SOF: Intel: Provide debugfs_add_region_item ops for core
  ASoC: SOF: loader: Use the generic ops for region debugfs handling
  ASoC: SOF: debug: No need to export the snd_sof_debugfs_io_item()

 sound/soc/sof/debug.c         |  24 ++++--
 sound/soc/sof/imx/imx8.c      |  17 +++--
 sound/soc/sof/imx/imx8m.c     |  13 +++-
 sound/soc/sof/intel/apl.c     |   1 +
 sound/soc/sof/intel/bdw.c     |   5 +-
 sound/soc/sof/intel/byt.c     |   2 +
 sound/soc/sof/intel/cnl.c     |   1 +
 sound/soc/sof/intel/icl.c     |   1 +
 sound/soc/sof/intel/pci-tng.c |   1 +
 sound/soc/sof/intel/tgl.c     |   1 +
 sound/soc/sof/ipc.c           |  34 +++------
 sound/soc/sof/loader.c        | 137 +++++++++++++++-------------------
 sound/soc/sof/ops.h           |  25 +++++--
 sound/soc/sof/sof-priv.h      |  55 +++++++-------
 sound/soc/sof/utils.c         |  28 +++++--
 15 files changed, 187 insertions(+), 158 deletions(-)

-- 
2.33.0

[PATCH 01/12] ASoC: SOF: Intel: bdw: Set the mailbox offset directly in bdw_probe

[Peter Ujfalusi]

To align with other platforms, set only the sdev->dsp_box.offset in
bdw_probe().
The mailbox offset must be set in order to be able to receive the firmware
ready message.

The offsets and sizes will be re-configured after the FW ready message
based on the window information.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/intel/bdw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c
index 89a6c1f04a55..37fa4f976a11 100644
--- a/sound/soc/sof/intel/bdw.c
+++ b/sound/soc/sof/intel/bdw.c
@@ -535,8 +535,8 @@ static int bdw_probe(struct snd_sof_dev *sdev)
 		return ret;
 	}
 
-	/* set default mailbox */
-	snd_sof_dsp_mailbox_init(sdev, MBOX_OFFSET, MBOX_SIZE, 0, 0);
+	/* set default mailbox offset for FW ready message */
+	sdev->dsp_box.offset = MBOX_OFFSET;
 
 	return ret;
 }
-- 
2.33.0

[PATCH 02/12] ASoC: SOF: ipc: Remove snd_sof_dsp_mailbox_init()

[Peter Ujfalusi]

The snd_sof_dsp_mailbox_init() is called only from sof_get_windows()
to set the sdev->dsp_box.offset/size and sdev->host_box.offset/size

Instead of using a function, set the offsets and sizes like we do for the
other boxes in sof_get_windows().

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/ipc.c      | 16 ----------------
 sound/soc/sof/loader.c   |  8 ++++++--
 sound/soc/sof/sof-priv.h |  3 ---
 3 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index a4fe007a0e4d..9ca3681d266d 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -790,22 +790,6 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 }
 EXPORT_SYMBOL(snd_sof_ipc_set_get_comp_data);
 
-/*
- * IPC layer enumeration.
- */
-
-int snd_sof_dsp_mailbox_init(struct snd_sof_dev *sdev, u32 dspbox,
-			     size_t dspbox_size, u32 hostbox,
-			     size_t hostbox_size)
-{
-	sdev->dsp_box.offset = dspbox;
-	sdev->dsp_box.size = dspbox_size;
-	sdev->host_box.offset = hostbox;
-	sdev->host_box.size = hostbox_size;
-	return 0;
-}
-EXPORT_SYMBOL(snd_sof_dsp_mailbox_init);
-
 int snd_sof_ipc_valid(struct snd_sof_dev *sdev)
 {
 	struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c
index 2b38a77cd594..5bedb81f6bfb 100644
--- a/sound/soc/sof/loader.c
+++ b/sound/soc/sof/loader.c
@@ -470,8 +470,12 @@ static void sof_get_windows(struct snd_sof_dev *sdev)
 		return;
 	}
 
-	snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
-				 outbox_offset, outbox_size);
+	sdev->dsp_box.offset = inbox_offset;
+	sdev->dsp_box.size = inbox_size;
+
+	sdev->host_box.offset = outbox_offset;
+	sdev->host_box.size = outbox_size;
+
 	sdev->stream_box.offset = stream_offset;
 	sdev->stream_box.size = stream_size;
 
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index fd8423172d8f..b98fc9bf660b 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -503,9 +503,6 @@ void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id);
 void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev);
 int snd_sof_ipc_stream_pcm_params(struct snd_sof_dev *sdev,
 				  struct sof_ipc_pcm_params *params);
-int snd_sof_dsp_mailbox_init(struct snd_sof_dev *sdev, u32 dspbox,
-			     size_t dspbox_size, u32 hostbox,
-			     size_t hostbox_size);
 int snd_sof_ipc_valid(struct snd_sof_dev *sdev);
 int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 		       void *msg_data, size_t msg_bytes, void *reply_data,
-- 
2.33.0

[PATCH 03/12] ASoC: SOF: imx: Do not initialize the snd_sof_dsp_ops.read64

[Peter Ujfalusi]

The read64 operation is not used by IMX along with other IO functions.
No need to set it for the ops.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Bard Liao <bard.liao@intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
---
 sound/soc/sof/imx/imx8.c  | 6 ------
 sound/soc/sof/imx/imx8m.c | 3 ---
 2 files changed, 9 deletions(-)

diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index 12fedf0984bd..326aa65166c2 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -419,9 +419,6 @@ struct snd_sof_dsp_ops sof_imx8_ops = {
 	.block_read	= sof_block_read,
 	.block_write	= sof_block_write,
 
-	/* Module IO */
-	.read64	= sof_io_read64,
-
 	/* ipc */
 	.send_msg	= imx8_send_msg,
 	.fw_ready	= sof_fw_ready,
@@ -468,9 +465,6 @@ struct snd_sof_dsp_ops sof_imx8x_ops = {
 	.block_read	= sof_block_read,
 	.block_write	= sof_block_write,
 
-	/* Module IO */
-	.read64	= sof_io_read64,
-
 	/* ipc */
 	.send_msg	= imx8_send_msg,
 	.fw_ready	= sof_fw_ready,
diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index cb822d953767..b5c739a5cbeb 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -271,9 +271,6 @@ struct snd_sof_dsp_ops sof_imx8m_ops = {
 	.block_read	= sof_block_read,
 	.block_write	= sof_block_write,
 
-	/* Module IO */
-	.read64	= sof_io_read64,
-
 	/* ipc */
 	.send_msg	= imx8m_send_msg,
 	.fw_ready	= sof_fw_ready,
-- 
2.33.0

[PATCH 04/12] ASoC: SOF: imx: imx8: Bar index is only valid for IRAM and SRAM types

[Peter Ujfalusi]

i.MX8 only uses SOF_FW_BLK_TYPE_IRAM (1) and SOF_FW_BLK_TYPE_SRAM (3)
bars, everything else is left as 0 in sdev->bar[] array.

If a broken or purposefully crafted firmware image is loaded with other
types of FW_BLK_TYPE then a kernel crash can be triggered.

Make sure that only IRAM/SRAM type is converted to bar index.
Fixes: 202acc565a1f0 ("ASoC: SOF: imx: Add i.MX8 HW support")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/imx/imx8.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index 326aa65166c2..150f1bc3fb95 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -365,7 +365,14 @@ static int imx8_remove(struct snd_sof_dev *sdev)
 /* on i.MX8 there is 1 to 1 match between type and BAR idx */
 static int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type)
 {
-	return type;
+	/* Only IRAM and SRAM bars are valid */
+	switch (type) {
+	case SOF_FW_BLK_TYPE_IRAM:
+	case SOF_FW_BLK_TYPE_SRAM:
+		return type;
+	default:
+		return -EINVAL;
+	}
 }
 
 static void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
-- 
2.33.0

Re: [PATCH 04/12] ASoC: SOF: imx: imx8: Bar index is only valid for IRAM and SRAM types

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 407 bytes --]

On Wed, Sep 15, 2021 at 03:21:08PM +0300, Peter Ujfalusi wrote:
> i.MX8 only uses SOF_FW_BLK_TYPE_IRAM (1) and SOF_FW_BLK_TYPE_SRAM (3)
> bars, everything else is left as 0 in sdev->bar[] array.
> 
> If a broken or purposefully crafted firmware image is loaded with other
> types of FW_BLK_TYPE then a kernel crash can be triggered.

This is a fix which should have been at the start of the series.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 04/12] ASoC: SOF: imx: imx8: Bar index is only valid for IRAM and SRAM types

[Péter Ujfalusi]

On 15/09/2021 16:00, Mark Brown wrote:
> On Wed, Sep 15, 2021 at 03:21:08PM +0300, Peter Ujfalusi wrote:
>> i.MX8 only uses SOF_FW_BLK_TYPE_IRAM (1) and SOF_FW_BLK_TYPE_SRAM (3)
>> bars, everything else is left as 0 in sdev->bar[] array.
>>
>> If a broken or purposefully crafted firmware image is loaded with other
>> types of FW_BLK_TYPE then a kernel crash can be triggered.
> 
> This is a fix which should have been at the start of the series.

The same is true for the next patch for imx8m, I'll move it ahead in the v2.

-- 
Péter

[PATCH 05/12] ASoC: SOF: imx: imx8m: Bar index is only valid for IRAM and SRAM types

[Peter Ujfalusi]

i.MX8 only uses SOF_FW_BLK_TYPE_IRAM (1) and SOF_FW_BLK_TYPE_SRAM (3)
bars, everything else is left as 0 in sdev->bar[] array.

If a broken or purposefully crafted firmware image is loaded with other
types of FW_BLK_TYPE then a kernel crash can be triggered.

Make sure that only IRAM/SRAM type is converted to bar index.

Fixes: afb93d716533d ("ASoC: SOF: imx: Add i.MX8M HW support")
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/imx/imx8m.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index b5c739a5cbeb..be5ab9942fc8 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -228,7 +228,14 @@ static int imx8m_remove(struct snd_sof_dev *sdev)
 /* on i.MX8 there is 1 to 1 match between type and BAR idx */
 static int imx8m_get_bar_index(struct snd_sof_dev *sdev, u32 type)
 {
-	return type;
+	/* Only IRAM and SRAM bars are valid */
+	switch (type) {
+	case SOF_FW_BLK_TYPE_IRAM:
+	case SOF_FW_BLK_TYPE_SRAM:
+		return type;
+	default:
+		return -EINVAL;
+	}
 }
 
 static void imx8m_ipc_msg_data(struct snd_sof_dev *sdev,
-- 
2.33.0

[PATCH 06/12] ASoC: SOF: loader: No need to export snd_sof_fw_parse_ext_data()

[Peter Ujfalusi]

snd_sof_fw_parse_ext_data() is used only internally within loader.c and
there is no need to export it.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/loader.c   | 3 +--
 sound/soc/sof/sof-priv.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c
index 5bedb81f6bfb..6d83ff11effc 100644
--- a/sound/soc/sof/loader.c
+++ b/sound/soc/sof/loader.c
@@ -86,7 +86,7 @@ static int get_cc_info(struct snd_sof_dev *sdev,
 }
 
 /* parse the extended FW boot data structures from FW boot message */
-int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset)
+static int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset)
 {
 	struct sof_ipc_ext_data_hdr *ext_hdr;
 	void *ext_data;
@@ -146,7 +146,6 @@ int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset)
 	kfree(ext_data);
 	return ret;
 }
-EXPORT_SYMBOL(snd_sof_fw_parse_ext_data);
 
 static int ext_man_get_fw_version(struct snd_sof_dev *sdev,
 				  const struct sof_ext_man_elem_header *hdr)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index b98fc9bf660b..f5a505a8ce15 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -492,7 +492,6 @@ int snd_sof_run_firmware(struct snd_sof_dev *sdev);
 int snd_sof_parse_module_memcpy(struct snd_sof_dev *sdev,
 				struct snd_sof_mod_hdr *module);
 void snd_sof_fw_unload(struct snd_sof_dev *sdev);
-int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset);
 
 /*
  * IPC low level APIs.
-- 
2.33.0

[PATCH 07/12] ASoC: SOF: core: Do not use 'bar' as parameter for block_read/write

[Peter Ujfalusi]

The use of bar in the core poses limits on the portability of the code
to other, non iomapped platforms.
To make the API more generic, remove the use of 'bar' as parameter
for the block copy API.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/ipc.c      | 18 +++++++-----
 sound/soc/sof/loader.c   | 62 ++++++++++++++++++----------------------
 sound/soc/sof/ops.h      | 14 +++++----
 sound/soc/sof/sof-priv.h | 20 ++++++-------
 sound/soc/sof/utils.c    | 28 +++++++++++++-----
 5 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index 9ca3681d266d..18e0bfc1d8a9 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -709,15 +709,19 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 		send_bytes = sizeof(struct sof_ipc_ctrl_value_chan) *
 		cdata->num_elems;
 		if (send)
-			snd_sof_dsp_block_write(sdev, sdev->mmio_bar,
-						scontrol->readback_offset,
-						cdata->chanv, send_bytes);
+			err = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_IRAM,
+						      scontrol->readback_offset,
+						      cdata->chanv, send_bytes);
 
 		else
-			snd_sof_dsp_block_read(sdev, sdev->mmio_bar,
-					       scontrol->readback_offset,
-					       cdata->chanv, send_bytes);
-		return 0;
+			err = snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_IRAM,
+						     scontrol->readback_offset,
+						     cdata->chanv, send_bytes);
+
+		if (err)
+			dev_err_once(sdev->dev, "error: %s TYPE_IRAM failed\n",
+				     send ? "write to" :  "read from");
+		return err;
 	}
 
 	cdata->rhdr.hdr.cmd = SOF_IPC_GLB_COMP_MSG | ipc_cmd;
diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c
index 6d83ff11effc..16d36249c181 100644
--- a/sound/soc/sof/loader.c
+++ b/sound/soc/sof/loader.c
@@ -86,7 +86,7 @@ static int get_cc_info(struct snd_sof_dev *sdev,
 }
 
 /* parse the extended FW boot data structures from FW boot message */
-static int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offset)
+static int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 offset)
 {
 	struct sof_ipc_ext_data_hdr *ext_hdr;
 	void *ext_data;
@@ -97,15 +97,16 @@ static int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offs
 		return -ENOMEM;
 
 	/* get first header */
-	snd_sof_dsp_block_read(sdev, bar, offset, ext_data,
+	snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM, offset, ext_data,
 			       sizeof(*ext_hdr));
 	ext_hdr = ext_data;
 
 	while (ext_hdr->hdr.cmd == SOF_IPC_FW_READY) {
 		/* read in ext structure */
-		snd_sof_dsp_block_read(sdev, bar, offset + sizeof(*ext_hdr),
-				   (void *)((u8 *)ext_data + sizeof(*ext_hdr)),
-				   ext_hdr->hdr.size - sizeof(*ext_hdr));
+		snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM,
+				       offset + sizeof(*ext_hdr),
+				       (void *)((u8 *)ext_data + sizeof(*ext_hdr)),
+				       ext_hdr->hdr.size - sizeof(*ext_hdr));
 
 		dev_dbg(sdev->dev, "found ext header type %d size 0x%x\n",
 			ext_hdr->type, ext_hdr->hdr.size);
@@ -138,7 +139,7 @@ static int snd_sof_fw_parse_ext_data(struct snd_sof_dev *sdev, u32 bar, u32 offs
 
 		/* move to next header */
 		offset += ext_hdr->hdr.size;
-		snd_sof_dsp_block_read(sdev, bar, offset, ext_data,
+		snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM, offset, ext_data,
 				       sizeof(*ext_hdr));
 		ext_hdr = ext_data;
 	}
@@ -361,6 +362,7 @@ static int snd_sof_fw_ext_man_parse(struct snd_sof_dev *sdev,
  */
 static void sof_get_windows(struct snd_sof_dev *sdev)
 {
+	int bar = snd_sof_dsp_get_bar_index(sdev, SOF_FW_BLK_TYPE_SRAM);
 	struct sof_ipc_window_elem *elem;
 	u32 outbox_offset = 0;
 	u32 stream_offset = 0;
@@ -371,7 +373,6 @@ static void sof_get_windows(struct snd_sof_dev *sdev)
 	u32 debug_size = 0;
 	u32 debug_offset = 0;
 	int window_offset;
-	int bar;
 	int i;
 
 	if (!sdev->info_window) {
@@ -379,12 +380,6 @@ static void sof_get_windows(struct snd_sof_dev *sdev)
 		return;
 	}
 
-	bar = snd_sof_dsp_get_bar_index(sdev, SOF_FW_BLK_TYPE_SRAM);
-	if (bar < 0) {
-		dev_err(sdev->dev, "error: have no bar mapping\n");
-		return;
-	}
-
 	for (i = 0; i < sdev->info_window->num_windows; i++) {
 		elem = &sdev->info_window->window[i];
 
@@ -496,7 +491,6 @@ int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
 {
 	struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
 	int offset;
-	int bar;
 	int ret;
 
 	/* mailbox must be on 4k boundary */
@@ -506,12 +500,6 @@ int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
 		return offset;
 	}
 
-	bar = snd_sof_dsp_get_bar_index(sdev, SOF_FW_BLK_TYPE_SRAM);
-	if (bar < 0) {
-		dev_err(sdev->dev, "error: have no bar mapping\n");
-		return -EINVAL;
-	}
-
 	dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",
 		msg_id, offset);
 
@@ -519,8 +507,17 @@ int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
 	if (!sdev->first_boot)
 		return 0;
 
-	/* copy data from the DSP FW ready offset */
-	snd_sof_dsp_block_read(sdev, bar, offset, fw_ready, sizeof(*fw_ready));
+	/*
+	 * copy data from the DSP FW ready offset
+	 * Subsequent error handling is not needed for BLK_TYPE_SRAM
+	 */
+	ret = snd_sof_dsp_block_read(sdev, SOF_FW_BLK_TYPE_SRAM, offset, fw_ready,
+				     sizeof(*fw_ready));
+	if (ret) {
+		dev_err(sdev->dev,
+			"error: unable to read fw_ready, read from TYPE_SRAM failed\n");
+		return ret;
+	}
 
 	/* make sure ABI version is compatible */
 	ret = snd_sof_ipc_valid(sdev);
@@ -528,8 +525,7 @@ int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
 		return ret;
 
 	/* now check for extended data */
-	snd_sof_fw_parse_ext_data(sdev, bar, offset +
-				  sizeof(struct sof_ipc_fw_ready));
+	snd_sof_fw_parse_ext_data(sdev, offset + sizeof(struct sof_ipc_fw_ready));
 
 	sof_get_windows(sdev);
 
@@ -542,7 +538,7 @@ int snd_sof_parse_module_memcpy(struct snd_sof_dev *sdev,
 				struct snd_sof_mod_hdr *module)
 {
 	struct snd_sof_blk_hdr *block;
-	int count, bar;
+	int count, ret;
 	u32 offset;
 	size_t remaining;
 
@@ -579,13 +575,6 @@ int snd_sof_parse_module_memcpy(struct snd_sof_dev *sdev,
 		case SOF_FW_BLK_TYPE_DRAM:
 		case SOF_FW_BLK_TYPE_SRAM:
 			offset = block->offset;
-			bar = snd_sof_dsp_get_bar_index(sdev, block->type);
-			if (bar < 0) {
-				dev_err(sdev->dev,
-					"error: no BAR mapping for block type 0x%x\n",
-					block->type);
-				return bar;
-			}
 			break;
 		default:
 			dev_err(sdev->dev, "error: bad type 0x%x for block 0x%x\n",
@@ -603,8 +592,13 @@ int snd_sof_parse_module_memcpy(struct snd_sof_dev *sdev,
 				block->size);
 			return -EINVAL;
 		}
-		snd_sof_dsp_block_write(sdev, bar, offset,
-					block + 1, block->size);
+		ret = snd_sof_dsp_block_write(sdev, block->type, offset,
+					      block + 1, block->size);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: write to block type 0x%x failed\n",
+				block->type);
+			return ret;
+		}
 
 		if (remaining < block->size) {
 			dev_err(sdev->dev, "error: not enough data remaining\n");
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index 4a5d6e497f05..8d7fb7fd72bb 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -297,16 +297,18 @@ static inline u64 snd_sof_dsp_read64(struct snd_sof_dev *sdev, u32 bar,
 }
 
 /* block IO */
-static inline void snd_sof_dsp_block_read(struct snd_sof_dev *sdev, u32 bar,
-					  u32 offset, void *dest, size_t bytes)
+static inline int snd_sof_dsp_block_read(struct snd_sof_dev *sdev,
+					 enum snd_sof_fw_blk_type blk_type,
+					 u32 offset, void *dest, size_t bytes)
 {
-	sof_ops(sdev)->block_read(sdev, bar, offset, dest, bytes);
+	return sof_ops(sdev)->block_read(sdev, blk_type, offset, dest, bytes);
 }
 
-static inline void snd_sof_dsp_block_write(struct snd_sof_dev *sdev, u32 bar,
-					   u32 offset, void *src, size_t bytes)
+static inline int snd_sof_dsp_block_write(struct snd_sof_dev *sdev,
+					  enum snd_sof_fw_blk_type blk_type,
+					  u32 offset, void *src, size_t bytes)
 {
-	sof_ops(sdev)->block_write(sdev, bar, offset, src, bytes);
+	return sof_ops(sdev)->block_write(sdev, blk_type, offset, src, bytes);
 }
 
 /* ipc */
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index f5a505a8ce15..f9a39141e752 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -127,12 +127,12 @@ struct snd_sof_dsp_ops {
 		      void __iomem *addr); /* optional */
 
 	/* memcpy IO */
-	void (*block_read)(struct snd_sof_dev *sof_dev, u32 bar,
-			   u32 offset, void *dest,
-			   size_t size); /* mandatory */
-	void (*block_write)(struct snd_sof_dev *sof_dev, u32 bar,
-			    u32 offset, void *src,
-			    size_t size); /* mandatory */
+	int (*block_read)(struct snd_sof_dev *sof_dev,
+			  enum snd_sof_fw_blk_type type, u32 offset,
+			  void *dest, size_t size); /* mandatory */
+	int (*block_write)(struct snd_sof_dev *sof_dev,
+			   enum snd_sof_fw_blk_type type, u32 offset,
+			   void *src, size_t size); /* mandatory */
 
 	/* doorbell */
 	irqreturn_t (*irq_handler)(int irq, void *context); /* optional */
@@ -569,10 +569,10 @@ void sof_mailbox_write(struct snd_sof_dev *sdev, u32 offset,
 		       void *message, size_t bytes);
 void sof_mailbox_read(struct snd_sof_dev *sdev, u32 offset,
 		      void *message, size_t bytes);
-void sof_block_write(struct snd_sof_dev *sdev, u32 bar, u32 offset, void *src,
-		     size_t size);
-void sof_block_read(struct snd_sof_dev *sdev, u32 bar, u32 offset, void *dest,
-		    size_t size);
+int sof_block_write(struct snd_sof_dev *sdev, enum snd_sof_fw_blk_type blk_type,
+		    u32 offset, void *src, size_t size);
+int sof_block_read(struct snd_sof_dev *sdev, enum snd_sof_fw_blk_type blk_type,
+		   u32 offset, void *dest, size_t size);
 
 int sof_fw_ready(struct snd_sof_dev *sdev, u32 msg_id);
 
diff --git a/sound/soc/sof/utils.c b/sound/soc/sof/utils.c
index 5539d3afbe8f..66fa6602fb67 100644
--- a/sound/soc/sof/utils.c
+++ b/sound/soc/sof/utils.c
@@ -14,6 +14,7 @@
 #include <sound/soc.h>
 #include <sound/sof.h>
 #include "sof-priv.h"
+#include "ops.h"
 
 /*
  * Register IO
@@ -72,15 +73,21 @@ EXPORT_SYMBOL(sof_mailbox_read);
  * Memory copy.
  */
 
-void sof_block_write(struct snd_sof_dev *sdev, u32 bar, u32 offset, void *src,
-		     size_t size)
+int sof_block_write(struct snd_sof_dev *sdev, enum snd_sof_fw_blk_type blk_type,
+		    u32 offset, void *src, size_t size)
 {
-	void __iomem *dest = sdev->bar[bar] + offset;
+	int bar = snd_sof_dsp_get_bar_index(sdev, blk_type);
 	const u8 *src_byte = src;
+	void __iomem *dest;
 	u32 affected_mask;
 	u32 tmp;
 	int m, n;
 
+	if (bar < 0)
+		return bar;
+
+	dest = sdev->bar[bar] + offset;
+
 	m = size / 4;
 	n = size % 4;
 
@@ -100,15 +107,22 @@ void sof_block_write(struct snd_sof_dev *sdev, u32 bar, u32 offset, void *src,
 		tmp |= *(u32 *)(src_byte + m * 4) & affected_mask;
 		iowrite32(tmp, dest + m * 4);
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL(sof_block_write);
 
-void sof_block_read(struct snd_sof_dev *sdev, u32 bar, u32 offset, void *dest,
-		    size_t size)
+int sof_block_read(struct snd_sof_dev *sdev, enum snd_sof_fw_blk_type blk_type,
+		   u32 offset, void *dest, size_t size)
 {
-	void __iomem *src = sdev->bar[bar] + offset;
+	int bar = snd_sof_dsp_get_bar_index(sdev, blk_type);
+
+	if (bar < 0)
+		return bar;
+
+	memcpy_fromio(dest, sdev->bar[bar] + offset, size);
 
-	memcpy_fromio(dest, src, size);
+	return 0;
 }
 EXPORT_SYMBOL(sof_block_read);
 
-- 
2.33.0

[PATCH 08/12] ASoC: SOF: debug: Add generic API and ops for DSP regions

[Peter Ujfalusi]

Add new debugfs_add_region_item along with a generic wrapper
snd_sof_debugfs_add_region_item() to abstract away the DSP regions related
debugfs support.

At the same commit add iomem based generic implementation for the new ops

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/debug.c    | 15 +++++++++++++++
 sound/soc/sof/ops.h      | 11 +++++++++++
 sound/soc/sof/sof-priv.h | 27 +++++++++++++++++----------
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index a51a928ea40a..c536ea71630f 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -588,6 +588,21 @@ int snd_sof_debugfs_io_item(struct snd_sof_dev *sdev,
 }
 EXPORT_SYMBOL_GPL(snd_sof_debugfs_io_item);
 
+int snd_sof_debugfs_add_region_item_iomem(struct snd_sof_dev *sdev,
+					  enum snd_sof_fw_blk_type blk_type, u32 offset,
+					  size_t size, const char *name,
+					  enum sof_debugfs_access_type access_type)
+{
+	int bar = snd_sof_dsp_get_bar_index(sdev, blk_type);
+
+	if (bar < 0)
+		return bar;
+
+	return snd_sof_debugfs_io_item(sdev, sdev->bar[bar] + offset, size, name,
+				       access_type);
+}
+EXPORT_SYMBOL_GPL(snd_sof_debugfs_add_region_item_iomem);
+
 /* create FS entry for debug files to expose kernel memory */
 int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev,
 			     void *base, size_t size,
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index 8d7fb7fd72bb..2271bd0b25e3 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -253,6 +253,17 @@ static inline void snd_sof_ipc_dump(struct snd_sof_dev *sdev)
 		sof_ops(sdev)->ipc_dump(sdev);
 }
 
+static inline int snd_sof_debugfs_add_region_item(struct snd_sof_dev *sdev,
+		enum snd_sof_fw_blk_type blk_type, u32 offset, size_t size,
+		const char *name, enum sof_debugfs_access_type access_type)
+{
+	if (sof_ops(sdev) && sof_ops(sdev)->debugfs_add_region_item)
+		return sof_ops(sdev)->debugfs_add_region_item(sdev, blk_type, offset,
+							      size, name, access_type);
+
+	return 0;
+}
+
 /* register IO */
 static inline void snd_sof_dsp_write(struct snd_sof_dev *sdev, u32 bar,
 				     u32 offset, u32 value)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index f9a39141e752..4f1dcfe4a7ec 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -83,6 +83,16 @@ enum sof_system_suspend_state {
 	SOF_SUSPEND_S3,
 };
 
+enum sof_dfsentry_type {
+	SOF_DFSENTRY_TYPE_IOMEM = 0,
+	SOF_DFSENTRY_TYPE_BUF,
+};
+
+enum sof_debugfs_access_type {
+	SOF_DEBUGFS_ACCESS_ALWAYS = 0,
+	SOF_DEBUGFS_ACCESS_D0_ONLY,
+};
+
 struct snd_sof_dev;
 struct snd_sof_ipc_msg;
 struct snd_sof_ipc;
@@ -237,6 +247,10 @@ struct snd_sof_dsp_ops {
 	void (*dbg_dump)(struct snd_sof_dev *sof_dev,
 			 u32 flags); /* optional */
 	void (*ipc_dump)(struct snd_sof_dev *sof_dev); /* optional */
+	int (*debugfs_add_region_item)(struct snd_sof_dev *sdev,
+				       enum snd_sof_fw_blk_type blk_type, u32 offset,
+				       size_t size, const char *name,
+				       enum sof_debugfs_access_type access_type); /* optional */
 
 	/* host DMA trace initialization */
 	int (*trace_init)(struct snd_sof_dev *sdev,
@@ -286,16 +300,6 @@ struct sof_ops_table {
 	const struct snd_sof_dsp_ops *ops;
 };
 
-enum sof_dfsentry_type {
-	SOF_DFSENTRY_TYPE_IOMEM = 0,
-	SOF_DFSENTRY_TYPE_BUF,
-};
-
-enum sof_debugfs_access_type {
-	SOF_DEBUGFS_ACCESS_ALWAYS = 0,
-	SOF_DEBUGFS_ACCESS_D0_ONLY,
-};
-
 /* FS entry for debug files that can expose DSP memories, registers */
 struct snd_sof_dfsentry {
 	size_t size;
@@ -535,6 +539,9 @@ void snd_sof_get_status(struct snd_sof_dev *sdev, u32 panic_code,
 int snd_sof_init_trace_ipc(struct snd_sof_dev *sdev);
 void snd_sof_handle_fw_exception(struct snd_sof_dev *sdev);
 int snd_sof_dbg_memory_info_init(struct snd_sof_dev *sdev);
+int snd_sof_debugfs_add_region_item_iomem(struct snd_sof_dev *sdev,
+		enum snd_sof_fw_blk_type blk_type, u32 offset, size_t size,
+		const char *name, enum sof_debugfs_access_type access_type);
 
 /*
  * Platform specific ops.
-- 
2.33.0

[PATCH 09/12] ASoC: SOF: imx: Provide debugfs_add_region_item ops for core

[Peter Ujfalusi]

Set the generic iomem callback for debugfs_add_region_item to avoid
regression when the core switches to use the generic interface for
the regions.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/imx/imx8.c  | 2 ++
 sound/soc/sof/imx/imx8m.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
index 150f1bc3fb95..4423e0b63d7b 100644
--- a/sound/soc/sof/imx/imx8.c
+++ b/sound/soc/sof/imx/imx8.c
@@ -443,6 +443,7 @@ struct snd_sof_dsp_ops sof_imx8_ops = {
 
 	/* Debug information */
 	.dbg_dump = imx8_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* Firmware ops */
 	.arch_ops = &sof_xtensa_arch_ops,
@@ -489,6 +490,7 @@ struct snd_sof_dsp_ops sof_imx8x_ops = {
 
 	/* Debug information */
 	.dbg_dump = imx8_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* Firmware ops */
 	.arch_ops = &sof_xtensa_arch_ops,
diff --git a/sound/soc/sof/imx/imx8m.c b/sound/soc/sof/imx/imx8m.c
index be5ab9942fc8..b9f8a9e8cc3a 100644
--- a/sound/soc/sof/imx/imx8m.c
+++ b/sound/soc/sof/imx/imx8m.c
@@ -295,6 +295,7 @@ struct snd_sof_dsp_ops sof_imx8m_ops = {
 
 	/* Debug information */
 	.dbg_dump = imx8_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* Firmware ops */
 	.arch_ops = &sof_xtensa_arch_ops,
-- 
2.33.0

[PATCH 10/12] ASoC: SOF: Intel: Provide debugfs_add_region_item ops for core

[Peter Ujfalusi]

Set the generic iomem callback for debugfs_add_region_item to avoid
regression when the core switches to use the generic interface for
the regions.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/intel/apl.c     | 1 +
 sound/soc/sof/intel/bdw.c     | 1 +
 sound/soc/sof/intel/byt.c     | 2 ++
 sound/soc/sof/intel/cnl.c     | 1 +
 sound/soc/sof/intel/icl.c     | 1 +
 sound/soc/sof/intel/pci-tng.c | 1 +
 sound/soc/sof/intel/tgl.c     | 1 +
 7 files changed, 8 insertions(+)

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index c7ed2b3d6abc..0da6f3528269 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -65,6 +65,7 @@ const struct snd_sof_dsp_ops sof_apl_ops = {
 	.debug_map_count	= ARRAY_SIZE(apl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
 	.ipc_dump	= hda_ipc_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
diff --git a/sound/soc/sof/intel/bdw.c b/sound/soc/sof/intel/bdw.c
index 37fa4f976a11..a8063e9b3e00 100644
--- a/sound/soc/sof/intel/bdw.c
+++ b/sound/soc/sof/intel/bdw.c
@@ -635,6 +635,7 @@ static const struct snd_sof_dsp_ops sof_bdw_ops = {
 	.debug_map  = bdw_debugfs,
 	.debug_map_count    = ARRAY_SIZE(bdw_debugfs),
 	.dbg_dump   = bdw_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= intel_pcm_open,
diff --git a/sound/soc/sof/intel/byt.c b/sound/soc/sof/intel/byt.c
index 8edaf6fdd218..d4e86f847ae6 100644
--- a/sound/soc/sof/intel/byt.c
+++ b/sound/soc/sof/intel/byt.c
@@ -249,6 +249,7 @@ static const struct snd_sof_dsp_ops sof_byt_ops = {
 	.debug_map	= byt_debugfs,
 	.debug_map_count	= ARRAY_SIZE(byt_debugfs),
 	.dbg_dump	= atom_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= intel_pcm_open,
@@ -326,6 +327,7 @@ static const struct snd_sof_dsp_ops sof_cht_ops = {
 	.debug_map	= cht_debugfs,
 	.debug_map_count	= ARRAY_SIZE(cht_debugfs),
 	.dbg_dump	= atom_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= intel_pcm_open,
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index e115e12a856f..eeb95cbb77a1 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -270,6 +270,7 @@ const struct snd_sof_dsp_ops sof_cnl_ops = {
 	.debug_map_count	= ARRAY_SIZE(cnl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
 	.ipc_dump	= cnl_ipc_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
diff --git a/sound/soc/sof/intel/icl.c b/sound/soc/sof/intel/icl.c
index ee095b8f2d01..f5e370c13fed 100644
--- a/sound/soc/sof/intel/icl.c
+++ b/sound/soc/sof/intel/icl.c
@@ -64,6 +64,7 @@ const struct snd_sof_dsp_ops sof_icl_ops = {
 	.debug_map_count	= ARRAY_SIZE(icl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
 	.ipc_dump	= cnl_ipc_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
diff --git a/sound/soc/sof/intel/pci-tng.c b/sound/soc/sof/intel/pci-tng.c
index 4bded668b672..c7a64a11e4ae 100644
--- a/sound/soc/sof/intel/pci-tng.c
+++ b/sound/soc/sof/intel/pci-tng.c
@@ -165,6 +165,7 @@ const struct snd_sof_dsp_ops sof_tng_ops = {
 	.debug_map	= tng_debugfs,
 	.debug_map_count	= ARRAY_SIZE(tng_debugfs),
 	.dbg_dump	= atom_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= intel_pcm_open,
diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c
index 199d41a7dc9b..e91ea80f766f 100644
--- a/sound/soc/sof/intel/tgl.c
+++ b/sound/soc/sof/intel/tgl.c
@@ -60,6 +60,7 @@ const struct snd_sof_dsp_ops sof_tgl_ops = {
 	.debug_map_count	= ARRAY_SIZE(tgl_dsp_debugfs),
 	.dbg_dump	= hda_dsp_dump,
 	.ipc_dump	= cnl_ipc_dump,
+	.debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem,
 
 	/* stream callbacks */
 	.pcm_open	= hda_dsp_pcm_open,
-- 
2.33.0

[PATCH 11/12] ASoC: SOF: loader: Use the generic ops for region debugfs handling

[Peter Ujfalusi]

Do not access the sdev->bar[] directly to make the code generic, use the
new generic ops for handing the regions for debugfs.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/loader.c | 68 +++++++++++++++++-------------------------
 1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/sound/soc/sof/loader.c b/sound/soc/sof/loader.c
index 16d36249c181..1d6fdf8270f7 100644
--- a/sound/soc/sof/loader.c
+++ b/sound/soc/sof/loader.c
@@ -362,7 +362,6 @@ static int snd_sof_fw_ext_man_parse(struct snd_sof_dev *sdev,
  */
 static void sof_get_windows(struct snd_sof_dev *sdev)
 {
-	int bar = snd_sof_dsp_get_bar_index(sdev, SOF_FW_BLK_TYPE_SRAM);
 	struct sof_ipc_window_elem *elem;
 	u32 outbox_offset = 0;
 	u32 stream_offset = 0;
@@ -394,64 +393,53 @@ static void sof_get_windows(struct snd_sof_dev *sdev)
 		case SOF_IPC_REGION_UPBOX:
 			inbox_offset = window_offset + elem->offset;
 			inbox_size = elem->size;
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						inbox_offset,
-						elem->size, "inbox",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							inbox_offset,
+							elem->size, "inbox",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_DOWNBOX:
 			outbox_offset = window_offset + elem->offset;
 			outbox_size = elem->size;
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						outbox_offset,
-						elem->size, "outbox",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							outbox_offset,
+							elem->size, "outbox",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_TRACE:
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						window_offset +
-						elem->offset,
-						elem->size, "etrace",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							window_offset + elem->offset,
+							elem->size, "etrace",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_DEBUG:
 			debug_offset = window_offset + elem->offset;
 			debug_size = elem->size;
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						window_offset +
-						elem->offset,
-						elem->size, "debug",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							window_offset + elem->offset,
+							elem->size, "debug",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_STREAM:
 			stream_offset = window_offset + elem->offset;
 			stream_size = elem->size;
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						stream_offset,
-						elem->size, "stream",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							stream_offset,
+							elem->size, "stream",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_REGS:
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						window_offset +
-						elem->offset,
-						elem->size, "regs",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							window_offset + elem->offset,
+							elem->size, "regs",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		case SOF_IPC_REGION_EXCEPTION:
 			sdev->dsp_oops_offset = window_offset + elem->offset;
-			snd_sof_debugfs_io_item(sdev,
-						sdev->bar[bar] +
-						window_offset +
-						elem->offset,
-						elem->size, "exception",
-						SOF_DEBUGFS_ACCESS_D0_ONLY);
+			snd_sof_debugfs_add_region_item(sdev, SOF_FW_BLK_TYPE_SRAM,
+							window_offset + elem->offset,
+							elem->size, "exception",
+							SOF_DEBUGFS_ACCESS_D0_ONLY);
 			break;
 		default:
 			dev_err(sdev->dev, "error: get illegal window info\n");
-- 
2.33.0

[PATCH 12/12] ASoC: SOF: debug: No need to export the snd_sof_debugfs_io_item()

[Peter Ujfalusi]

The snd_sof_debugfs_io_item() only used within debug.c, no need to export
it.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Daniel Baluta <daniel.baluta@gmail.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
---
 sound/soc/sof/debug.c    | 9 ++++-----
 sound/soc/sof/sof-priv.h | 4 ----
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c
index c536ea71630f..af92baacd23e 100644
--- a/sound/soc/sof/debug.c
+++ b/sound/soc/sof/debug.c
@@ -546,10 +546,10 @@ static const struct file_operations sof_dfs_fops = {
 };
 
 /* create FS entry for debug files that can expose DSP memories, registers */
-int snd_sof_debugfs_io_item(struct snd_sof_dev *sdev,
-			    void __iomem *base, size_t size,
-			    const char *name,
-			    enum sof_debugfs_access_type access_type)
+static int snd_sof_debugfs_io_item(struct snd_sof_dev *sdev,
+				   void __iomem *base, size_t size,
+				   const char *name,
+				   enum sof_debugfs_access_type access_type)
 {
 	struct snd_sof_dfsentry *dfse;
 
@@ -586,7 +586,6 @@ int snd_sof_debugfs_io_item(struct snd_sof_dev *sdev,
 
 	return 0;
 }
-EXPORT_SYMBOL_GPL(snd_sof_debugfs_io_item);
 
 int snd_sof_debugfs_add_region_item_iomem(struct snd_sof_dev *sdev,
 					  enum snd_sof_fw_blk_type blk_type, u32 offset,
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 4f1dcfe4a7ec..d391fff1e02c 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -522,10 +522,6 @@ void snd_sof_release_trace(struct snd_sof_dev *sdev);
 void snd_sof_free_trace(struct snd_sof_dev *sdev);
 int snd_sof_dbg_init(struct snd_sof_dev *sdev);
 void snd_sof_free_debug(struct snd_sof_dev *sdev);
-int snd_sof_debugfs_io_item(struct snd_sof_dev *sdev,
-			    void __iomem *base, size_t size,
-			    const char *name,
-			    enum sof_debugfs_access_type access_type);
 int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev,
 			     void *base, size_t size,
 			     const char *name, mode_t mode);
-- 
2.33.0

Re: [PATCH 00/12] ASoC: SOF: Abstract away the 'bar' usage (mostly)

[Mark Brown]

On Wed, 15 Sep 2021 15:21:04 +0300, Peter Ujfalusi wrote:
> The direct use of 'bar' as address regions in SOF can limit portability and
> makes assumption that all platforms are using memory mapped regions or BARs
> to communicate with the DSP.
> 
> The core API should not use 'bar' in it's parameter lists or make assumption
> that the platform needs to comply with this system integration.
> 
> [...]

Applied to

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

Thanks!

[01/12] ASoC: SOF: Intel: bdw: Set the mailbox offset directly in bdw_probe
        commit: 6375dbdbde67725220f2a07e428259c944d4c42d
[02/12] ASoC: SOF: ipc: Remove snd_sof_dsp_mailbox_init()
        commit: b295818346aa7140dac865054a6c5efe8d4ec3ae
[03/12] ASoC: SOF: imx: Do not initialize the snd_sof_dsp_ops.read64
        commit: 098a68f2c5735b2fa8051ffe854b94b6d5b0a6a8
[04/12] ASoC: SOF: imx: imx8: Bar index is only valid for IRAM and SRAM types
        commit: 10d93a98190aec2c3ff98d9472ab1bf0543aa02c
[05/12] ASoC: SOF: imx: imx8m: Bar index is only valid for IRAM and SRAM types
        commit: d9be4a88c3627c270bbe032b623dc43f3b764565
[06/12] ASoC: SOF: loader: No need to export snd_sof_fw_parse_ext_data()
        commit: 4ff134e2f90ee2816ca5a8069802ff5cb602a2f1
[07/12] ASoC: SOF: core: Do not use 'bar' as parameter for block_read/write
        commit: 4624bb2f03d3c153e00d21c1baa1da34cfc19afd
[08/12] ASoC: SOF: debug: Add generic API and ops for DSP regions
        commit: 07e833b473e417f13c5a62aa6f63dbbd3028d277
[09/12] ASoC: SOF: imx: Provide debugfs_add_region_item ops for core
        commit: ff2f99b078a839c973434bcc9c1094814a38ae76
[10/12] ASoC: SOF: Intel: Provide debugfs_add_region_item ops for core
        commit: fe509b34b745d2284c3026abae8aaf02413a0594
[11/12] ASoC: SOF: loader: Use the generic ops for region debugfs handling
        commit: 55dfc2a74d8e8d34d6f562a1e4173e711bbd916d
[12/12] ASoC: SOF: debug: No need to export the snd_sof_debugfs_io_item()
        commit: bde4f08cff47632f0a52e15a613365e26608d003

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