diff mbox series

[1/2] ASoC: SOF: imx: Add i.MX8ULP HW support

Message ID 1658208367-24376-1-git-send-email-shengjiu.wang@nxp.com (mailing list archive)
State New, archived
Headers show
Series [1/2] ASoC: SOF: imx: Add i.MX8ULP HW support | expand

Commit Message

Shengjiu Wang July 19, 2022, 5:26 a.m. UTC 
From: Zhang Peng <peng.zhang_8@nxp.com>

This adds skeleton support for the audio DSP hardware found on
NXP i.MX8ULP platform.

On i.MX8ULP resources (clocks, power, etc) are managed by the
System Integration Module in LPAV domain and XRDC which is handled
by arm trusted firmware.

Signed-off-by: Zhang Peng <peng.zhang_8@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/sof/imx/Kconfig   |   9 +
 sound/soc/sof/imx/Makefile  |   2 +
 sound/soc/sof/imx/imx8ulp.c | 541 ++++++++++++++++++++++++++++++++++++
 3 files changed, 552 insertions(+)
 create mode 100644 sound/soc/sof/imx/imx8ulp.c

Comments

kernel test robot July 19, 2022, 9:27 a.m. UTC | #1 
Hi Shengjiu,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on robh/for-next broonie-sound/for-next linus/master v5.19-rc7 next-20220718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shengjiu-Wang/ASoC-SOF-imx-Add-i-MX8ULP-HW-support/20220719-134348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: sh-allmodconfig
compiler: sh4-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7cd457928803203c18dc543f6f98c304fa601a30
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Shengjiu-Wang/ASoC-SOF-imx-Add-i-MX8ULP-HW-support/20220719-134348
        git checkout 7cd457928803203c18dc543f6f98c304fa601a30
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=sh SHELL=/bin/bash sound/soc/sof/imx/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   sound/soc/sof/imx/imx8ulp.c: In function 'imx8ulp_reset':
>> sound/soc/sof/imx/imx8ulp.c:186:9: error: implicit declaration of function 'arm_smccc_smc' [-Werror=implicit-function-declaration]
     186 |         arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource);
         |         ^~~~~~~~~~~~~
   sound/soc/sof/imx/imx8ulp.c: At top level:
>> sound/soc/sof/imx/imx8ulp.c:42:33: error: storage size of 'smc_resource' isn't known
      42 | struct arm_smccc_res            smc_resource;
         |                                 ^~~~~~~~~~~~
   cc1: some warnings being treated as errors


vim +/arm_smccc_smc +186 sound/soc/sof/imx/imx8ulp.c

    41	
  > 42	struct arm_smccc_res		smc_resource;
    43	
    44	static struct clk_bulk_data imx8ulp_dsp_clks[] = {
    45		{ .id = "core" },
    46		{ .id = "ipg" },
    47		{ .id = "ocram" },
    48		{ .id = "mu" },
    49	};
    50	
    51	struct imx8ulp_priv {
    52		struct device *dev;
    53		struct snd_sof_dev *sdev;
    54	
    55		/* DSP IPC handler */
    56		struct imx_dsp_ipc *dsp_ipc;
    57		struct platform_device *ipc_dev;
    58	
    59		struct regmap *regmap;
    60		struct imx_clocks *clks;
    61	};
    62	
    63	static void imx8ulp_sim_lpav_start(struct imx8ulp_priv *priv)
    64	{
    65		/* Controls the HiFi4 DSP Reset: 1 in reset, 0 out of reset */
    66		regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, 0);
    67		/* Reset HiFi4 DSP Debug logic: 1 reset, 0 not set */
    68		regmap_update_bits(priv->regmap, SYSCTRL0, DEBUG_LOGIC_BIT, 0);
    69		/* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
    70		regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, 0);
    71	}
    72	
    73	static void imx8ulp_get_reply(struct snd_sof_dev *sdev)
    74	{
    75		struct snd_sof_ipc_msg *msg = sdev->msg;
    76		struct sof_ipc_reply reply;
    77		int ret = 0;
    78	
    79		if (!msg) {
    80			dev_warn(sdev->dev, "unexpected ipc interrupt\n");
    81			return;
    82		}
    83	
    84		/* get reply */
    85		sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
    86	
    87		if (reply.error < 0) {
    88			memcpy(msg->reply_data, &reply, sizeof(reply));
    89			ret = reply.error;
    90		} else {
    91			/* reply has correct size? */
    92			if (reply.hdr.size != msg->reply_size) {
    93				dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
    94					msg->reply_size, reply.hdr.size);
    95				ret = -EINVAL;
    96			}
    97	
    98			/* read the message */
    99			if (msg->reply_size > 0)
   100				sof_mailbox_read(sdev, sdev->host_box.offset,
   101						 msg->reply_data, msg->reply_size);
   102		}
   103	
   104		msg->reply_error = ret;
   105	}
   106	
   107	static int imx8ulp_get_mailbox_offset(struct snd_sof_dev *sdev)
   108	{
   109		return MBOX_OFFSET;
   110	}
   111	
   112	static int imx8ulp_get_window_offset(struct snd_sof_dev *sdev, u32 id)
   113	{
   114		return MBOX_OFFSET;
   115	}
   116	
   117	static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
   118	{
   119		struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
   120		unsigned long flags;
   121	
   122		spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
   123	
   124		imx8ulp_get_reply(priv->sdev);
   125		snd_sof_ipc_reply(priv->sdev, 0);
   126		spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
   127	}
   128	
   129	static void imx8ulp_dsp_handle_request(struct imx_dsp_ipc *ipc)
   130	{
   131		struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
   132		u32 p; /* panic code */
   133	
   134		/* Read the message from the debug box. */
   135		sof_mailbox_read(priv->sdev, priv->sdev->debug_box.offset + 4, &p, sizeof(p));
   136	
   137		/* Check to see if the message is a panic code (0x0dead***) */
   138		if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC)
   139			snd_sof_dsp_panic(priv->sdev, p, true);
   140		else
   141			snd_sof_ipc_msgs_rx(priv->sdev);
   142	}
   143	
   144	static struct imx_dsp_ops dsp_ops = {
   145		.handle_reply		= imx8ulp_dsp_handle_reply,
   146		.handle_request		= imx8ulp_dsp_handle_request,
   147	};
   148	
   149	static int imx8ulp_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
   150	{
   151		struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
   152	
   153		sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
   154				  msg->msg_size);
   155		imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
   156	
   157		return 0;
   158	}
   159	
   160	static int imx8ulp_run(struct snd_sof_dev *sdev)
   161	{
   162		struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
   163	
   164		imx8ulp_sim_lpav_start(priv);
   165	
   166		return 0;
   167	}
   168	
   169	static int imx8ulp_reset(struct snd_sof_dev *sdev)
   170	{
   171		struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
   172	
   173		/* HiFi4 Platform Clock Enable: 1 enabled, 0 disabled */
   174		regmap_update_bits(priv->regmap, SYSCTRL0, PLAT_CLK_BIT, PLAT_CLK_BIT);
   175		/* HiFi4 PBCLK clock enable: 1 enabled, 0 disabled */
   176		regmap_update_bits(priv->regmap, SYSCTRL0, PB_CLK_BIT, PB_CLK_BIT);
   177		/* HiFi4 Clock Enable: 1 enabled, 0 disabled */
   178		regmap_update_bits(priv->regmap, SYSCTRL0, HIFI4_CLK_BIT, HIFI4_CLK_BIT);
   179	
   180		regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, RESET_BIT);
   181		usleep_range(1, 2);
   182		/* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
   183		regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
   184		usleep_range(1, 2);
   185	
 > 186		arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource);
   187	
   188		return 0;
   189	}
   190
Mark Brown July 19, 2022, 11:41 a.m. UTC | #2 
On Tue, Jul 19, 2022 at 01:26:06PM +0800, Shengjiu Wang wrote:

Not a thorough review, just a few nitpicks:

> +#define MBOX_SIZE		0x1000
> +
> +struct arm_smccc_res		smc_resource;

This should be static shouldn't it?

> +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> +{
> +	struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
> +
> +	imx8ulp_get_reply(priv->sdev);
> +	snd_sof_ipc_reply(priv->sdev, 0);
> +	spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);

Minor nitpick but a blank line before the unlock to match the one after
the lock would be a bit easier to read.

> +	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
> +	usleep_range(1, 2);
> +
> +	arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource);

You need linux/arm-smccc.h for this (as 0day said).

> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to init reserved memory region %d\n", ret);
> +		goto exit_pdev_unregister;
> +	}
> +
> +	priv->clks->dsp_clks = imx8ulp_dsp_clks;
> +	priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks);
> +
> +	ret = imx8_parse_clocks(sdev, priv->clks);
> +	if (ret < 0)
> +		goto exit_pdev_unregister;
> +
> +	ret = imx8_enable_clocks(sdev, priv->clks);
> +	if (ret < 0)
> +		goto exit_pdev_unregister;

We're registering the platform device before we enable the clocks - is
that safe?

> +static int imx8ulp_remove(struct snd_sof_dev *sdev)
> +{
> +	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> +
> +	platform_device_unregister(priv->ipc_dev);
> +
> +	return 0;
> +}

Could we just use devm?  I'm not seeing an ordering issue but I might be
missing something.
Pierre-Louis Bossart July 19, 2022, 2:19 p.m. UTC | #3 
> +static void imx8ulp_get_reply(struct snd_sof_dev *sdev)
> +{
> +	struct snd_sof_ipc_msg *msg = sdev->msg;
> +	struct sof_ipc_reply reply;
> +	int ret = 0;
> +
> +	if (!msg) {
> +		dev_warn(sdev->dev, "unexpected ipc interrupt\n");
> +		return;
> +	}
> +
> +	/* get reply */
> +	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
> +
> +	if (reply.error < 0) {
> +		memcpy(msg->reply_data, &reply, sizeof(reply));
> +		ret = reply.error;
> +	} else {
> +		/* reply has correct size? */
> +		if (reply.hdr.size != msg->reply_size) {
> +			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
> +				msg->reply_size, reply.hdr.size);
> +			ret = -EINVAL;
> +		}
> +
> +		/* read the message */
> +		if (msg->reply_size > 0)
> +			sof_mailbox_read(sdev, sdev->host_box.offset,
> +					 msg->reply_data, msg->reply_size);
> +	}
> +
> +	msg->reply_error = ret;
> +}

Can you double-check if this helper is needed? It looks completely
generic and I vaguely remember that this get_reply() was moved to common
code, and it's no longer in the other existing iMX support files.

> +
> +static int imx8ulp_get_mailbox_offset(struct snd_sof_dev *sdev)
> +{
> +	return MBOX_OFFSET;
> +}
> +
> +static int imx8ulp_get_window_offset(struct snd_sof_dev *sdev, u32 id)
> +{
> +	return MBOX_OFFSET;
> +}
> +
> +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> +{
> +	struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
> +
> +	imx8ulp_get_reply(priv->sdev);
> +	snd_sof_ipc_reply(priv->sdev, 0);
> +	spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);

this is old stuff that wasn't updated, we now have an inline that
combines this:

static inline void snd_sof_ipc_process_reply(struct snd_sof_dev *sdev,
u32 msg_id)
{
	snd_sof_ipc_get_reply(sdev);
	snd_sof_ipc_reply(sdev, msg_id);
}

see the code in im8xm.c....

> +}
> +
> +static void imx8ulp_dsp_handle_request(struct imx_dsp_ipc *ipc)
> +{
> +	struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> +	u32 p; /* panic code */
> +
> +	/* Read the message from the debug box. */
> +	sof_mailbox_read(priv->sdev, priv->sdev->debug_box.offset + 4, &p, sizeof(p));
> +
> +	/* Check to see if the message is a panic code (0x0dead***) */
> +	if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC)
> +		snd_sof_dsp_panic(priv->sdev, p, true);
> +	else
> +		snd_sof_ipc_msgs_rx(priv->sdev);
> +}
> +
> +static struct imx_dsp_ops dsp_ops = {
> +	.handle_reply		= imx8ulp_dsp_handle_reply,
> +	.handle_request		= imx8ulp_dsp_handle_request,
> +};
> +
> +static int imx8ulp_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
> +{
> +	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> +
> +	sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
> +			  msg->msg_size);
> +	imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
> +
> +	return 0;
> +}
> +
> +static int imx8ulp_run(struct snd_sof_dev *sdev)
> +{
> +	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> +
> +	imx8ulp_sim_lpav_start(priv);
> +
> +	return 0;
> +}
> +
> +static int imx8ulp_reset(struct snd_sof_dev *sdev)
> +{
> +	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> +
> +	/* HiFi4 Platform Clock Enable: 1 enabled, 0 disabled */
> +	regmap_update_bits(priv->regmap, SYSCTRL0, PLAT_CLK_BIT, PLAT_CLK_BIT);
> +	/* HiFi4 PBCLK clock enable: 1 enabled, 0 disabled */
> +	regmap_update_bits(priv->regmap, SYSCTRL0, PB_CLK_BIT, PB_CLK_BIT);
> +	/* HiFi4 Clock Enable: 1 enabled, 0 disabled */
> +	regmap_update_bits(priv->regmap, SYSCTRL0, HIFI4_CLK_BIT, HIFI4_CLK_BIT);
> +
> +	regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, RESET_BIT);
> +	usleep_range(1, 2);
> +	/* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
> +	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
> +	usleep_range(1, 2);


adding newlines would help make the comments more readable, this is a
bit of an eyesore.

> +	arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource);
> +
> +	return 0;
> +}
> +
> +static int imx8ulp_probe(struct snd_sof_dev *sdev)
> +{
> +	struct platform_device *pdev =
> +		container_of(sdev->dev, struct platform_device, dev);
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *res_node;
> +	struct resource *mmio;
> +	struct imx8ulp_priv *priv;
> +	struct resource res;
> +	u32 base, size;
> +	int ret = 0;
> +
> +	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->clks = devm_kzalloc(&pdev->dev, sizeof(*priv->clks), GFP_KERNEL);
> +	if (!priv->clks)
> +		return -ENOMEM;
> +
> +	sdev->num_cores = 1;
> +	sdev->pdata->hw_pdata = priv;
> +	priv->dev = sdev->dev;
> +	priv->sdev = sdev;
> +
> +	/* System integration module(SIM) control dsp configurtion */

typo: configuration

> +	priv->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,dsp-ctrl");
> +	if (IS_ERR(priv->regmap))
> +		return PTR_ERR(priv->regmap);
> +
> +	priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
> +						      PLATFORM_DEVID_NONE,
> +						      pdev, sizeof(*pdev));
> +	if (IS_ERR(priv->ipc_dev))
> +		return PTR_ERR(priv->ipc_dev);
> +
> +	priv->dsp_ipc = dev_get_drvdata(&priv->ipc_dev->dev);
> +	if (!priv->dsp_ipc) {
> +		/* DSP IPC driver not probed yet, try later */
> +		ret = -EPROBE_DEFER;
> +		dev_err(sdev->dev, "Failed to get drvdata\n");
> +		goto exit_pdev_unregister;
> +	}
> +
> +	imx_dsp_set_data(priv->dsp_ipc, priv);
> +	priv->dsp_ipc->ops = &dsp_ops;
> +
> +	/* DSP base */
> +	mmio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (mmio) {
> +		base = mmio->start;
> +		size = resource_size(mmio);
> +	} else {
> +		dev_err(sdev->dev, "error: failed to get DSP base at idx 0\n");
> +		ret = -EINVAL;
> +		goto exit_pdev_unregister;
> +	}
> +
> +	sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev, base, size);
> +	if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) {
> +		dev_err(sdev->dev, "failed to ioremap base 0x%x size 0x%x\n",
> +			base, size);
> +		ret = -ENODEV;
> +		goto exit_pdev_unregister;
> +	}
> +	sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
> +
> +	res_node = of_parse_phandle(np, "memory-reserved", 0);
> +	if (!res_node) {
> +		dev_err(&pdev->dev, "failed to get memory region node\n");
> +		ret = -ENODEV;
> +		goto exit_pdev_unregister;
> +	}
> +
> +	ret = of_address_to_resource(res_node, 0, &res);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to get reserved region address\n");
> +		goto exit_pdev_unregister;
> +	}
> +
> +	sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start,
> +							  resource_size(&res));
> +	if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
> +		dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n",
> +			base, size);
> +		ret = -ENOMEM;
> +		goto exit_pdev_unregister;
> +	}
> +	sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
> +
> +	/* set default mailbox offset for FW ready message */
> +	sdev->dsp_box.offset = MBOX_OFFSET;
> +
> +	ret = of_reserved_mem_device_init(sdev->dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to init reserved memory region %d\n", ret);
> +		goto exit_pdev_unregister;
> +	}
> +
> +	priv->clks->dsp_clks = imx8ulp_dsp_clks;
> +	priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks);
> +
> +	ret = imx8_parse_clocks(sdev, priv->clks);
> +	if (ret < 0)
> +		goto exit_pdev_unregister;
> +
> +	ret = imx8_enable_clocks(sdev, priv->clks);
> +	if (ret < 0)
> +		goto exit_pdev_unregister;
> +
> +	return 0;
> +
> +exit_pdev_unregister:
> +	platform_device_unregister(priv->ipc_dev);
> +
> +	return ret;
> +}
> +

> +static int imx8ulp_suspend(struct snd_sof_dev *sdev)
> +{
> +	int i;
> +	struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata;
> +
> +	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
> +
> +	for (i = 0; i < DSP_MU_CHAN_NUM; i++)
> +		imx_dsp_free_channel(priv->dsp_ipc, i);
> +
> +	imx8_disable_clocks(sdev, priv->clks);
> +
> +	return 0;
> +}
> +
> +static int imx8ulp_resume(struct snd_sof_dev *sdev)
> +{
> +	struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata;
> +	int i;
> +
> +	imx8_enable_clocks(sdev, priv->clks);
> +
> +	for (i = 0; i < DSP_MU_CHAN_NUM; i++)
> +		imx_dsp_request_channel(priv->dsp_ipc, i);
> +
> +	return 0;

is the assymetry between suspend and resume intentional? You are missing
the update_bit for EXECUTE_BIT?

> +}
> +
Shengjiu Wang July 20, 2022, 6:12 a.m. UTC | #4 
On Tue, Jul 19, 2022 at 7:41 PM Mark Brown <broonie@kernel.org> wrote:

> On Tue, Jul 19, 2022 at 01:26:06PM +0800, Shengjiu Wang wrote:
>
> Not a thorough review, just a few nitpicks:
>

Thanks.

>
> > +#define MBOX_SIZE            0x1000
> > +
> > +struct arm_smccc_res         smc_resource;
>
> This should be static shouldn't it?
>

I will move it to function as a local variable.

>
> > +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> > +{
> > +     struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
> > +
> > +     imx8ulp_get_reply(priv->sdev);
> > +     snd_sof_ipc_reply(priv->sdev, 0);
> > +     spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
>
> Minor nitpick but a blank line before the unlock to match the one after
> the lock would be a bit easier to read.
>

ok, will update

>
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT,
> EXECUTE_BIT);
> > +     usleep_range(1, 2);
> > +
> > +     arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0,
> &smc_resource);
>
> You need linux/arm-smccc.h for this (as 0day said).
>
Yes, right.


>
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "failed to init reserved memory region
> %d\n", ret);
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     priv->clks->dsp_clks = imx8ulp_dsp_clks;
> > +     priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks);
> > +
> > +     ret = imx8_parse_clocks(sdev, priv->clks);
> > +     if (ret < 0)
> > +             goto exit_pdev_unregister;
> > +
> > +     ret = imx8_enable_clocks(sdev, priv->clks);
> > +     if (ret < 0)
> > +             goto exit_pdev_unregister;
>
> We're registering the platform device before we enable the clocks - is
> that safe?
>

Yes, it is safe.

>
> > +static int imx8ulp_remove(struct snd_sof_dev *sdev)
> > +{
> > +     struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> > +
> > +     platform_device_unregister(priv->ipc_dev);
> > +
> > +     return 0;
> > +}
>
> Could we just use devm?  I'm not seeing an ordering issue but I might be
> missing something.
>

Which devm do you mean?  There seems no
devm_platform_device_register_data().

best regards
wang shengjiu
Shengjiu Wang July 20, 2022, 8:57 a.m. UTC | #5 
On Tue, Jul 19, 2022 at 10:19 PM Pierre-Louis Bossart <
pierre-louis.bossart@linux.intel.com> wrote:

>
> > +static void imx8ulp_get_reply(struct snd_sof_dev *sdev)
> > +{
> > +     struct snd_sof_ipc_msg *msg = sdev->msg;
> > +     struct sof_ipc_reply reply;
> > +     int ret = 0;
> > +
> > +     if (!msg) {
> > +             dev_warn(sdev->dev, "unexpected ipc interrupt\n");
> > +             return;
> > +     }
> > +
> > +     /* get reply */
> > +     sof_mailbox_read(sdev, sdev->host_box.offset, &reply,
> sizeof(reply));
> > +
> > +     if (reply.error < 0) {
> > +             memcpy(msg->reply_data, &reply, sizeof(reply));
> > +             ret = reply.error;
> > +     } else {
> > +             /* reply has correct size? */
> > +             if (reply.hdr.size != msg->reply_size) {
> > +                     dev_err(sdev->dev, "error: reply expected %zu got
> %u bytes\n",
> > +                             msg->reply_size, reply.hdr.size);
> > +                     ret = -EINVAL;
> > +             }
> > +
> > +             /* read the message */
> > +             if (msg->reply_size > 0)
> > +                     sof_mailbox_read(sdev, sdev->host_box.offset,
> > +                                      msg->reply_data, msg->reply_size);
> > +     }
> > +
> > +     msg->reply_error = ret;
> > +}
>
> Can you double-check if this helper is needed? It looks completely
> generic and I vaguely remember that this get_reply() was moved to common
> code, and it's no longer in the other existing iMX support files.
>

ok,  I will use the latest interface.


>
> > +
> > +static int imx8ulp_get_mailbox_offset(struct snd_sof_dev *sdev)
> > +{
> > +     return MBOX_OFFSET;
> > +}
> > +
> > +static int imx8ulp_get_window_offset(struct snd_sof_dev *sdev, u32 id)
> > +{
> > +     return MBOX_OFFSET;
> > +}
> > +
> > +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> > +{
> > +     struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> > +     unsigned long flags;
> > +
> > +     spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
> > +
> > +     imx8ulp_get_reply(priv->sdev);
> > +     snd_sof_ipc_reply(priv->sdev, 0);
> > +     spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
>
> this is old stuff that wasn't updated, we now have an inline that
> combines this:
>
> static inline void snd_sof_ipc_process_reply(struct snd_sof_dev *sdev,
> u32 msg_id)
> {
>         snd_sof_ipc_get_reply(sdev);
>         snd_sof_ipc_reply(sdev, msg_id);
> }
>
> see the code in im8xm.c....
>

ok,  I will use the latest interface.

>
> > +}
> > +
> > +static void imx8ulp_dsp_handle_request(struct imx_dsp_ipc *ipc)
> > +{
> > +     struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
> > +     u32 p; /* panic code */
> > +
> > +     /* Read the message from the debug box. */
> > +     sof_mailbox_read(priv->sdev, priv->sdev->debug_box.offset + 4, &p,
> sizeof(p));
> > +
> > +     /* Check to see if the message is a panic code (0x0dead***) */
> > +     if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC)
> > +             snd_sof_dsp_panic(priv->sdev, p, true);
> > +     else
> > +             snd_sof_ipc_msgs_rx(priv->sdev);
> > +}
> > +
> > +static struct imx_dsp_ops dsp_ops = {
> > +     .handle_reply           = imx8ulp_dsp_handle_reply,
> > +     .handle_request         = imx8ulp_dsp_handle_request,
> > +};
> > +
> > +static int imx8ulp_send_msg(struct snd_sof_dev *sdev, struct
> snd_sof_ipc_msg *msg)
> > +{
> > +     struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> > +
> > +     sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
> > +                       msg->msg_size);
> > +     imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
> > +
> > +     return 0;
> > +}
> > +
> > +static int imx8ulp_run(struct snd_sof_dev *sdev)
> > +{
> > +     struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> > +
> > +     imx8ulp_sim_lpav_start(priv);
> > +
> > +     return 0;
> > +}
> > +
> > +static int imx8ulp_reset(struct snd_sof_dev *sdev)
> > +{
> > +     struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
> > +
> > +     /* HiFi4 Platform Clock Enable: 1 enabled, 0 disabled */
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, PLAT_CLK_BIT,
> PLAT_CLK_BIT);
> > +     /* HiFi4 PBCLK clock enable: 1 enabled, 0 disabled */
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, PB_CLK_BIT, PB_CLK_BIT);
> > +     /* HiFi4 Clock Enable: 1 enabled, 0 disabled */
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, HIFI4_CLK_BIT,
> HIFI4_CLK_BIT);
> > +
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, RESET_BIT);
> > +     usleep_range(1, 2);
> > +     /* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT,
> EXECUTE_BIT);
> > +     usleep_range(1, 2);
>
>
> adding newlines would help make the comments more readable, this is a
> bit of an eyesore.
>

ok.


>
> > +     arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0,
> &smc_resource);
> > +
> > +     return 0;
> > +}
> > +
> > +static int imx8ulp_probe(struct snd_sof_dev *sdev)
> > +{
> > +     struct platform_device *pdev =
> > +             container_of(sdev->dev, struct platform_device, dev);
> > +     struct device_node *np = pdev->dev.of_node;
> > +     struct device_node *res_node;
> > +     struct resource *mmio;
> > +     struct imx8ulp_priv *priv;
> > +     struct resource res;
> > +     u32 base, size;
> > +     int ret = 0;
> > +
> > +     priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > +     if (!priv)
> > +             return -ENOMEM;
> > +
> > +     priv->clks = devm_kzalloc(&pdev->dev, sizeof(*priv->clks),
> GFP_KERNEL);
> > +     if (!priv->clks)
> > +             return -ENOMEM;
> > +
> > +     sdev->num_cores = 1;
> > +     sdev->pdata->hw_pdata = priv;
> > +     priv->dev = sdev->dev;
> > +     priv->sdev = sdev;
> > +
> > +     /* System integration module(SIM) control dsp configurtion */
>
> typo: configuration
>
> Yes, I will fix it.


> > +     priv->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,dsp-ctrl");
> > +     if (IS_ERR(priv->regmap))
> > +             return PTR_ERR(priv->regmap);
> > +
> > +     priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
> > +                                                   PLATFORM_DEVID_NONE,
> > +                                                   pdev, sizeof(*pdev));
> > +     if (IS_ERR(priv->ipc_dev))
> > +             return PTR_ERR(priv->ipc_dev);
> > +
> > +     priv->dsp_ipc = dev_get_drvdata(&priv->ipc_dev->dev);
> > +     if (!priv->dsp_ipc) {
> > +             /* DSP IPC driver not probed yet, try later */
> > +             ret = -EPROBE_DEFER;
> > +             dev_err(sdev->dev, "Failed to get drvdata\n");
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     imx_dsp_set_data(priv->dsp_ipc, priv);
> > +     priv->dsp_ipc->ops = &dsp_ops;
> > +
> > +     /* DSP base */
> > +     mmio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +     if (mmio) {
> > +             base = mmio->start;
> > +             size = resource_size(mmio);
> > +     } else {
> > +             dev_err(sdev->dev, "error: failed to get DSP base at idx
> 0\n");
> > +             ret = -EINVAL;
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev, base,
> size);
> > +     if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) {
> > +             dev_err(sdev->dev, "failed to ioremap base 0x%x size
> 0x%x\n",
> > +                     base, size);
> > +             ret = -ENODEV;
> > +             goto exit_pdev_unregister;
> > +     }
> > +     sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
> > +
> > +     res_node = of_parse_phandle(np, "memory-reserved", 0);
> > +     if (!res_node) {
> > +             dev_err(&pdev->dev, "failed to get memory region node\n");
> > +             ret = -ENODEV;
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     ret = of_address_to_resource(res_node, 0, &res);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "failed to get reserved region
> address\n");
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev,
> res.start,
> > +
>  resource_size(&res));
> > +     if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
> > +             dev_err(sdev->dev, "failed to ioremap mem 0x%x size
> 0x%x\n",
> > +                     base, size);
> > +             ret = -ENOMEM;
> > +             goto exit_pdev_unregister;
> > +     }
> > +     sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
> > +
> > +     /* set default mailbox offset for FW ready message */
> > +     sdev->dsp_box.offset = MBOX_OFFSET;
> > +
> > +     ret = of_reserved_mem_device_init(sdev->dev);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "failed to init reserved memory region
> %d\n", ret);
> > +             goto exit_pdev_unregister;
> > +     }
> > +
> > +     priv->clks->dsp_clks = imx8ulp_dsp_clks;
> > +     priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks);
> > +
> > +     ret = imx8_parse_clocks(sdev, priv->clks);
> > +     if (ret < 0)
> > +             goto exit_pdev_unregister;
> > +
> > +     ret = imx8_enable_clocks(sdev, priv->clks);
> > +     if (ret < 0)
> > +             goto exit_pdev_unregister;
> > +
> > +     return 0;
> > +
> > +exit_pdev_unregister:
> > +     platform_device_unregister(priv->ipc_dev);
> > +
> > +     return ret;
> > +}
> > +
>
> > +static int imx8ulp_suspend(struct snd_sof_dev *sdev)
> > +{
> > +     int i;
> > +     struct imx8ulp_priv *priv = (struct imx8ulp_priv
> *)sdev->pdata->hw_pdata;
> > +
> > +     regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT,
> EXECUTE_BIT);
> > +
> > +     for (i = 0; i < DSP_MU_CHAN_NUM; i++)
> > +             imx_dsp_free_channel(priv->dsp_ipc, i);
> > +
> > +     imx8_disable_clocks(sdev, priv->clks);
> > +
> > +     return 0;
> > +}
> > +
> > +static int imx8ulp_resume(struct snd_sof_dev *sdev)
> > +{
> > +     struct imx8ulp_priv *priv = (struct imx8ulp_priv
> *)sdev->pdata->hw_pdata;
> > +     int i;
> > +
> > +     imx8_enable_clocks(sdev, priv->clks);
> > +
> > +     for (i = 0; i < DSP_MU_CHAN_NUM; i++)
> > +             imx_dsp_request_channel(priv->dsp_ipc, i);
> > +
> > +     return 0;
>
> is the assymetry between suspend and resume intentional? You are missing
> the update_bit for EXECUTE_BIT?
>

Yes, intentional. After resume the firmware is reloaded and  EXECUTE_BIT
will be updated at trigger DSP start.

best regards
wang shengjiu

>
> > +}
> > +
>
>
>
Pierre-Louis Bossart July 20, 2022, 1:24 p.m. UTC | #6 
On 7/20/22 03:57, Shengjiu Wang wrote:
>     > +static int imx8ulp_resume(struct snd_sof_dev *sdev)
>     > +{
>     > +     struct imx8ulp_priv *priv = (struct imx8ulp_priv
>     *)sdev->pdata->hw_pdata;
>     > +     int i;
>     > +
>     > +     imx8_enable_clocks(sdev, priv->clks);
>     > +
>     > +     for (i = 0; i < DSP_MU_CHAN_NUM; i++)
>     > +             imx_dsp_request_channel(priv->dsp_ipc, i);
>     > +
>     > +     return 0;
> 
>     is the assymetry between suspend and resume intentional? You are missing
>     the update_bit for EXECUTE_BIT?
> 
> 
> Yes, intentional. After resume the firmware is reloaded and  EXECUTE_BIT
> will be updated at trigger DSP start.

That's worthy of a comment to help reviewers, thanks.
Shengjiu Wang July 20, 2022, 1:49 p.m. UTC | #7 
On Wed, Jul 20, 2022 at 9:25 PM Pierre-Louis Bossart <
pierre-louis.bossart@linux.intel.com> wrote:

>
>
> On 7/20/22 03:57, Shengjiu Wang wrote:
> >     > +static int imx8ulp_resume(struct snd_sof_dev *sdev)
> >     > +{
> >     > +     struct imx8ulp_priv *priv = (struct imx8ulp_priv
> >     *)sdev->pdata->hw_pdata;
> >     > +     int i;
> >     > +
> >     > +     imx8_enable_clocks(sdev, priv->clks);
> >     > +
> >     > +     for (i = 0; i < DSP_MU_CHAN_NUM; i++)
> >     > +             imx_dsp_request_channel(priv->dsp_ipc, i);
> >     > +
> >     > +     return 0;
> >
> >     is the assymetry between suspend and resume intentional? You are
> missing
> >     the update_bit for EXECUTE_BIT?
> >
> >
> > Yes, intentional. After resume the firmware is reloaded and  EXECUTE_BIT
> > will be updated at trigger DSP start.
>
> That's worthy of a comment to help reviewers, thanks.
>

ok, thanks.

best regards
wang shengjiu
diff mbox series

Patch

diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
index cc6e695f913a..4751b04d5e6f 100644
--- a/sound/soc/sof/imx/Kconfig
+++ b/sound/soc/sof/imx/Kconfig
@@ -41,4 +41,13 @@  config SND_SOC_SOF_IMX8M
 	  Say Y if you have such a device.
 	  If unsure select "N".
 
+config SND_SOC_SOF_IMX8ULP
+	tristate "SOF support for i.MX8ULP"
+	depends on IMX_DSP
+	select SND_SOC_SOF_IMX_COMMON
+	help
+	  This adds support for Sound Open Firmware for NXP i.MX8ULP platforms.
+	  Say Y if you have such a device.
+	  If unsure select "N".
+
 endif ## SND_SOC_SOF_IMX_TOPLEVEL
diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
index dba93c3466ec..798b43a415bf 100644
--- a/sound/soc/sof/imx/Makefile
+++ b/sound/soc/sof/imx/Makefile
@@ -1,9 +1,11 @@ 
 # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
 snd-sof-imx8-objs := imx8.o
 snd-sof-imx8m-objs := imx8m.o
+snd-sof-imx8ulp-objs := imx8ulp.o
 
 snd-sof-imx-common-objs := imx-common.o
 
 obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o
 obj-$(CONFIG_SND_SOC_SOF_IMX8M) += snd-sof-imx8m.o
+obj-$(CONFIG_SND_SOC_SOF_IMX8ULP) += snd-sof-imx8ulp.o
 obj-$(CONFIG_SND_SOC_SOF_IMX_COMMON) += imx-common.o
diff --git a/sound/soc/sof/imx/imx8ulp.c b/sound/soc/sof/imx/imx8ulp.c
new file mode 100644
index 000000000000..ecb8750de7ea
--- /dev/null
+++ b/sound/soc/sof/imx/imx8ulp.c
@@ -0,0 +1,541 @@ 
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// Copyright 2021-2022 NXP
+//
+// Author: Peng Zhang <peng.zhang_8@nxp.com>
+//
+// Hardware interface for audio DSP on i.MX8ULP
+
+#include <linux/clk.h>
+#include <linux/firmware.h>
+#include <linux/firmware/imx/dsp.h>
+#include <linux/firmware/imx/ipc.h>
+#include <linux/firmware/imx/svc/misc.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+
+#include <sound/sof.h>
+#include <sound/sof/xtensa.h>
+
+#include "../ops.h"
+#include "../sof-of-dev.h"
+#include "imx-common.h"
+
+#define FSL_SIP_HIFI_XRDC	0xc200000e
+
+/* SIM Domain register */
+#define SYSCTRL0		0x8
+#define EXECUTE_BIT		BIT(13)
+#define RESET_BIT		BIT(16)
+#define HIFI4_CLK_BIT		BIT(17)
+#define PB_CLK_BIT		BIT(18)
+#define PLAT_CLK_BIT		BIT(19)
+#define DEBUG_LOGIC_BIT		BIT(25)
+
+#define MBOX_OFFSET		0x800000
+#define MBOX_SIZE		0x1000
+
+struct arm_smccc_res		smc_resource;
+
+static struct clk_bulk_data imx8ulp_dsp_clks[] = {
+	{ .id = "core" },
+	{ .id = "ipg" },
+	{ .id = "ocram" },
+	{ .id = "mu" },
+};
+
+struct imx8ulp_priv {
+	struct device *dev;
+	struct snd_sof_dev *sdev;
+
+	/* DSP IPC handler */
+	struct imx_dsp_ipc *dsp_ipc;
+	struct platform_device *ipc_dev;
+
+	struct regmap *regmap;
+	struct imx_clocks *clks;
+};
+
+static void imx8ulp_sim_lpav_start(struct imx8ulp_priv *priv)
+{
+	/* Controls the HiFi4 DSP Reset: 1 in reset, 0 out of reset */
+	regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, 0);
+	/* Reset HiFi4 DSP Debug logic: 1 reset, 0 not set */
+	regmap_update_bits(priv->regmap, SYSCTRL0, DEBUG_LOGIC_BIT, 0);
+	/* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
+	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, 0);
+}
+
+static void imx8ulp_get_reply(struct snd_sof_dev *sdev)
+{
+	struct snd_sof_ipc_msg *msg = sdev->msg;
+	struct sof_ipc_reply reply;
+	int ret = 0;
+
+	if (!msg) {
+		dev_warn(sdev->dev, "unexpected ipc interrupt\n");
+		return;
+	}
+
+	/* get reply */
+	sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
+
+	if (reply.error < 0) {
+		memcpy(msg->reply_data, &reply, sizeof(reply));
+		ret = reply.error;
+	} else {
+		/* reply has correct size? */
+		if (reply.hdr.size != msg->reply_size) {
+			dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",
+				msg->reply_size, reply.hdr.size);
+			ret = -EINVAL;
+		}
+
+		/* read the message */
+		if (msg->reply_size > 0)
+			sof_mailbox_read(sdev, sdev->host_box.offset,
+					 msg->reply_data, msg->reply_size);
+	}
+
+	msg->reply_error = ret;
+}
+
+static int imx8ulp_get_mailbox_offset(struct snd_sof_dev *sdev)
+{
+	return MBOX_OFFSET;
+}
+
+static int imx8ulp_get_window_offset(struct snd_sof_dev *sdev, u32 id)
+{
+	return MBOX_OFFSET;
+}
+
+static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc)
+{
+	struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->sdev->ipc_lock, flags);
+
+	imx8ulp_get_reply(priv->sdev);
+	snd_sof_ipc_reply(priv->sdev, 0);
+	spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags);
+}
+
+static void imx8ulp_dsp_handle_request(struct imx_dsp_ipc *ipc)
+{
+	struct imx8ulp_priv *priv = imx_dsp_get_data(ipc);
+	u32 p; /* panic code */
+
+	/* Read the message from the debug box. */
+	sof_mailbox_read(priv->sdev, priv->sdev->debug_box.offset + 4, &p, sizeof(p));
+
+	/* Check to see if the message is a panic code (0x0dead***) */
+	if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC)
+		snd_sof_dsp_panic(priv->sdev, p, true);
+	else
+		snd_sof_ipc_msgs_rx(priv->sdev);
+}
+
+static struct imx_dsp_ops dsp_ops = {
+	.handle_reply		= imx8ulp_dsp_handle_reply,
+	.handle_request		= imx8ulp_dsp_handle_request,
+};
+
+static int imx8ulp_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
+{
+	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
+
+	sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
+			  msg->msg_size);
+	imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
+
+	return 0;
+}
+
+static int imx8ulp_run(struct snd_sof_dev *sdev)
+{
+	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
+
+	imx8ulp_sim_lpav_start(priv);
+
+	return 0;
+}
+
+static int imx8ulp_reset(struct snd_sof_dev *sdev)
+{
+	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
+
+	/* HiFi4 Platform Clock Enable: 1 enabled, 0 disabled */
+	regmap_update_bits(priv->regmap, SYSCTRL0, PLAT_CLK_BIT, PLAT_CLK_BIT);
+	/* HiFi4 PBCLK clock enable: 1 enabled, 0 disabled */
+	regmap_update_bits(priv->regmap, SYSCTRL0, PB_CLK_BIT, PB_CLK_BIT);
+	/* HiFi4 Clock Enable: 1 enabled, 0 disabled */
+	regmap_update_bits(priv->regmap, SYSCTRL0, HIFI4_CLK_BIT, HIFI4_CLK_BIT);
+
+	regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, RESET_BIT);
+	usleep_range(1, 2);
+	/* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */
+	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
+	usleep_range(1, 2);
+
+	arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource);
+
+	return 0;
+}
+
+static int imx8ulp_probe(struct snd_sof_dev *sdev)
+{
+	struct platform_device *pdev =
+		container_of(sdev->dev, struct platform_device, dev);
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *res_node;
+	struct resource *mmio;
+	struct imx8ulp_priv *priv;
+	struct resource res;
+	u32 base, size;
+	int ret = 0;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->clks = devm_kzalloc(&pdev->dev, sizeof(*priv->clks), GFP_KERNEL);
+	if (!priv->clks)
+		return -ENOMEM;
+
+	sdev->num_cores = 1;
+	sdev->pdata->hw_pdata = priv;
+	priv->dev = sdev->dev;
+	priv->sdev = sdev;
+
+	/* System integration module(SIM) control dsp configurtion */
+	priv->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,dsp-ctrl");
+	if (IS_ERR(priv->regmap))
+		return PTR_ERR(priv->regmap);
+
+	priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
+						      PLATFORM_DEVID_NONE,
+						      pdev, sizeof(*pdev));
+	if (IS_ERR(priv->ipc_dev))
+		return PTR_ERR(priv->ipc_dev);
+
+	priv->dsp_ipc = dev_get_drvdata(&priv->ipc_dev->dev);
+	if (!priv->dsp_ipc) {
+		/* DSP IPC driver not probed yet, try later */
+		ret = -EPROBE_DEFER;
+		dev_err(sdev->dev, "Failed to get drvdata\n");
+		goto exit_pdev_unregister;
+	}
+
+	imx_dsp_set_data(priv->dsp_ipc, priv);
+	priv->dsp_ipc->ops = &dsp_ops;
+
+	/* DSP base */
+	mmio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (mmio) {
+		base = mmio->start;
+		size = resource_size(mmio);
+	} else {
+		dev_err(sdev->dev, "error: failed to get DSP base at idx 0\n");
+		ret = -EINVAL;
+		goto exit_pdev_unregister;
+	}
+
+	sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev, base, size);
+	if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) {
+		dev_err(sdev->dev, "failed to ioremap base 0x%x size 0x%x\n",
+			base, size);
+		ret = -ENODEV;
+		goto exit_pdev_unregister;
+	}
+	sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
+
+	res_node = of_parse_phandle(np, "memory-reserved", 0);
+	if (!res_node) {
+		dev_err(&pdev->dev, "failed to get memory region node\n");
+		ret = -ENODEV;
+		goto exit_pdev_unregister;
+	}
+
+	ret = of_address_to_resource(res_node, 0, &res);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to get reserved region address\n");
+		goto exit_pdev_unregister;
+	}
+
+	sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start,
+							  resource_size(&res));
+	if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
+		dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n",
+			base, size);
+		ret = -ENOMEM;
+		goto exit_pdev_unregister;
+	}
+	sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
+
+	/* set default mailbox offset for FW ready message */
+	sdev->dsp_box.offset = MBOX_OFFSET;
+
+	ret = of_reserved_mem_device_init(sdev->dev);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to init reserved memory region %d\n", ret);
+		goto exit_pdev_unregister;
+	}
+
+	priv->clks->dsp_clks = imx8ulp_dsp_clks;
+	priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks);
+
+	ret = imx8_parse_clocks(sdev, priv->clks);
+	if (ret < 0)
+		goto exit_pdev_unregister;
+
+	ret = imx8_enable_clocks(sdev, priv->clks);
+	if (ret < 0)
+		goto exit_pdev_unregister;
+
+	return 0;
+
+exit_pdev_unregister:
+	platform_device_unregister(priv->ipc_dev);
+
+	return ret;
+}
+
+static int imx8ulp_remove(struct snd_sof_dev *sdev)
+{
+	struct imx8ulp_priv *priv = sdev->pdata->hw_pdata;
+
+	platform_device_unregister(priv->ipc_dev);
+
+	return 0;
+}
+
+/* on i.MX8 there is 1 to 1 match between type and BAR idx */
+static int imx8ulp_get_bar_index(struct snd_sof_dev *sdev, u32 type)
+{
+	return type;
+}
+
+static int imx8ulp_suspend(struct snd_sof_dev *sdev)
+{
+	int i;
+	struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata;
+
+	regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT);
+
+	for (i = 0; i < DSP_MU_CHAN_NUM; i++)
+		imx_dsp_free_channel(priv->dsp_ipc, i);
+
+	imx8_disable_clocks(sdev, priv->clks);
+
+	return 0;
+}
+
+static int imx8ulp_resume(struct snd_sof_dev *sdev)
+{
+	struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata;
+	int i;
+
+	imx8_enable_clocks(sdev, priv->clks);
+
+	for (i = 0; i < DSP_MU_CHAN_NUM; i++)
+		imx_dsp_request_channel(priv->dsp_ipc, i);
+
+	return 0;
+}
+
+static int imx8ulp_dsp_runtime_resume(struct snd_sof_dev *sdev)
+{
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = SOF_DSP_PM_D0,
+		.substate = 0,
+	};
+
+	imx8ulp_resume(sdev);
+
+	return snd_sof_dsp_set_power_state(sdev, &target_dsp_state);
+}
+
+static int imx8ulp_dsp_runtime_suspend(struct snd_sof_dev *sdev)
+{
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = SOF_DSP_PM_D3,
+		.substate = 0,
+	};
+
+	imx8ulp_suspend(sdev);
+
+	return snd_sof_dsp_set_power_state(sdev, &target_dsp_state);
+}
+
+static int imx8ulp_dsp_suspend(struct snd_sof_dev *sdev, unsigned int target_state)
+{
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = target_state,
+		.substate = 0,
+	};
+
+	if (!pm_runtime_suspended(sdev->dev))
+		imx8ulp_suspend(sdev);
+
+	return snd_sof_dsp_set_power_state(sdev, &target_dsp_state);
+}
+
+static int imx8ulp_dsp_resume(struct snd_sof_dev *sdev)
+{
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = SOF_DSP_PM_D0,
+		.substate = 0,
+	};
+
+	imx8ulp_resume(sdev);
+
+	if (pm_runtime_suspended(sdev->dev)) {
+		pm_runtime_disable(sdev->dev);
+		pm_runtime_set_active(sdev->dev);
+		pm_runtime_mark_last_busy(sdev->dev);
+		pm_runtime_enable(sdev->dev);
+		pm_runtime_idle(sdev->dev);
+	}
+
+	return snd_sof_dsp_set_power_state(sdev, &target_dsp_state);
+}
+
+static struct snd_soc_dai_driver imx8ulp_dai[] = {
+	{
+		.name = "sai5",
+		.playback = {
+			.channels_min = 1,
+			.channels_max = 32,
+		},
+		.capture = {
+			.channels_min = 1,
+			.channels_max = 32,
+		},
+	},
+	{
+		.name = "sai6",
+		.playback = {
+			.channels_min = 1,
+			.channels_max = 32,
+		},
+		.capture = {
+			.channels_min = 1,
+			.channels_max = 32,
+		},
+	},
+};
+
+static int imx8ulp_dsp_set_power_state(struct snd_sof_dev *sdev,
+				       const struct sof_dsp_power_state *target_state)
+{
+	sdev->dsp_power_state = *target_state;
+
+	return 0;
+}
+
+/* i.MX8 ops */
+struct snd_sof_dsp_ops sof_imx8ulp_ops = {
+	/* probe and remove */
+	.probe		= imx8ulp_probe,
+	.remove		= imx8ulp_remove,
+	/* DSP core boot */
+	.run		= imx8ulp_run,
+	.reset		= imx8ulp_reset,
+
+	/* Block IO */
+	.block_read	= sof_block_read,
+	.block_write	= sof_block_write,
+
+	/* Module IO */
+	.read64		= sof_io_read64,
+
+	/* Mailbox IO */
+	.mailbox_read	= sof_mailbox_read,
+	.mailbox_write	= sof_mailbox_write,
+
+	/* ipc */
+	.send_msg	= imx8ulp_send_msg,
+	.get_mailbox_offset	= imx8ulp_get_mailbox_offset,
+	.get_window_offset	= imx8ulp_get_window_offset,
+
+	.ipc_msg_data	= sof_ipc_msg_data,
+	.set_stream_data_offset = sof_set_stream_data_offset,
+
+	/* stream callbacks */
+	.pcm_open	= sof_stream_pcm_open,
+	.pcm_close	= sof_stream_pcm_close,
+
+	/* module loading */
+	.get_bar_index	= imx8ulp_get_bar_index,
+	/* firmware loading */
+	.load_firmware	= snd_sof_load_firmware_memcpy,
+
+	/* Debug information */
+	.dbg_dump	= imx8_dump,
+
+	/* Firmware ops */
+	.dsp_arch_ops	= &sof_xtensa_arch_ops,
+
+	/* DAI drivers */
+	.drv		= imx8ulp_dai,
+	.num_drv	= ARRAY_SIZE(imx8ulp_dai),
+
+	/* ALSA HW info flags */
+	.hw_info	= SNDRV_PCM_INFO_MMAP |
+			SNDRV_PCM_INFO_MMAP_VALID |
+			SNDRV_PCM_INFO_INTERLEAVED |
+			SNDRV_PCM_INFO_PAUSE |
+			SNDRV_PCM_INFO_NO_PERIOD_WAKEUP,
+
+	/* PM */
+	.runtime_suspend	= imx8ulp_dsp_runtime_suspend,
+	.runtime_resume		= imx8ulp_dsp_runtime_resume,
+
+	.suspend	= imx8ulp_dsp_suspend,
+	.resume		= imx8ulp_dsp_resume,
+
+	.set_power_state	= imx8ulp_dsp_set_power_state,
+};
+
+static struct sof_dev_desc sof_of_imx8ulp_desc = {
+	.ipc_supported_mask     = BIT(SOF_IPC),
+	.ipc_default            = SOF_IPC,
+	.default_fw_path = {
+		[SOF_IPC] = "imx/sof",
+	},
+	.default_tplg_path = {
+		[SOF_IPC] = "imx/sof-tplg",
+	},
+	.default_fw_filename = {
+		[SOF_IPC] = "sof-imx8ulp.ri",
+	},
+	.nocodec_tplg_filename = "sof-imx8ulp-nocodec.tplg",
+	.ops = &sof_imx8ulp_ops,
+};
+
+static const struct of_device_id sof_of_imx8ulp_ids[] = {
+	{ .compatible = "fsl,imx8ulp-dsp", .data = &sof_of_imx8ulp_desc},
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sof_of_imx8ulp_ids);
+
+/* DT driver definition */
+static struct platform_driver snd_sof_of_imx8ulp_driver = {
+	.probe = sof_of_probe,
+	.remove = sof_of_remove,
+	.driver = {
+		.name = "sof-audio-of-imx8ulp",
+		.pm = &sof_of_pm,
+		.of_match_table = sof_of_imx8ulp_ids,
+	},
+};
+module_platform_driver(snd_sof_of_imx8ulp_driver);
+
+MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA);
+MODULE_LICENSE("Dual BSD/GPL");