* [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 8:40 [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
2019-07-23 15:01 ` [Sound-open-firmware] " Pierre-Louis Bossart
2019-07-23 8:41 ` [PATCH v2 2/5] ASoC: SOF: topology: Add dummy support for i.MX8 DAIs Daniel Baluta
` (3 subsequent siblings)
4 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:41 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
linux-kernel, linux-imx, shengjiu.wang, paul.olaru, robh+dt,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware, Daniel Baluta
Add support for the audio DSP hardware found on NXP i.MX8 platform.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
- This is also on review with SOF community here:
https://github.com/thesofproject/linux/pull/1048
sound/soc/sof/Kconfig | 1 +
sound/soc/sof/Makefile | 1 +
sound/soc/sof/imx/Kconfig | 20 ++
sound/soc/sof/imx/Makefile | 7 +
sound/soc/sof/imx/imx8.c | 464 +++++++++++++++++++++++++++++++++++++
5 files changed, 493 insertions(+)
create mode 100644 sound/soc/sof/imx/Kconfig
create mode 100644 sound/soc/sof/imx/Makefile
create mode 100644 sound/soc/sof/imx/imx8.c
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index fb01f0ca6027..61b97fc55bb2 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -163,6 +163,7 @@ config SND_SOC_SOF_PROBE_WORK_QUEUE
When selected, the probe is handled in two steps, for example to
avoid lockdeps if request_module is used in the probe.
+source "sound/soc/sof/imx/Kconfig"
source "sound/soc/sof/intel/Kconfig"
source "sound/soc/sof/xtensa/Kconfig"
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 8f14c9d2950b..6e43d411ffef 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -15,4 +15,5 @@ obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o
obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o
obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/
+obj-$(CONFIG_SND_SOC_SOF_IMX_TOPLEVEL) += imx/
obj-$(CONFIG_SND_SOC_SOF_XTENSA) += xtensa/
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
new file mode 100644
index 000000000000..fff64a9970f0
--- /dev/null
+++ b/sound/soc/sof/imx/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+
+config SND_SOC_SOF_IMX_TOPLEVEL
+ bool "SOF support for NXP i.MX audio DSPs"
+ depends on ARM64 || COMPILE_TEST
+ help
+ This adds support for Sound Open Firmware for NXP i.MX platforms.
+ Say Y if you have such a device.
+ If unsure select "N".
+
+if SND_SOC_SOF_IMX_TOPLEVEL
+
+config SND_SOC_SOF_IMX8
+ tristate "SOF support for i.MX8"
+ help
+ This adds support for Sound Open Firmware for NXP i.MX8 platforms
+ Say Y if you have such a device.
+ If unsure select "N".
+
+endif ## SND_SOC_SOF_IMX_IMX_TOPLEVEL
diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
new file mode 100644
index 000000000000..c69237971da5
--- /dev/null
+++ b/sound/soc/sof/imx/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+
+ccflags-y += -DDEBUG
+
+snd-sof-imx8-objs := imx8.o
+
+obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o
diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c
new file mode 100644
index 000000000000..086606aa4b20
--- /dev/null
+++ b/sound/soc/sof/imx/imx8.c
@@ -0,0 +1,464 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// Copyright 2019 NXP
+//
+// Author: Daniel Baluta <daniel.baluta@nxp.com>
+//
+// Hardware interface for audio DSP on i.MX8
+
+#include <linux/firmware.h>
+#include <linux/of_platform.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/pm_domain.h>
+
+#include <linux/module.h>
+#include <sound/sof.h>
+#include <sound/sof/xtensa.h>
+#include <linux/firmware/imx/ipc.h>
+#include <linux/firmware/imx/dsp.h>
+
+#include <linux/firmware/imx/svc/misc.h>
+#include <dt-bindings/firmware/imx/rsrc.h>
+#include "../ops.h"
+
+/* DSP memories */
+#define IRAM_OFFSET 0x10000
+#define IRAM_SIZE (2 * 1024)
+#define DRAM0_OFFSET 0x0
+#define DRAM0_SIZE (32 * 1024)
+#define DRAM1_OFFSET 0x8000
+#define DRAM1_SIZE (32 * 1024)
+#define SYSRAM_OFFSET 0x18000
+#define SYSRAM_SIZE (256 * 1024)
+#define SYSROM_OFFSET 0x58000
+#define SYSROM_SIZE (192 * 1024)
+
+#define RESET_VECTOR_VADDR 0x596f8000
+
+#define MBOX_OFFSET 0x800000
+#define MBOX_SIZE 0x1000
+
+struct imx8_priv {
+ struct device *dev;
+ struct snd_sof_dev *sdev;
+ struct imx_dsp_ipc *dsp_ipc;
+ struct imx_sc_ipc *sc_ipc;
+};
+
+static void imx8_get_windows(struct snd_sof_dev *sdev)
+{
+ struct sof_ipc_window_elem *elem;
+ u32 outbox_offset = 0;
+ u32 stream_offset = 0;
+ u32 inbox_offset = 0;
+ u32 outbox_size = 0;
+ u32 stream_size = 0;
+ u32 inbox_size = 0;
+ int i;
+
+ if (!sdev->info_window) {
+ dev_err(sdev->dev, "error: have no window info\n");
+ return;
+ }
+
+ for (i = 0; i < sdev->info_window->num_windows; i++) {
+ elem = &sdev->info_window->window[i];
+
+ switch (elem->type) {
+ case SOF_IPC_REGION_UPBOX:
+ inbox_offset = elem->offset + MBOX_OFFSET;
+ inbox_size = elem->size;
+ snd_sof_debugfs_io_item(sdev,
+ sdev->bar[SOF_FW_BLK_TYPE_SRAM]
+ + inbox_offset,
+ elem->size, "inbox",
+ SOF_DEBUGFS_ACCESS_D0_ONLY);
+ break;
+ case SOF_IPC_REGION_DOWNBOX:
+ outbox_offset = elem->offset + MBOX_OFFSET;
+ outbox_size = elem->size;
+ snd_sof_debugfs_io_item(sdev,
+ sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
+ + elem->offset + MBOX_OFFSET,
+ elem->size, "etrace",
+ SOF_DEBUGFS_ACCESS_D0_ONLY);
+ break;
+ case SOF_IPC_REGION_DEBUG:
+ snd_sof_debugfs_io_item(sdev,
+ sdev->bar[SOF_FW_BLK_TYPE_SRAM]
+ + elem->offset + MBOX_OFFSET,
+ elem->size, "debug",
+ SOF_DEBUGFS_ACCESS_D0_ONLY);
+ break;
+ case SOF_IPC_REGION_STREAM:
+ stream_offset = elem->offset + MBOX_OFFSET;
+ stream_size = elem->size;
+ snd_sof_debugfs_io_item(sdev,
+ sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
+ + elem->offset + MBOX_OFFSET,
+ elem->size, "regs",
+ SOF_DEBUGFS_ACCESS_D0_ONLY);
+ break;
+ case SOF_IPC_REGION_EXCEPTION:
+ sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET;
+ snd_sof_debugfs_io_item(sdev,
+ sdev->bar[SOF_FW_BLK_TYPE_SRAM]
+ + elem->offset + MBOX_OFFSET,
+ elem->size, "exception",
+ SOF_DEBUGFS_ACCESS_D0_ONLY);
+ break;
+ default:
+ dev_err(sdev->dev, "error: get illegal window info\n");
+ return;
+ }
+ }
+
+ if (outbox_size == 0 || inbox_size == 0) {
+ dev_err(sdev->dev, "error: get illegal mailbox window\n");
+ return;
+ }
+
+ snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
+ outbox_offset, outbox_size);
+ sdev->stream_box.offset = stream_offset;
+ sdev->stream_box.size = stream_size;
+
+ dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
+ inbox_offset, inbox_size);
+ dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
+ outbox_offset, outbox_size);
+ dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
+ stream_offset, stream_size);
+}
+
+/*
+ * IPC Firmware ready.
+ */
+static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
+{
+ struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
+ u32 offset;
+ int ret;
+
+ /* mailbox must be on 4k boundary */
+ offset = MBOX_OFFSET;
+
+ dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",
+ msg_id, offset);
+
+ /* no need to re-check version/ABI for subsequent boots */
+ if (!sdev->first_boot)
+ return 0;
+
+ /* copy data from the DSP FW ready offset */
+ sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready,
+ sizeof(*fw_ready));
+ snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
+ fw_ready->dspbox_size,
+ fw_ready->hostbox_offset,
+ fw_ready->hostbox_size);
+
+ /* make sure ABI version is compatible */
+ ret = snd_sof_ipc_valid(sdev);
+ if (ret < 0)
+ return ret;
+
+ /* now check for extended data */
+ snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET +
+ sizeof(struct sof_ipc_fw_ready));
+
+ imx8_get_windows(sdev);
+
+ return 0;
+}
+
+static void imx8_get_reply(struct snd_sof_dev *sdev)
+{
+ struct snd_sof_ipc_msg *msg = sdev->msg;
+ struct sof_ipc_reply reply;
+ unsigned long flags;
+ 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));
+
+ spin_lock_irqsave(&sdev->ipc_lock, flags);
+
+ 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;
+
+ spin_unlock_irqrestore(&sdev->ipc_lock, flags);
+}
+
+void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc)
+{
+ struct imx8_priv *priv = imx_dsp_get_data(ipc);
+
+ imx8_get_reply(priv->sdev);
+ snd_sof_ipc_reply(priv->sdev, 0);
+}
+
+void imx_dsp_handle_request(struct imx_dsp_ipc *ipc)
+{
+ struct imx8_priv *priv = imx_dsp_get_data(ipc);
+
+ snd_sof_ipc_msgs_rx(priv->sdev);
+}
+
+struct imx_dsp_ops dsp_ops = {
+ .handle_reply = imx_dsp_handle_reply,
+ .handle_request = imx_dsp_handle_request,
+};
+
+static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
+{
+ struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
+
+ sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
+ msg->msg_size);
+ imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
+
+ return 0;
+}
+
+/*
+ * DSP control.
+ */
+static int imx8_run(struct snd_sof_dev *sdev)
+{
+ int ret;
+ struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private;
+
+ ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
+ IMX_SC_C_OFS_SEL, 1);
+ if (ret < 0) {
+ dev_err(sdev->dev, "Error system address offset source select\n");
+ return ret;
+ }
+
+ ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
+ IMX_SC_C_OFS_AUDIO, 0x80);
+ if (ret < 0) {
+ dev_err(sdev->dev, "Error system address offset of AUDIO\n");
+ return ret;
+ }
+
+ ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
+ IMX_SC_C_OFS_PERIPH, 0x5A);
+ if (ret < 0) {
+ dev_err(sdev->dev, "Error system address offset of PERIPH %d\n",
+ ret);
+ return ret;
+ }
+
+ ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
+ IMX_SC_C_OFS_IRQ, 0x51);
+ if (ret < 0) {
+ dev_err(sdev->dev, "Error system address offset of IRQ\n");
+ return ret;
+ }
+
+ imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true,
+ RESET_VECTOR_VADDR);
+
+ return 0;
+}
+
+static int imx8_probe(struct snd_sof_dev *sdev)
+{
+ struct imx8_priv *priv;
+ int i;
+ struct platform_device *pdev =
+ container_of(sdev->dev, struct platform_device, dev);
+ struct platform_device *ipc_dev;
+ struct resource *mmio;
+ int num_domains = 0;
+ u32 base, size;
+ int ret = 0;
+ struct device_node *np = pdev->dev.of_node;
+ struct device_node *res_node;
+ struct resource res;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ sdev->private = priv;
+ priv->dev = sdev->dev;
+ priv->sdev = sdev;
+
+ ret = imx_scu_get_handle(&priv->sc_ipc);
+ if (ret) {
+ dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",
+ ret);
+ return ret;
+ }
+
+ ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
+ PLATFORM_DEVID_NONE,
+ pdev, sizeof(*pdev));
+ if (IS_ERR(ipc_dev)) {
+ dev_err(sdev->dev, "Failed to register platform device\n");
+ return PTR_ERR(ipc_dev);
+ }
+
+ priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev);
+ if (!priv->dsp_ipc)
+ return -EPROBE_DEFER;
+
+ imx_dsp_set_data(priv->dsp_ipc, priv);
+ priv->dsp_ipc->ops = &dsp_ops;
+
+ num_domains = of_count_phandle_with_args(np, "power-domains",
+ "#power-domain-cells");
+ for (i = 0; i < num_domains; i++) {
+ struct device *pd_dev;
+ struct device_link *link;
+
+ pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i);
+ if (IS_ERR(pd_dev))
+ return PTR_ERR(pd_dev);
+
+ link = device_link_add(&pdev->dev, pd_dev,
+ DL_FLAG_STATELESS |
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_RPM_ACTIVE);
+ if (IS_ERR(link))
+ return PTR_ERR(link);
+ }
+
+ /* 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");
+ return -EINVAL;
+ }
+
+ 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);
+ return -ENODEV;
+ }
+ sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
+
+ res_node = of_parse_phandle(np, "memory-region", 0);
+ if (!res_node) {
+ dev_err(&pdev->dev, "failed to get memory region node\n");
+ return -ENODEV;
+ }
+ if (of_address_to_resource(res_node, 0, &res)) {
+ dev_err(&pdev->dev, "failed to get reserved region address\n");
+ return -EINVAL;
+ }
+
+ sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start,
+ res.end - res.start +
+ 1);
+ if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
+ dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n",
+ base, size);
+ return -ENODEV;
+ }
+ sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
+
+ return ret;
+}
+
+/* on i.MX8 there is 1 to 1 match between type and BAR idx */
+int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type)
+{
+ return type;
+}
+
+void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
+ struct snd_pcm_substream *substream,
+ void *p, size_t sz)
+{
+ sof_mailbox_read(sdev, sdev->dsp_box.offset, p, sz);
+}
+
+int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
+ struct snd_pcm_substream *substream,
+ const struct sof_ipc_pcm_params_reply *reply)
+{
+ return 0;
+}
+
+static struct snd_soc_dai_driver imx8_dai[] = {
+{
+ .name = "esai-port",
+},
+};
+
+/* i.MX8 ops */
+struct snd_sof_dsp_ops sof_imx8_ops = {
+ /* device init */
+ .probe = imx8_probe,
+
+ /* DSP core boot */
+ .run = imx8_run,
+
+ /* Block IO */
+ .block_read = sof_block_read,
+ .block_write = sof_block_write,
+
+ /* ipc */
+ .send_msg = imx8_send_msg,
+ .fw_ready = imx8_fw_ready,
+
+ .ipc_msg_data = imx8_ipc_msg_data,
+ .ipc_pcm_params = imx8_ipc_pcm_params,
+
+ /* module loading */
+ .load_module = snd_sof_parse_module_memcpy,
+ .get_bar_index = imx8_get_bar_index,
+ /* firmware loading */
+ .load_firmware = snd_sof_load_firmware_memcpy,
+
+ /* DAI drivers */
+ .drv = imx8_dai,
+ .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
+};
+EXPORT_SYMBOL(sof_imx8_ops);
+
+MODULE_LICENSE("Dual BSD/GPL");
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 8:41 ` [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support Daniel Baluta
@ 2019-07-23 15:01 ` Pierre-Louis Bossart
2019-07-24 6:54 ` Daniel Baluta
0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2019-07-23 15:01 UTC (permalink / raw)
To: Daniel Baluta, m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
shengjiu.wang, linux-kernel, paul.olaru, robh+dt, linux-imx,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
> diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
> new file mode 100644
> index 000000000000..c69237971da5
> --- /dev/null
> +++ b/sound/soc/sof/imx/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +
> +ccflags-y += -DDEBUG
this should be removed or in a separate patch.
> +struct imx8_priv {
> + struct device *dev;
> + struct snd_sof_dev *sdev;
> + struct imx_dsp_ipc *dsp_ipc;
> + struct imx_sc_ipc *sc_ipc;
maybe a comment to explain what 'sc' stands for?
> +};
> +
> +static void imx8_get_windows(struct snd_sof_dev *sdev)
> +{
> + struct sof_ipc_window_elem *elem;
> + u32 outbox_offset = 0;
> + u32 stream_offset = 0;
> + u32 inbox_offset = 0;
> + u32 outbox_size = 0;
> + u32 stream_size = 0;
> + u32 inbox_size = 0;
> + int i;
> +
> + if (!sdev->info_window) {
> + dev_err(sdev->dev, "error: have no window info\n");
> + return;
> + }
> +
> + for (i = 0; i < sdev->info_window->num_windows; i++) {
> + elem = &sdev->info_window->window[i];
> +
> + switch (elem->type) {
> + case SOF_IPC_REGION_UPBOX:
> + inbox_offset = elem->offset + MBOX_OFFSET;
> + inbox_size = elem->size;
> + snd_sof_debugfs_io_item(sdev,
> + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> + + inbox_offset,
> + elem->size, "inbox",
> + SOF_DEBUGFS_ACCESS_D0_ONLY);
> + break;
> + case SOF_IPC_REGION_DOWNBOX:
> + outbox_offset = elem->offset + MBOX_OFFSET;
> + outbox_size = elem->size;
> + snd_sof_debugfs_io_item(sdev,
> + sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
> + + elem->offset + MBOX_OFFSET,
> + elem->size, "etrace",
> + SOF_DEBUGFS_ACCESS_D0_ONLY);
> + break;
> + case SOF_IPC_REGION_DEBUG:
> + snd_sof_debugfs_io_item(sdev,
> + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> + + elem->offset + MBOX_OFFSET,
> + elem->size, "debug",
> + SOF_DEBUGFS_ACCESS_D0_ONLY);
> + break;
> + case SOF_IPC_REGION_STREAM:
> + stream_offset = elem->offset + MBOX_OFFSET;
> + stream_size = elem->size;
> + snd_sof_debugfs_io_item(sdev,
> + sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
> + + elem->offset + MBOX_OFFSET,
> + elem->size, "regs",
> + SOF_DEBUGFS_ACCESS_D0_ONLY);
> + break;
> + case SOF_IPC_REGION_EXCEPTION:
> + sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET;
> + snd_sof_debugfs_io_item(sdev,
> + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> + + elem->offset + MBOX_OFFSET,
> + elem->size, "exception",
> + SOF_DEBUGFS_ACCESS_D0_ONLY);
> + break;
> + default:
> + dev_err(sdev->dev, "error: get illegal window info\n");
> + return;
> + }
> + }
> +
> + if (outbox_size == 0 || inbox_size == 0) {
> + dev_err(sdev->dev, "error: get illegal mailbox window\n");
> + return;
> + }
> +
> + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
> + outbox_offset, outbox_size);
> + sdev->stream_box.offset = stream_offset;
> + sdev->stream_box.size = stream_size;
> +
> + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
> + inbox_offset, inbox_size);
> + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
> + outbox_offset, outbox_size);
> + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
> + stream_offset, stream_size);
> +}
This looks 100% similar to Baytrail?
> +
> +/*
> + * IPC Firmware ready.
> + */
> +static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
> +{
> + struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
> + u32 offset;
> + int ret;
> +
> + /* mailbox must be on 4k boundary */
> + offset = MBOX_OFFSET;
> +
> + dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",
> + msg_id, offset);
> +
> + /* no need to re-check version/ABI for subsequent boots */
> + if (!sdev->first_boot)
> + return 0;
> +
> + /* copy data from the DSP FW ready offset */
> + sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready,
> + sizeof(*fw_ready));
> + snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
> + fw_ready->dspbox_size,
> + fw_ready->hostbox_offset,
> + fw_ready->hostbox_size);
> +
> + /* make sure ABI version is compatible */
> + ret = snd_sof_ipc_valid(sdev);
> + if (ret < 0)
> + return ret;
> +
> + /* now check for extended data */
> + snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET +
> + sizeof(struct sof_ipc_fw_ready));
> +
> + imx8_get_windows(sdev);
> +
> + return 0;
> +}
That code looks nearly similar to the baytrail one except for the last
line, we should look into factoring this.
> +
> +static void imx8_get_reply(struct snd_sof_dev *sdev)
> +{
> + struct snd_sof_ipc_msg *msg = sdev->msg;
> + struct sof_ipc_reply reply;
> + unsigned long flags;
> + 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));
> +
> + spin_lock_irqsave(&sdev->ipc_lock, flags);
> +
> + 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;
> +
> + spin_unlock_irqrestore(&sdev->ipc_lock, flags);
I don't see a spin_lock/unlock for the get_reply in the Intel code, is
this necessary?
> +}
> +
> +void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> +{
> + struct imx8_priv *priv = imx_dsp_get_data(ipc);
> +
> + imx8_get_reply(priv->sdev);
> + snd_sof_ipc_reply(priv->sdev, 0);
> +}
> +
> +void imx_dsp_handle_request(struct imx_dsp_ipc *ipc)
> +{
> + struct imx8_priv *priv = imx_dsp_get_data(ipc);
> +
> + snd_sof_ipc_msgs_rx(priv->sdev);
> +}
> +
> +struct imx_dsp_ops dsp_ops = {
> + .handle_reply = imx_dsp_handle_reply,
> + .handle_request = imx_dsp_handle_request,
> +};
> +
> +static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
> +{
> + struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
> +
> + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
> + msg->msg_size);
> + imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
> +
> + return 0;
> +}
> +
> +/*
> + * DSP control.
> + */
> +static int imx8_run(struct snd_sof_dev *sdev)
> +{
> + int ret;
> + struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private;
> +
> + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> + IMX_SC_C_OFS_SEL, 1);
> + if (ret < 0) {
> + dev_err(sdev->dev, "Error system address offset source select\n");
> + return ret;
> + }
> +
> + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> + IMX_SC_C_OFS_AUDIO, 0x80);
> + if (ret < 0) {
> + dev_err(sdev->dev, "Error system address offset of AUDIO\n");
> + return ret;
> + }
> +
> + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> + IMX_SC_C_OFS_PERIPH, 0x5A);
> + if (ret < 0) {
> + dev_err(sdev->dev, "Error system address offset of PERIPH %d\n",
> + ret);
> + return ret;
> + }
> +
> + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> + IMX_SC_C_OFS_IRQ, 0x51);
> + if (ret < 0) {
> + dev_err(sdev->dev, "Error system address offset of IRQ\n");
> + return ret;
> + }
> +
> + imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true,
> + RESET_VECTOR_VADDR);
> +
> + return 0;
> +}
> +
> +static int imx8_probe(struct snd_sof_dev *sdev)
> +{
> + struct imx8_priv *priv;
> + int i;
> + struct platform_device *pdev =
> + container_of(sdev->dev, struct platform_device, dev);
> + struct platform_device *ipc_dev;
> + struct resource *mmio;
> + int num_domains = 0;
> + u32 base, size;
> + int ret = 0;
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *res_node;
> + struct resource res;
nit-pick: can we reorder so that we have all counters last and a nice
xmas-tree shape.
> +
> + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + sdev->private = priv;
> + priv->dev = sdev->dev;
> + priv->sdev = sdev;
> +
> + ret = imx_scu_get_handle(&priv->sc_ipc);
> + if (ret) {
> + dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",
> + ret);
> + return ret;
> + }
> +
> + ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
> + PLATFORM_DEVID_NONE,
> + pdev, sizeof(*pdev));
> + if (IS_ERR(ipc_dev)) {
> + dev_err(sdev->dev, "Failed to register platform device\n");
> + return PTR_ERR(ipc_dev);
> + }
> +
> + priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev);
> + if (!priv->dsp_ipc)
> + return -EPROBE_DEFER;
> +
> + imx_dsp_set_data(priv->dsp_ipc, priv);
> + priv->dsp_ipc->ops = &dsp_ops;
> +
> + num_domains = of_count_phandle_with_args(np, "power-domains",
> + "#power-domain-cells");
> + for (i = 0; i < num_domains; i++) {
> + struct device *pd_dev;
> + struct device_link *link;
> +
> + pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i);
> + if (IS_ERR(pd_dev))
> + return PTR_ERR(pd_dev);
> +
> + link = device_link_add(&pdev->dev, pd_dev,
> + DL_FLAG_STATELESS |
> + DL_FLAG_PM_RUNTIME |
> + DL_FLAG_RPM_ACTIVE);
> + if (IS_ERR(link))
> + return PTR_ERR(link);
Question: is the error flow final? Wondering if we release all the
resources/memory/devices on errors?
Also are all the resources device-managed, I don't see a remove()?
> + }
> +
> + /* 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");
> + return -EINVAL;
> + }
> +
> + 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);
> + return -ENODEV;
> + }
> + sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
> +
> + res_node = of_parse_phandle(np, "memory-region", 0);
> + if (!res_node) {
> + dev_err(&pdev->dev, "failed to get memory region node\n");
> + return -ENODEV;
> + }
> + if (of_address_to_resource(res_node, 0, &res)) {
> + dev_err(&pdev->dev, "failed to get reserved region address\n");
> + return -EINVAL;
> + }
> +
> + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start,
> + res.end - res.start +
> + 1);
> + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
> + dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n",
> + base, size);
> + return -ENODEV;
> + }
> + sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
> +
> + return ret;
> +}
> +
> +/* on i.MX8 there is 1 to 1 match between type and BAR idx */
> +int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type)
> +{
> + return type;
> +}
> +
> +void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
> + struct snd_pcm_substream *substream,
> + void *p, size_t sz)
> +{
> + sof_mailbox_read(sdev, sdev->dsp_box.offset, p, sz);
> +}
> +
> +int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
> + struct snd_pcm_substream *substream,
> + const struct sof_ipc_pcm_params_reply *reply)
> +{
> + return 0;
> +}
> +
> +static struct snd_soc_dai_driver imx8_dai[] = {
> +{
> + .name = "esai-port",
> +},
> +};
> +
> +/* i.MX8 ops */
> +struct snd_sof_dsp_ops sof_imx8_ops = {
> + /* device init */
> + .probe = imx8_probe,
> +
> + /* DSP core boot */
> + .run = imx8_run,
> +
> + /* Block IO */
> + .block_read = sof_block_read,
> + .block_write = sof_block_write,
> +
> + /* ipc */
> + .send_msg = imx8_send_msg,
> + .fw_ready = imx8_fw_ready,
> +
> + .ipc_msg_data = imx8_ipc_msg_data,
> + .ipc_pcm_params = imx8_ipc_pcm_params,
> +
> + /* module loading */
> + .load_module = snd_sof_parse_module_memcpy,
> + .get_bar_index = imx8_get_bar_index,
> + /* firmware loading */
> + .load_firmware = snd_sof_load_firmware_memcpy,
> +
> + /* DAI drivers */
> + .drv = imx8_dai,
> + .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
> +};
> +EXPORT_SYMBOL(sof_imx8_ops);
> +
> +MODULE_LICENSE("Dual BSD/GPL");
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 15:01 ` [Sound-open-firmware] " Pierre-Louis Bossart
@ 2019-07-24 6:54 ` Daniel Baluta
2019-07-24 8:32 ` Lucas Stach
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-07-24 6:54 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Mark Rutland, Aisheng Dong, Peng Fan, Fabio Estevam, Anson Huang,
Devicetree List, Daniel Baluta, S.j. Wang, Marco Felsch,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Shawn Guo,
linux-arm-kernel, sound-open-firmware
On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> > diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile
> > new file mode 100644
> > index 000000000000..c69237971da5
> > --- /dev/null
> > +++ b/sound/soc/sof/imx/Makefile
> > @@ -0,0 +1,7 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > +
> > +ccflags-y += -DDEBUG
>
> this should be removed or in a separate patch.
All right.
>
>
> > +struct imx8_priv {
> > + struct device *dev;
> > + struct snd_sof_dev *sdev;
> > + struct imx_dsp_ipc *dsp_ipc;
> > + struct imx_sc_ipc *sc_ipc;
>
> maybe a comment to explain what 'sc' stands for?
Sure.
>
> > +};
> > +
> > +static void imx8_get_windows(struct snd_sof_dev *sdev)
> > +{
> > + struct sof_ipc_window_elem *elem;
> > + u32 outbox_offset = 0;
> > + u32 stream_offset = 0;
> > + u32 inbox_offset = 0;
> > + u32 outbox_size = 0;
> > + u32 stream_size = 0;
> > + u32 inbox_size = 0;
> > + int i;
> > +
> > + if (!sdev->info_window) {
> > + dev_err(sdev->dev, "error: have no window info\n");
> > + return;
> > + }
> > +
> > + for (i = 0; i < sdev->info_window->num_windows; i++) {
> > + elem = &sdev->info_window->window[i];
> > +
> > + switch (elem->type) {
> > + case SOF_IPC_REGION_UPBOX:
> > + inbox_offset = elem->offset + MBOX_OFFSET;
> > + inbox_size = elem->size;
> > + snd_sof_debugfs_io_item(sdev,
> > + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> > + + inbox_offset,
> > + elem->size, "inbox",
> > + SOF_DEBUGFS_ACCESS_D0_ONLY);
> > + break;
> > + case SOF_IPC_REGION_DOWNBOX:
> > + outbox_offset = elem->offset + MBOX_OFFSET;
> > + outbox_size = elem->size;
> > + snd_sof_debugfs_io_item(sdev,
> > + sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
> > + + elem->offset + MBOX_OFFSET,
> > + elem->size, "etrace",
> > + SOF_DEBUGFS_ACCESS_D0_ONLY);
> > + break;
> > + case SOF_IPC_REGION_DEBUG:
> > + snd_sof_debugfs_io_item(sdev,
> > + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> > + + elem->offset + MBOX_OFFSET,
> > + elem->size, "debug",
> > + SOF_DEBUGFS_ACCESS_D0_ONLY);
> > + break;
> > + case SOF_IPC_REGION_STREAM:
> > + stream_offset = elem->offset + MBOX_OFFSET;
> > + stream_size = elem->size;
> > + snd_sof_debugfs_io_item(sdev,
> > + sdev->bar[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[SOF_FW_BLK_TYPE_SRAM]
> > + + elem->offset + MBOX_OFFSET,
> > + elem->size, "regs",
> > + SOF_DEBUGFS_ACCESS_D0_ONLY);
> > + break;
> > + case SOF_IPC_REGION_EXCEPTION:
> > + sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET;
> > + snd_sof_debugfs_io_item(sdev,
> > + sdev->bar[SOF_FW_BLK_TYPE_SRAM]
> > + + elem->offset + MBOX_OFFSET,
> > + elem->size, "exception",
> > + SOF_DEBUGFS_ACCESS_D0_ONLY);
> > + break;
> > + default:
> > + dev_err(sdev->dev, "error: get illegal window info\n");
> > + return;
> > + }
> > + }
> > +
> > + if (outbox_size == 0 || inbox_size == 0) {
> > + dev_err(sdev->dev, "error: get illegal mailbox window\n");
> > + return;
> > + }
> > +
> > + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
> > + outbox_offset, outbox_size);
> > + sdev->stream_box.offset = stream_offset;
> > + sdev->stream_box.size = stream_size;
> > +
> > + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
> > + inbox_offset, inbox_size);
> > + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
> > + outbox_offset, outbox_size);
> > + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
> > + stream_offset, stream_size);
> > +}
>
> This looks 100% similar to Baytrail?
Yes!
>
> > +
> > +/*
> > + * IPC Firmware ready.
> > + */
> > +static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id)
> > +{
> > + struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
> > + u32 offset;
> > + int ret;
> > +
> > + /* mailbox must be on 4k boundary */
> > + offset = MBOX_OFFSET;
> > +
> > + dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",
> > + msg_id, offset);
> > +
> > + /* no need to re-check version/ABI for subsequent boots */
> > + if (!sdev->first_boot)
> > + return 0;
> > +
> > + /* copy data from the DSP FW ready offset */
> > + sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready,
> > + sizeof(*fw_ready));
> > + snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
> > + fw_ready->dspbox_size,
> > + fw_ready->hostbox_offset,
> > + fw_ready->hostbox_size);
> > +
> > + /* make sure ABI version is compatible */
> > + ret = snd_sof_ipc_valid(sdev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* now check for extended data */
> > + snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET +
> > + sizeof(struct sof_ipc_fw_ready));
> > +
> > + imx8_get_windows(sdev);
> > +
> > + return 0;
> > +}
>
> That code looks nearly similar to the baytrail one except for the last
> line, we should look into factoring this.
Yes, I got most of my inspiration from intel code.
>
> > +
> > +static void imx8_get_reply(struct snd_sof_dev *sdev)
> > +{
> > + struct snd_sof_ipc_msg *msg = sdev->msg;
> > + struct sof_ipc_reply reply;
> > + unsigned long flags;
> > + 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));
> > +
> > + spin_lock_irqsave(&sdev->ipc_lock, flags);
> > +
> > + 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;
> > +
> > + spin_unlock_irqrestore(&sdev->ipc_lock, flags);
>
> I don't see a spin_lock/unlock for the get_reply in the Intel code, is
> this necessary?
Hmm, you are right. I think I've used an older version of the intel code
where there a lock.
>
> > +}
> > +
> > +void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc)
> > +{
> > + struct imx8_priv *priv = imx_dsp_get_data(ipc);
> > +
> > + imx8_get_reply(priv->sdev);
> > + snd_sof_ipc_reply(priv->sdev, 0);
> > +}
> > +
> > +void imx_dsp_handle_request(struct imx_dsp_ipc *ipc)
> > +{
> > + struct imx8_priv *priv = imx_dsp_get_data(ipc);
> > +
> > + snd_sof_ipc_msgs_rx(priv->sdev);
> > +}
> > +
> > +struct imx_dsp_ops dsp_ops = {
> > + .handle_reply = imx_dsp_handle_reply,
> > + .handle_request = imx_dsp_handle_request,
> > +};
> > +
> > +static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg)
> > +{
> > + struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
> > +
> > + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
> > + msg->msg_size);
> > + imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * DSP control.
> > + */
> > +static int imx8_run(struct snd_sof_dev *sdev)
> > +{
> > + int ret;
> > + struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private;
> > +
> > + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> > + IMX_SC_C_OFS_SEL, 1);
> > + if (ret < 0) {
> > + dev_err(sdev->dev, "Error system address offset source select\n");
> > + return ret;
> > + }
> > +
> > + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> > + IMX_SC_C_OFS_AUDIO, 0x80);
> > + if (ret < 0) {
> > + dev_err(sdev->dev, "Error system address offset of AUDIO\n");
> > + return ret;
> > + }
> > +
> > + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> > + IMX_SC_C_OFS_PERIPH, 0x5A);
> > + if (ret < 0) {
> > + dev_err(sdev->dev, "Error system address offset of PERIPH %d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
> > + IMX_SC_C_OFS_IRQ, 0x51);
> > + if (ret < 0) {
> > + dev_err(sdev->dev, "Error system address offset of IRQ\n");
> > + return ret;
> > + }
> > +
> > + imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true,
> > + RESET_VECTOR_VADDR);
> > +
> > + return 0;
> > +}
> > +
> > +static int imx8_probe(struct snd_sof_dev *sdev)
> > +{
> > + struct imx8_priv *priv;
> > + int i;
> > + struct platform_device *pdev =
> > + container_of(sdev->dev, struct platform_device, dev);
> > + struct platform_device *ipc_dev;
> > + struct resource *mmio;
> > + int num_domains = 0;
> > + u32 base, size;
> > + int ret = 0;
> > + struct device_node *np = pdev->dev.of_node;
> > + struct device_node *res_node;
> > + struct resource res;
>
> nit-pick: can we reorder so that we have all counters last and a nice
> xmas-tree shape.
Ack.
>
> > +
> > + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
> > + if (!priv)
> > + return -ENOMEM;
> > +
> > + sdev->private = priv;
> > + priv->dev = sdev->dev;
> > + priv->sdev = sdev;
> > +
> > + ret = imx_scu_get_handle(&priv->sc_ipc);
> > + if (ret) {
> > + dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
> > + PLATFORM_DEVID_NONE,
> > + pdev, sizeof(*pdev));
> > + if (IS_ERR(ipc_dev)) {
> > + dev_err(sdev->dev, "Failed to register platform device\n");
> > + return PTR_ERR(ipc_dev);
> > + }
> > +
> > + priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev);
> > + if (!priv->dsp_ipc)
> > + return -EPROBE_DEFER;
> > +
> > + imx_dsp_set_data(priv->dsp_ipc, priv);
> > + priv->dsp_ipc->ops = &dsp_ops;
> > +
> > + num_domains = of_count_phandle_with_args(np, "power-domains",
> > + "#power-domain-cells");
> > + for (i = 0; i < num_domains; i++) {
> > + struct device *pd_dev;
> > + struct device_link *link;
> > +
> > + pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i);
> > + if (IS_ERR(pd_dev))
> > + return PTR_ERR(pd_dev);
> > +
> > + link = device_link_add(&pdev->dev, pd_dev,
> > + DL_FLAG_STATELESS |
> > + DL_FLAG_PM_RUNTIME |
> > + DL_FLAG_RPM_ACTIVE);
> > + if (IS_ERR(link))
> > + return PTR_ERR(link);
>
> Question: is the error flow final? Wondering if we release all the
> resources/memory/devices on errors?
Will check again. It seemed no need for resource freeing.
>
> Also are all the resources device-managed, I don't see a remove()?
Good catch for pm stuff. We mostly didn't care about remove because
drivers are always Y in our distribution.
Thanks Pierre for review, I will let some time for others to have a look and
send a new version.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-24 6:54 ` Daniel Baluta
@ 2019-07-24 8:32 ` Lucas Stach
2019-07-24 8:58 ` Daniel Baluta
0 siblings, 1 reply; 19+ messages in thread
From: Lucas Stach @ 2019-07-24 8:32 UTC (permalink / raw)
To: Daniel Baluta, Pierre-Louis Bossart
Cc: Mark Rutland, Aisheng Dong, Peng Fan, Fabio Estevam, Anson Huang,
Devicetree List, Daniel Baluta, S.j. Wang, Marco Felsch,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Shawn Guo,
linux-arm-kernel, sound-open-firmware
Hi Daniel,
Am Mittwoch, den 24.07.2019, 09:54 +0300 schrieb Daniel Baluta:
> On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart
[...]
>
> > Also are all the resources device-managed, I don't see a remove()?
>
> Good catch for pm stuff. We mostly didn't care about remove because
> drivers are always Y in our distribution.
Linux drivers need to be hotplug aware, even if they are not built as a
module. You can test things by manually unbinding the driver from the
device via sysfs.
Regards,
Lucas
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-24 8:32 ` Lucas Stach
@ 2019-07-24 8:58 ` Daniel Baluta
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Baluta @ 2019-07-24 8:58 UTC (permalink / raw)
To: Lucas Stach
Cc: Pierre-Louis Bossart, Mark Rutland, Aisheng Dong, Peng Fan,
Fabio Estevam, Anson Huang, Devicetree List, Daniel Baluta,
S.j. Wang, Marco Felsch, Linux Kernel Mailing List, Paul Olaru,
Rob Herring, dl-linux-imx, Pengutronix Kernel Team,
Leonard Crestez, Shawn Guo, linux-arm-kernel, sound-open-fir
On Wed, Jul 24, 2019 at 11:32 AM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> Hi Daniel,
>
> Am Mittwoch, den 24.07.2019, 09:54 +0300 schrieb Daniel Baluta:
> > On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart
> [...]
> >
> > > Also are all the resources device-managed, I don't see a remove()?
> >
> > Good catch for pm stuff. We mostly didn't care about remove because
> > drivers are always Y in our distribution.
>
> Linux drivers need to be hotplug aware, even if they are not built as a
> module. You can test things by manually unbinding the driver from the
> device via sysfs.
Agree. Will take this into consideration when sending next version!
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 2/5] ASoC: SOF: topology: Add dummy support for i.MX8 DAIs
2019-07-23 8:40 [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support Daniel Baluta
` (2 subsequent siblings)
4 siblings, 0 replies; 19+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:41 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
linux-kernel, linux-imx, shengjiu.wang, paul.olaru, robh+dt,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware, Daniel Baluta
Add dummy support for SAI/ESAI digital audio interface
IPs found on i.MX8 boards.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
This is also on review with SOF community here:
https://github.com/thesofproject/linux/pull/1048
include/sound/sof/dai.h | 2 ++
include/uapi/sound/sof/tokens.h | 8 ++++++++
sound/soc/sof/topology.c | 30 ++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+)
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h
index 3d174e20aa53..ec3b5c080537 100644
--- a/include/sound/sof/dai.h
+++ b/include/sound/sof/dai.h
@@ -50,6 +50,8 @@ enum sof_ipc_dai_type {
SOF_DAI_INTEL_DMIC, /**< Intel DMIC */
SOF_DAI_INTEL_HDA, /**< Intel HD/A */
SOF_DAI_INTEL_SOUNDWIRE, /**< Intel SoundWire */
+ SOF_DAI_IMX_SAI, /**< i.MX SAI */
+ SOF_DAI_IMX_ESAI, /**< i.MX ESAI */
};
/* general purpose DAI configuration */
diff --git a/include/uapi/sound/sof/tokens.h b/include/uapi/sound/sof/tokens.h
index dc1b27daaac6..347ce10bfd2d 100644
--- a/include/uapi/sound/sof/tokens.h
+++ b/include/uapi/sound/sof/tokens.h
@@ -105,4 +105,12 @@
/* for backward compatibility */
#define SOF_TKN_EFFECT_TYPE SOF_TKN_PROCESS_TYPE
+/* SAI */
+#define SOF_TKN_IMX_SAI_FIRST_TOKEN 1000
+/* TODO: Add SAI tokens */
+
+/* ESAI */
+#define SOF_TKN_IMX_ESAI_FIRST_TOKEN 1100
+/* TODO: Add ESAI tokens */
+
#endif
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
index 432ae343f960..b5399f520366 100644
--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -339,6 +339,8 @@ static const struct sof_dai_types sof_dais[] = {
{"SSP", SOF_DAI_INTEL_SSP},
{"HDA", SOF_DAI_INTEL_HDA},
{"DMIC", SOF_DAI_INTEL_DMIC},
+ {"SAI", SOF_DAI_IMX_SAI},
+ {"ESAI", SOF_DAI_IMX_ESAI},
};
static enum sof_ipc_dai_type find_dai(const char *name)
@@ -2457,6 +2459,26 @@ static int sof_link_ssp_load(struct snd_soc_component *scomp, int index,
return ret;
}
+static int sof_link_sai_load(struct snd_soc_component *scomp, int index,
+ struct snd_soc_dai_link *link,
+ struct snd_soc_tplg_link_config *cfg,
+ struct snd_soc_tplg_hw_config *hw_config,
+ struct sof_ipc_dai_config *config)
+{
+ /*TODO: Add implementation */
+ return 0;
+}
+
+static int sof_link_esai_load(struct snd_soc_component *scomp, int index,
+ struct snd_soc_dai_link *link,
+ struct snd_soc_tplg_link_config *cfg,
+ struct snd_soc_tplg_hw_config *hw_config,
+ struct sof_ipc_dai_config *config)
+{
+ /*TODO: Add implementation */
+ return 0;
+}
+
static int sof_link_dmic_load(struct snd_soc_component *scomp, int index,
struct snd_soc_dai_link *link,
struct snd_soc_tplg_link_config *cfg,
@@ -2781,6 +2803,14 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
ret = sof_link_hda_load(scomp, index, link, cfg, hw_config,
&config);
break;
+ case SOF_DAI_IMX_SAI:
+ ret = sof_link_sai_load(scomp, index, link, cfg, hw_config,
+ &config);
+ break;
+ case SOF_DAI_IMX_ESAI:
+ ret = sof_link_esai_load(scomp, index, link, cfg, hw_config,
+ &config);
+ break;
default:
dev_err(sdev->dev, "error: invalid DAI type %d\n", config.type);
ret = -EINVAL;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 8:40 [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 2/5] ASoC: SOF: topology: Add dummy support for i.MX8 DAIs Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
2019-07-23 15:11 ` [Sound-open-firmware] " Pierre-Louis Bossart
2019-07-23 8:41 ` [PATCH v2 4/5] arm64: dts: imx8qxp: Add DSP DT node Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support Daniel Baluta
4 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:41 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
linux-kernel, linux-imx, shengjiu.wang, paul.olaru, robh+dt,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware, Daniel Baluta
Add support for device tree based SOF DSP devices.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
This is also on review with SOF community here:
https://github.com/thesofproject/linux/pull/1048
sound/soc/sof/Kconfig | 9 +++
sound/soc/sof/Makefile | 3 +
sound/soc/sof/imx/Kconfig | 1 +
sound/soc/sof/sof-dt-dev.c | 159 +++++++++++++++++++++++++++++++++++++
4 files changed, 172 insertions(+)
create mode 100644 sound/soc/sof/sof-dt-dev.c
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
index 61b97fc55bb2..2aa3a1cdf60c 100644
--- a/sound/soc/sof/Kconfig
+++ b/sound/soc/sof/Kconfig
@@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
Say Y if you need this option
If unsure select "N".
+config SND_SOC_SOF_DT
+ tristate "SOF DT enumeration support"
+ select SND_SOC_SOF
+ select SND_SOC_SOF_OPTIONS
+ help
+ This adds support for Device Tree enumeration. This option is
+ required to enable i.MX8 devices.
+ Say Y if you need this option. If unsure select "N".
+
config SND_SOC_SOF_OPTIONS
tristate
help
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile
index 6e43d411ffef..8025d9cec4fb 100644
--- a/sound/soc/sof/Makefile
+++ b/sound/soc/sof/Makefile
@@ -5,6 +5,8 @@ snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
snd-sof-pci-objs := sof-pci-dev.o
snd-sof-acpi-objs := sof-acpi-dev.o
+snd-sof-dt-objs := sof-dt-dev.o
+
snd-sof-nocodec-objs := nocodec.o
obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o
@@ -12,6 +14,7 @@ obj-$(CONFIG_SND_SOC_SOF_NOCODEC) += snd-sof-nocodec.o
obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o
+obj-$(CONFIG_SND_SOC_SOF_DT) += sof-dt-dev.o
obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o
obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
index fff64a9970f0..fa35994a79c4 100644
--- a/sound/soc/sof/imx/Kconfig
+++ b/sound/soc/sof/imx/Kconfig
@@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8
tristate "SOF support for i.MX8"
+ select SND_SOC_SOF_DT
help
This adds support for Sound Open Firmware for NXP i.MX8 platforms
Say Y if you have such a device.
diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c
new file mode 100644
index 000000000000..31429bbb5c7e
--- /dev/null
+++ b/sound/soc/sof/sof-dt-dev.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// Copyright 2019 NXP
+//
+// Author: Daniel Baluta <daniel.baluta@nxp.com>
+//
+
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <sound/sof.h>
+
+#include "ops.h"
+
+extern struct snd_sof_dsp_ops sof_imx8_ops;
+
+static char *fw_path;
+module_param(fw_path, charp, 0444);
+MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
+
+static char *tplg_path;
+module_param(tplg_path, charp, 0444);
+MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
+
+/* platform specific devices */
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
+static struct sof_dev_desc sof_dt_imx8qxp_desc = {
+ .default_fw_path = "imx/sof",
+ .default_tplg_path = "imx/sof-tplg",
+ .nocodec_fw_filename = "sof-imx8.ri",
+ .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
+ .ops = &sof_imx8_ops,
+};
+#endif
+
+static const struct dev_pm_ops sof_dt_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
+ SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
+ NULL)
+};
+
+static void sof_dt_probe_complete(struct device *dev)
+{
+ /* allow runtime_pm */
+ pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_enable(dev);
+}
+
+static int sof_dt_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ const struct sof_dev_desc *desc;
+ /*TODO: create a generic snd_soc_xxx_mach */
+ struct snd_soc_acpi_mach *mach;
+ struct snd_sof_pdata *sof_pdata;
+ const struct snd_sof_dsp_ops *ops;
+ int ret;
+
+ dev_info(&pdev->dev, "DT DSP detected");
+
+ sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL);
+ if (!sof_pdata)
+ return -ENOMEM;
+
+ desc = device_get_match_data(dev);
+ if (!desc)
+ return -ENODEV;
+
+ /* get ops for platform */
+ ops = desc->ops;
+ if (!ops) {
+ dev_err(dev, "error: no matching DT descriptor ops\n");
+ return -ENODEV;
+ }
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_FORCE_NOCODEC_MODE)
+ /* force nocodec mode */
+ dev_warn(dev, "Force to use nocodec mode\n");
+ mach = devm_kzalloc(dev, sizeof(*mach), GFP_KERNEL);
+ if (!mach)
+ return -ENOMEM;
+ ret = sof_nocodec_setup(dev, sof_pdata, mach, desc, ops);
+ if (ret < 0)
+ return ret;
+#else
+ /* TODO: implement case where we actually have a codec */
+ return -ENODEV;
+#endif
+
+ if (mach)
+ mach->mach_params.platform = dev_name(dev);
+
+ sof_pdata->machine = mach;
+ sof_pdata->desc = desc;
+ sof_pdata->dev = &pdev->dev;
+ sof_pdata->platform = dev_name(dev);
+
+ /* alternate fw and tplg filenames */
+ if (fw_path)
+ sof_pdata->fw_filename_prefix = fw_path;
+ else
+ sof_pdata->fw_filename_prefix =
+ sof_pdata->desc->default_fw_path;
+ if (tplg_path)
+ sof_pdata->tplg_filename_prefix = tplg_path;
+ else
+ sof_pdata->tplg_filename_prefix =
+ sof_pdata->desc->default_tplg_path;
+
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
+ /* set callback to enable runtime_pm */
+ sof_pdata->sof_probe_complete = sof_dt_probe_complete;
+#endif
+ /* call sof helper for DSP hardware probe */
+ ret = snd_sof_device_probe(dev, sof_pdata);
+ if (ret) {
+ dev_err(dev, "error: failed to probe DSP hardware\n");
+ return ret;
+ }
+
+#if !IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
+ sof_dt_probe_complete(dev);
+#endif
+
+ return ret;
+}
+
+static int sof_dt_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(&pdev->dev);
+
+ /* call sof helper for DSP hardware remove */
+ snd_sof_device_remove(&pdev->dev);
+
+ return 0;
+}
+
+static const struct of_device_id sof_dt_ids[] = {
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
+ { .compatible = "fsl,imx8qxp-dsp", .data = &sof_dt_imx8qxp_desc},
+#endif
+ { }
+};
+MODULE_DEVICE_TABLE(of, sof_dt_ids);
+
+/* DT driver definition */
+static struct platform_driver snd_sof_dt_driver = {
+ .probe = sof_dt_probe,
+ .remove = sof_dt_remove,
+ .driver = {
+ .name = "sof-audio-dt",
+ .pm = &sof_dt_pm,
+ .of_match_table = sof_dt_ids
+ },
+};
+module_platform_driver(snd_sof_dt_driver);
+
+MODULE_LICENSE("Dual BSD/GPL");
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 8:41 ` [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support Daniel Baluta
@ 2019-07-23 15:11 ` Pierre-Louis Bossart
2019-07-24 7:04 ` Daniel Baluta
2019-08-07 15:29 ` Daniel Baluta
0 siblings, 2 replies; 19+ messages in thread
From: Pierre-Louis Bossart @ 2019-07-23 15:11 UTC (permalink / raw)
To: Daniel Baluta, m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
shengjiu.wang, linux-kernel, paul.olaru, robh+dt, linux-imx,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> index 61b97fc55bb2..2aa3a1cdf60c 100644
> --- a/sound/soc/sof/Kconfig
> +++ b/sound/soc/sof/Kconfig
> @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
> Say Y if you need this option
> If unsure select "N".
>
> +config SND_SOC_SOF_DT
> + tristate "SOF DT enumeration support"
> + select SND_SOC_SOF
> + select SND_SOC_SOF_OPTIONS
> + help
> + This adds support for Device Tree enumeration. This option is
> + required to enable i.MX8 devices.
> + Say Y if you need this option. If unsure select "N".
> +
[snip]
> diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> index fff64a9970f0..fa35994a79c4 100644
> --- a/sound/soc/sof/imx/Kconfig
> +++ b/sound/soc/sof/imx/Kconfig
> @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
>
> config SND_SOC_SOF_IMX8
> tristate "SOF support for i.MX8"
> + select SND_SOC_SOF_DT
This looks upside down. You should select SOF_DT first then include the
NXP stuff.
> help
> This adds support for Sound Open Firmware for NXP i.MX8 platforms
> Say Y if you have such a device.
> diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c
> new file mode 100644
> index 000000000000..31429bbb5c7e
> --- /dev/null
> +++ b/sound/soc/sof/sof-dt-dev.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> +//
> +// Copyright 2019 NXP
> +//
> +// Author: Daniel Baluta <daniel.baluta@nxp.com>
> +//
> +
> +#include <linux/firmware.h>
> +#include <linux/module.h>
> +#include <linux/pm_runtime.h>
> +#include <sound/sof.h>
> +
> +#include "ops.h"
> +
> +extern struct snd_sof_dsp_ops sof_imx8_ops;
> +
> +static char *fw_path;
> +module_param(fw_path, charp, 0444);
> +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
> +
> +static char *tplg_path;
> +module_param(tplg_path, charp, 0444);
> +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
> +
> +/* platform specific devices */
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
> +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
> + .default_fw_path = "imx/sof",
> + .default_tplg_path = "imx/sof-tplg",
> + .nocodec_fw_filename = "sof-imx8.ri",
> + .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> + .ops = &sof_imx8_ops,
> +};
> +#endif
> +
> +static const struct dev_pm_ops sof_dt_pm = {
> + SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
> + SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
> + NULL)
> +};
> +
> +static void sof_dt_probe_complete(struct device *dev)
> +{
> + /* allow runtime_pm */
> + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
> + pm_runtime_enable(dev);
> +}
> +
> +static int sof_dt_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + const struct sof_dev_desc *desc;
> + /*TODO: create a generic snd_soc_xxx_mach */
> + struct snd_soc_acpi_mach *mach;
I wonder if you really need to use the same structures. For Intel we get
absolutely zero info from the firmware so rely on an ACPI codec ID as a
key to find information on which firmware and topology to use, and which
machine driver to load. You could have all this information in a DT blob?
> + struct snd_sof_pdata *sof_pdata;
> + const struct snd_sof_dsp_ops *ops;
> + int ret;
> +
> + dev_info(&pdev->dev, "DT DSP detected");
> +
> + sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL);
> + if (!sof_pdata)
> + return -ENOMEM;
> +
> + desc = device_get_match_data(dev);
> + if (!desc)
> + return -ENODEV;
> +
> + /* get ops for platform */
> + ops = desc->ops;
> + if (!ops) {
> + dev_err(dev, "error: no matching DT descriptor ops\n");
> + return -ENODEV;
> + }
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_FORCE_NOCODEC_MODE)
> + /* force nocodec mode */
> + dev_warn(dev, "Force to use nocodec mode\n");
> + mach = devm_kzalloc(dev, sizeof(*mach), GFP_KERNEL);
> + if (!mach)
> + return -ENOMEM;
> + ret = sof_nocodec_setup(dev, sof_pdata, mach, desc, ops);
> + if (ret < 0)
> + return ret;
> +#else
> + /* TODO: implement case where we actually have a codec */
> + return -ENODEV;
> +#endif
> +
> + if (mach)
> + mach->mach_params.platform = dev_name(dev);
> +
> + sof_pdata->machine = mach;
> + sof_pdata->desc = desc;
> + sof_pdata->dev = &pdev->dev;
> + sof_pdata->platform = dev_name(dev);
> +
> + /* alternate fw and tplg filenames */
> + if (fw_path)
> + sof_pdata->fw_filename_prefix = fw_path;
> + else
> + sof_pdata->fw_filename_prefix =
> + sof_pdata->desc->default_fw_path;
> + if (tplg_path)
> + sof_pdata->tplg_filename_prefix = tplg_path;
> + else
> + sof_pdata->tplg_filename_prefix =
> + sof_pdata->desc->default_tplg_path;
> +
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
> + /* set callback to enable runtime_pm */
> + sof_pdata->sof_probe_complete = sof_dt_probe_complete;
> +#endif
> + /* call sof helper for DSP hardware probe */
> + ret = snd_sof_device_probe(dev, sof_pdata);
> + if (ret) {
> + dev_err(dev, "error: failed to probe DSP hardware\n");
> + return ret;
> + }
> +
> +#if !IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
> + sof_dt_probe_complete(dev);
> +#endif
> +
> + return ret;
> +}
> +
> +static int sof_dt_remove(struct platform_device *pdev)
> +{
> + pm_runtime_disable(&pdev->dev);
> +
> + /* call sof helper for DSP hardware remove */
> + snd_sof_device_remove(&pdev->dev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sof_dt_ids[] = {
> +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
> + { .compatible = "fsl,imx8qxp-dsp", .data = &sof_dt_imx8qxp_desc},
> +#endif
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, sof_dt_ids);
> +
> +/* DT driver definition */
> +static struct platform_driver snd_sof_dt_driver = {
> + .probe = sof_dt_probe,
> + .remove = sof_dt_remove,
> + .driver = {
> + .name = "sof-audio-dt",
> + .pm = &sof_dt_pm,
> + .of_match_table = sof_dt_ids
> + },
> +};
> +module_platform_driver(snd_sof_dt_driver);
> +
> +MODULE_LICENSE("Dual BSD/GPL");
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 15:11 ` [Sound-open-firmware] " Pierre-Louis Bossart
@ 2019-07-24 7:04 ` Daniel Baluta
2019-08-07 14:39 ` Daniel Baluta
2019-08-07 15:29 ` Daniel Baluta
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-07-24 7:04 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Mark Rutland, Aisheng Dong, Peng Fan, Fabio Estevam, Anson Huang,
Devicetree List, Daniel Baluta, S.j. Wang, Marco Felsch,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Shawn Guo,
linux-arm-kernel, sound-open-firmware
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> > index 61b97fc55bb2..2aa3a1cdf60c 100644
> > --- a/sound/soc/sof/Kconfig
> > +++ b/sound/soc/sof/Kconfig
> > @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
> > Say Y if you need this option
> > If unsure select "N".
> >
> > +config SND_SOC_SOF_DT
> > + tristate "SOF DT enumeration support"
> > + select SND_SOC_SOF
> > + select SND_SOC_SOF_OPTIONS
> > + help
> > + This adds support for Device Tree enumeration. This option is
> > + required to enable i.MX8 devices.
> > + Say Y if you need this option. If unsure select "N".
> > +
>
> [snip]
>
> > diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> > index fff64a9970f0..fa35994a79c4 100644
> > --- a/sound/soc/sof/imx/Kconfig
> > +++ b/sound/soc/sof/imx/Kconfig
> > @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
> >
> > config SND_SOC_SOF_IMX8
> > tristate "SOF support for i.MX8"
> > + select SND_SOC_SOF_DT
>
> This looks upside down. You should select SOF_DT first then include the
> NXP stuff.
>
> > help
> > This adds support for Sound Open Firmware for NXP i.MX8 platforms
> > Say Y if you have such a device.
> > diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c
> > new file mode 100644
> > index 000000000000..31429bbb5c7e
> > --- /dev/null
> > +++ b/sound/soc/sof/sof-dt-dev.c
> > @@ -0,0 +1,159 @@
> > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > +//
> > +// Copyright 2019 NXP
> > +//
> > +// Author: Daniel Baluta <daniel.baluta@nxp.com>
> > +//
> > +
> > +#include <linux/firmware.h>
> > +#include <linux/module.h>
> > +#include <linux/pm_runtime.h>
> > +#include <sound/sof.h>
> > +
> > +#include "ops.h"
> > +
> > +extern struct snd_sof_dsp_ops sof_imx8_ops;
> > +
> > +static char *fw_path;
> > +module_param(fw_path, charp, 0444);
> > +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
> > +
> > +static char *tplg_path;
> > +module_param(tplg_path, charp, 0444);
> > +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
> > +
> > +/* platform specific devices */
> > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
> > +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
> > + .default_fw_path = "imx/sof",
> > + .default_tplg_path = "imx/sof-tplg",
> > + .nocodec_fw_filename = "sof-imx8.ri",
> > + .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> > + .ops = &sof_imx8_ops,
> > +};
> > +#endif
> > +
> > +static const struct dev_pm_ops sof_dt_pm = {
> > + SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
> > + SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
> > + NULL)
> > +};
> > +
> > +static void sof_dt_probe_complete(struct device *dev)
> > +{
> > + /* allow runtime_pm */
> > + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
> > + pm_runtime_use_autosuspend(dev);
> > + pm_runtime_enable(dev);
> > +}
> > +
> > +static int sof_dt_probe(struct platform_device *pdev)
> > +{
> > + struct device *dev = &pdev->dev;
> > + const struct sof_dev_desc *desc;
> > + /*TODO: create a generic snd_soc_xxx_mach */
> > + struct snd_soc_acpi_mach *mach;
>
> I wonder if you really need to use the same structures. For Intel we get
> absolutely zero info from the firmware so rely on an ACPI codec ID as a
> key to find information on which firmware and topology to use, and which
> machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for
snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach {
union {
struct snd_soc_acpi_mach acpi_mach;
struct snd_soc_of_mach of_mach;
}
};
and then change the prototype of sof_nocodec_setup.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-24 7:04 ` Daniel Baluta
@ 2019-08-07 14:39 ` Daniel Baluta
2019-08-07 15:22 ` Pierre-Louis Bossart
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-08-07 14:39 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
On Wed, Jul 24, 2019 at 10:04 AM Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
> >
> >
> > > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> > > index 61b97fc55bb2..2aa3a1cdf60c 100644
> > > --- a/sound/soc/sof/Kconfig
> > > +++ b/sound/soc/sof/Kconfig
> > > @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
> > > Say Y if you need this option
> > > If unsure select "N".
> > >
> > > +config SND_SOC_SOF_DT
> > > + tristate "SOF DT enumeration support"
> > > + select SND_SOC_SOF
> > > + select SND_SOC_SOF_OPTIONS
> > > + help
> > > + This adds support for Device Tree enumeration. This option is
> > > + required to enable i.MX8 devices.
> > > + Say Y if you need this option. If unsure select "N".
> > > +
> >
> > [snip]
> >
> > > diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> > > index fff64a9970f0..fa35994a79c4 100644
> > > --- a/sound/soc/sof/imx/Kconfig
> > > +++ b/sound/soc/sof/imx/Kconfig
> > > @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
> > >
> > > config SND_SOC_SOF_IMX8
> > > tristate "SOF support for i.MX8"
> > > + select SND_SOC_SOF_DT
> >
> > This looks upside down. You should select SOF_DT first then include the
> > NXP stuff.
> >
> > > help
> > > This adds support for Sound Open Firmware for NXP i.MX8 platforms
> > > Say Y if you have such a device.
> > > diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c
> > > new file mode 100644
> > > index 000000000000..31429bbb5c7e
> > > --- /dev/null
> > > +++ b/sound/soc/sof/sof-dt-dev.c
> > > @@ -0,0 +1,159 @@
> > > +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> > > +//
> > > +// Copyright 2019 NXP
> > > +//
> > > +// Author: Daniel Baluta <daniel.baluta@nxp.com>
> > > +//
> > > +
> > > +#include <linux/firmware.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <sound/sof.h>
> > > +
> > > +#include "ops.h"
> > > +
> > > +extern struct snd_sof_dsp_ops sof_imx8_ops;
> > > +
> > > +static char *fw_path;
> > > +module_param(fw_path, charp, 0444);
> > > +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
> > > +
> > > +static char *tplg_path;
> > > +module_param(tplg_path, charp, 0444);
> > > +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
> > > +
> > > +/* platform specific devices */
> > > +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
> > > +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
> > > + .default_fw_path = "imx/sof",
> > > + .default_tplg_path = "imx/sof-tplg",
> > > + .nocodec_fw_filename = "sof-imx8.ri",
> > > + .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> > > + .ops = &sof_imx8_ops,
> > > +};
> > > +#endif
> > > +
> > > +static const struct dev_pm_ops sof_dt_pm = {
> > > + SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
> > > + SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
> > > + NULL)
> > > +};
> > > +
> > > +static void sof_dt_probe_complete(struct device *dev)
> > > +{
> > > + /* allow runtime_pm */
> > > + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
> > > + pm_runtime_use_autosuspend(dev);
> > > + pm_runtime_enable(dev);
> > > +}
> > > +
> > > +static int sof_dt_probe(struct platform_device *pdev)
> > > +{
> > > + struct device *dev = &pdev->dev;
> > > + const struct sof_dev_desc *desc;
> > > + /*TODO: create a generic snd_soc_xxx_mach */
> > > + struct snd_soc_acpi_mach *mach;
> >
> > I wonder if you really need to use the same structures. For Intel we get
> > absolutely zero info from the firmware so rely on an ACPI codec ID as a
> > key to find information on which firmware and topology to use, and which
> > machine driver to load. You could have all this information in a DT blob?
>
> Yes. I see your point. I will still need to make a generic structure for
> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
>
> Maybe something like this:
>
> struct snd_soc_mach {
> union {
> struct snd_soc_acpi_mach acpi_mach;
> struct snd_soc_of_mach of_mach;
> }
> };
>
> and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are
now and later switch to
generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only
implemented no codec
version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx
version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later
we will get options to
read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on
i.MX. This will be used
to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = {
» .default_fw_path = "imx/sof",
» .default_tplg_path = "imx/sof-tplg",
» .nocodec_fw_filename = "sof-imx8.ri",
» .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
» .ops = &sof_imx8_ops,
};
For the moment we will only use the default values.
thanks,
Daniel.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-08-07 14:39 ` Daniel Baluta
@ 2019-08-07 15:22 ` Pierre-Louis Bossart
2019-08-07 15:28 ` Daniel Baluta
0 siblings, 1 reply; 19+ messages in thread
From: Pierre-Louis Bossart @ 2019-08-07 15:22 UTC (permalink / raw)
To: Daniel Baluta
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
>>>> +static int sof_dt_probe(struct platform_device *pdev)
>>>> +{
>>>> + struct device *dev = &pdev->dev;
>>>> + const struct sof_dev_desc *desc;
>>>> + /*TODO: create a generic snd_soc_xxx_mach */
>>>> + struct snd_soc_acpi_mach *mach;
>>>
>>> I wonder if you really need to use the same structures. For Intel we get
>>> absolutely zero info from the firmware so rely on an ACPI codec ID as a
>>> key to find information on which firmware and topology to use, and which
>>> machine driver to load. You could have all this information in a DT blob?
>>
>> Yes. I see your point. I will still need to make a generic structure for
>> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
>>
>> Maybe something like this:
>>
>> struct snd_soc_mach {
>> union {
>> struct snd_soc_acpi_mach acpi_mach;
>> struct snd_soc_of_mach of_mach;
>> }
>> };
>>
>> and then change the prototype of sof_nocodec_setup.
>
> Hi Pierre,
>
> I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
> with a generic snd_soc_mach is not trivial task.
>
> I wonder if it is acceptable to get the initial patches as they are
> now and later switch to
> generic ACPI/OF abstraction.
>
> Asking this because for the moment on the i.MX side I have only
> implemented no codec
> version and we don't probe any of the machine drivers we have.
>
> So, there is this only one member of snd_soc_acpi_mach that imx
> version is making use of:
>
> mach->drv_name = "sof-nocodec";
>
> That's all.
>
> I think the change as it is now is very clean and non-intrusive. Later
> we will get options to
> read firmware name and stuff from DT.
>
> Anyhow, I don't think we can get rid of snd_dev_desc structure on
> i.MX. This will be used
> to store the information read from DT:
>
> static struct sof_dev_desc sof_of_imx8qxp_desc = {
> » .default_fw_path = "imx/sof",
> » .default_tplg_path = "imx/sof-tplg",
> » .nocodec_fw_filename = "sof-imx8.ri",
> » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> » .ops = &sof_imx8_ops,
> };
>
> For the moment we will only use the default values.
Yes, that's fine for now. If you don't have a real machine driver then
there's nothing urgent to change.
Is the new version on GitHub?
Thanks
-Pierre
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-08-07 15:22 ` Pierre-Louis Bossart
@ 2019-08-07 15:28 ` Daniel Baluta
2019-08-08 6:27 ` Daniel Baluta
0 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-08-07 15:28 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
On Wed, Aug 7, 2019 at 6:22 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> >>>> +static int sof_dt_probe(struct platform_device *pdev)
> >>>> +{
> >>>> + struct device *dev = &pdev->dev;
> >>>> + const struct sof_dev_desc *desc;
> >>>> + /*TODO: create a generic snd_soc_xxx_mach */
> >>>> + struct snd_soc_acpi_mach *mach;
> >>>
> >>> I wonder if you really need to use the same structures. For Intel we get
> >>> absolutely zero info from the firmware so rely on an ACPI codec ID as a
> >>> key to find information on which firmware and topology to use, and which
> >>> machine driver to load. You could have all this information in a DT blob?
> >>
> >> Yes. I see your point. I will still need to make a generic structure for
> >> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
> >>
> >> Maybe something like this:
> >>
> >> struct snd_soc_mach {
> >> union {
> >> struct snd_soc_acpi_mach acpi_mach;
> >> struct snd_soc_of_mach of_mach;
> >> }
> >> };
> >>
> >> and then change the prototype of sof_nocodec_setup.
> >
> > Hi Pierre,
> >
> > I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
> > with a generic snd_soc_mach is not trivial task.
> >
> > I wonder if it is acceptable to get the initial patches as they are
> > now and later switch to
> > generic ACPI/OF abstraction.
> >
> > Asking this because for the moment on the i.MX side I have only
> > implemented no codec
> > version and we don't probe any of the machine drivers we have.
> >
> > So, there is this only one member of snd_soc_acpi_mach that imx
> > version is making use of:
> >
> > mach->drv_name = "sof-nocodec";
> >
> > That's all.
> >
> > I think the change as it is now is very clean and non-intrusive. Later
> > we will get options to
> > read firmware name and stuff from DT.
> >
> > Anyhow, I don't think we can get rid of snd_dev_desc structure on
> > i.MX. This will be used
> > to store the information read from DT:
> >
> > static struct sof_dev_desc sof_of_imx8qxp_desc = {
> > » .default_fw_path = "imx/sof",
> > » .default_tplg_path = "imx/sof-tplg",
> > » .nocodec_fw_filename = "sof-imx8.ri",
> > » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> > » .ops = &sof_imx8_ops,
> > };
> >
> > For the moment we will only use the default values.
>
> Yes, that's fine for now. If you don't have a real machine driver then
> there's nothing urgent to change.
>
> Is the new version on GitHub?
Not yet, will push it today and ping you.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-08-07 15:28 ` Daniel Baluta
@ 2019-08-08 6:27 ` Daniel Baluta
0 siblings, 0 replies; 19+ messages in thread
From: Daniel Baluta @ 2019-08-08 6:27 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Mark Rutland, Aisheng Dong, Peng Fan, Fabio Estevam, Anson Huang,
Devicetree List, Daniel Baluta, S.j. Wang, Marco Felsch,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Shawn Guo,
linux-arm-kernel, sound-open-firmware
On Wed, Aug 7, 2019 at 6:28 PM Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> On Wed, Aug 7, 2019 at 6:22 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
> >
> >
> > >>>> +static int sof_dt_probe(struct platform_device *pdev)
> > >>>> +{
> > >>>> + struct device *dev = &pdev->dev;
> > >>>> + const struct sof_dev_desc *desc;
> > >>>> + /*TODO: create a generic snd_soc_xxx_mach */
> > >>>> + struct snd_soc_acpi_mach *mach;
> > >>>
> > >>> I wonder if you really need to use the same structures. For Intel we get
> > >>> absolutely zero info from the firmware so rely on an ACPI codec ID as a
> > >>> key to find information on which firmware and topology to use, and which
> > >>> machine driver to load. You could have all this information in a DT blob?
> > >>
> > >> Yes. I see your point. I will still need to make a generic structure for
> > >> snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
> > >>
> > >> Maybe something like this:
> > >>
> > >> struct snd_soc_mach {
> > >> union {
> > >> struct snd_soc_acpi_mach acpi_mach;
> > >> struct snd_soc_of_mach of_mach;
> > >> }
> > >> };
> > >>
> > >> and then change the prototype of sof_nocodec_setup.
> > >
> > > Hi Pierre,
> > >
> > > I fixed all the comments except the one above. Replacing snd_soc_acpi_mach
> > > with a generic snd_soc_mach is not trivial task.
> > >
> > > I wonder if it is acceptable to get the initial patches as they are
> > > now and later switch to
> > > generic ACPI/OF abstraction.
> > >
> > > Asking this because for the moment on the i.MX side I have only
> > > implemented no codec
> > > version and we don't probe any of the machine drivers we have.
> > >
> > > So, there is this only one member of snd_soc_acpi_mach that imx
> > > version is making use of:
> > >
> > > mach->drv_name = "sof-nocodec";
> > >
> > > That's all.
> > >
> > > I think the change as it is now is very clean and non-intrusive. Later
> > > we will get options to
> > > read firmware name and stuff from DT.
> > >
> > > Anyhow, I don't think we can get rid of snd_dev_desc structure on
> > > i.MX. This will be used
> > > to store the information read from DT:
> > >
> > > static struct sof_dev_desc sof_of_imx8qxp_desc = {
> > > » .default_fw_path = "imx/sof",
> > > » .default_tplg_path = "imx/sof-tplg",
> > > » .nocodec_fw_filename = "sof-imx8.ri",
> > > » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
> > > » .ops = &sof_imx8_ops,
> > > };
> > >
> > > For the moment we will only use the default values.
> >
> > Yes, that's fine for now. If you don't have a real machine driver then
> > there's nothing urgent to change.
> >
> > Is the new version on GitHub?
>
> Not yet, will push it today and ping you.
PR updated: https://github.com/thesofproject/linux/pull/1048
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 15:11 ` [Sound-open-firmware] " Pierre-Louis Bossart
2019-07-24 7:04 ` Daniel Baluta
@ 2019-08-07 15:29 ` Daniel Baluta
2019-08-07 16:01 ` Pierre-Louis Bossart
1 sibling, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-08-07 15:29 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Daniel Baluta, Marco Felsch, Shawn Guo, Mark Rutland,
Aisheng Dong, Peng Fan, Anson Huang, Devicetree List, S.j. Wang,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Leonard Crestez, Fabio Estevam,
linux-arm-kernel, sound-open-firmware
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart
<pierre-louis.bossart@linux.intel.com> wrote:
>
>
> > diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
> > index 61b97fc55bb2..2aa3a1cdf60c 100644
> > --- a/sound/soc/sof/Kconfig
> > +++ b/sound/soc/sof/Kconfig
> > @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
> > Say Y if you need this option
> > If unsure select "N".
> >
> > +config SND_SOC_SOF_DT
> > + tristate "SOF DT enumeration support"
> > + select SND_SOC_SOF
> > + select SND_SOC_SOF_OPTIONS
> > + help
> > + This adds support for Device Tree enumeration. This option is
> > + required to enable i.MX8 devices.
> > + Say Y if you need this option. If unsure select "N".
> > +
>
> [snip]
>
> > diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
> > index fff64a9970f0..fa35994a79c4 100644
> > --- a/sound/soc/sof/imx/Kconfig
> > +++ b/sound/soc/sof/imx/Kconfig
> > @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
> >
> > config SND_SOC_SOF_IMX8
> > tristate "SOF support for i.MX8"
> > + select SND_SOC_SOF_DT
>
> This looks upside down. You should select SOF_DT first then include the
> NXP stuff.
One more thing: So this should be 'depends on SND_SOC_SOF_DT' right?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-08-07 15:29 ` Daniel Baluta
@ 2019-08-07 16:01 ` Pierre-Louis Bossart
0 siblings, 0 replies; 19+ messages in thread
From: Pierre-Louis Bossart @ 2019-08-07 16:01 UTC (permalink / raw)
To: Daniel Baluta
Cc: Mark Rutland, Aisheng Dong, Peng Fan, Anson Huang,
Devicetree List, Daniel Baluta, S.j. Wang, Marco Felsch,
Linux Kernel Mailing List, Paul Olaru, Rob Herring, dl-linux-imx,
Pengutronix Kernel Team, Shawn Guo, Leonard Crestez,
Fabio Estevam, linux-arm-kernel, sound-open-firmware
On 8/7/19 10:29 AM, Daniel Baluta wrote:
> On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart
> <pierre-louis.bossart@linux.intel.com> wrote:
>>
>>
>>> diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig
>>> index 61b97fc55bb2..2aa3a1cdf60c 100644
>>> --- a/sound/soc/sof/Kconfig
>>> +++ b/sound/soc/sof/Kconfig
>>> @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI
>>> Say Y if you need this option
>>> If unsure select "N".
>>>
>>> +config SND_SOC_SOF_DT
>>> + tristate "SOF DT enumeration support"
>>> + select SND_SOC_SOF
>>> + select SND_SOC_SOF_OPTIONS
>>> + help
>>> + This adds support for Device Tree enumeration. This option is
>>> + required to enable i.MX8 devices.
>>> + Say Y if you need this option. If unsure select "N".
>>> +
>>
>> [snip]
>>
>>> diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig
>>> index fff64a9970f0..fa35994a79c4 100644
>>> --- a/sound/soc/sof/imx/Kconfig
>>> +++ b/sound/soc/sof/imx/Kconfig
>>> @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
>>>
>>> config SND_SOC_SOF_IMX8
>>> tristate "SOF support for i.MX8"
>>> + select SND_SOC_SOF_DT
>>
>> This looks upside down. You should select SOF_DT first then include the
>> NXP stuff.
>
> One more thing: So this should be 'depends on SND_SOC_SOF_DT' right?
I would do this:
config SND_SOC_SOF_DT
tristate "SOF DT enumeration support"
depends on OF # or whatever the top-level DT dependency is
select SND_SOC_SOF
select SND_SOC_SOF_OPTIONS
config SND_SOC_SOF_IMX_TOPLEVEL
bool "SOF support for NXP i.MX audio DSPs"
depends on ARM64 && SND_SOC_SOF_DT || COMPILE_TEST
if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8
tristate "SOF support for i.MX8"
In other words push the dependencies at a higher level.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v2 4/5] arm64: dts: imx8qxp: Add DSP DT node
2019-07-23 8:40 [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver Daniel Baluta
` (2 preceding siblings ...)
2019-07-23 8:41 ` [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
2019-07-23 8:41 ` [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support Daniel Baluta
4 siblings, 0 replies; 19+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:41 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
linux-kernel, linux-imx, shengjiu.wang, paul.olaru, robh+dt,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware, Daniel Baluta
This includes DSP reserved memory, ADMA DSP device and DSP MU
communication channels description.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 +++
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 32 +++++++++++++++++++
2 files changed, 36 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index bfdada2db176..19468058e6ae 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -230,3 +230,7 @@
>;
};
};
+
+&adma_dsp {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 05fa0b7f36bb..b6c408fb2b7f 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -113,6 +113,17 @@
interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
};
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ dsp_reserved: dsp@92400000 {
+ reg = <0 0x92400000 0 0x2000000>;
+ no-map;
+ };
+ };
+
pmu {
compatible = "arm,armv8-pmuv3";
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
@@ -204,6 +215,27 @@
#clock-cells = <1>;
};
+ adma_dsp: dsp@596e8000 {
+ compatible = "fsl,imx8qxp-dsp";
+ reg = <0x596e8000 0x88000>;
+ clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>,
+ <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>,
+ <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>;
+ clock-names = "ipg", "ocram", "core";
+ power-domains = <&pd IMX_SC_R_MU_13A>,
+ <&pd IMX_SC_R_MU_13B>,
+ <&pd IMX_SC_R_DSP>,
+ <&pd IMX_SC_R_DSP_RAM>;
+ mbox-names = "txdb0", "txdb1",
+ "rxdb0", "rxdb1";
+ mboxes = <&lsio_mu13 2 0>,
+ <&lsio_mu13 2 1>,
+ <&lsio_mu13 3 0>,
+ <&lsio_mu13 3 1>;
+ memory-region = <&dsp_reserved>;
+ status = "disabled";
+ };
+
adma_lpuart0: serial@5a060000 {
compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart";
reg = <0x5a060000 0x1000>;
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
2019-07-23 8:40 [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver Daniel Baluta
` (3 preceding siblings ...)
2019-07-23 8:41 ` [PATCH v2 4/5] arm64: dts: imx8qxp: Add DSP DT node Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
2019-07-23 22:32 ` Rob Herring
4 siblings, 1 reply; 19+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:41 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
linux-kernel, linux-imx, shengjiu.wang, paul.olaru, robh+dt,
kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware, Daniel Baluta
This describes the DSP device tree node.
Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
.../devicetree/bindings/dsp/fsl,dsp.yaml | 87 +++++++++++++++++++
1 file changed, 87 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
new file mode 100644
index 000000000000..d112486eda0e
--- /dev/null
+++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
@@ -0,0 +1,87 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/freescale/fsl,dsp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX8 DSP core
+
+maintainers:
+ - Daniel Baluta <daniel.baluta@nxp.com>
+
+description: |
+ Some boards from i.MX8 family contain a DSP core used for
+ advanced pre- and post- audio processing.
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx8qxp-dsp
+
+ reg:
+ description: Should contain register location and length
+
+ clocks:
+ items:
+ - description: ipg clock
+ - description: ocram clock
+ - description: core clock
+
+ clock-names:
+ items:
+ - const: ipg
+ - const: ocram
+ - const: core
+
+ power-domains:
+ description:
+ List of phandle and PM domain specifier as documented in
+ Documentation/devicetree/bindings/power/power_domain.txt
+ maxItems: 4
+ mboxes:
+ description:
+ List of <&phandle type channel> - 2 channels for TXDB, 2 channels for RXDB
+ (see mailbox/fsl,mu.txt)
+ maxItems: 4
+
+ mbox-names:
+ items:
+ - const: txdb0
+ - const: txdb1
+ - const: rxdb0
+ - const: rxdb1
+
+ memory-region:
+ description:
+ phandle to a node describing reserved memory (System RAM memory)
+ used by DSP (see bindings/reserved-memory/reserved-memory.txt)
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - power-domains
+ - mboxes
+ - mbox-names
+ - memory-region
+
+examples:
+ - |
+ #include <dt-bindings/firmware/imx/rsrc.h>
+ #include <dt-bindings/clock/imx8-clock.h>
+ dsp@596e8000 {
+ compatbile = "fsl,imx8qxp-dsp";
+ reg = <0x596e8000 0x88000>;
+ clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>,
+ <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>,
+ <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>;
+ clock-names = "ipg", "ocram", "core";
+ power-domains = <&pd IMX_SC_R_MU_13A>,
+ <&pd IMX_SC_R_MU_13B>,
+ <&pd IMX_SC_R_DSP>,
+ <&pd IMX_SC_R_DSP_RAM>;
+ mbox-names = "txdb0", "txdb1", "rxdb0", "rxdb1";
+ mboxes = <&lsio_mu13 2 0>, <&lsio_mu13 2 1>, <&lsio_mu13 3 0>, <&lsio_mu13 3 1>;
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
2019-07-23 8:41 ` [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support Daniel Baluta
@ 2019-07-23 22:32 ` Rob Herring
0 siblings, 0 replies; 19+ messages in thread
From: Rob Herring @ 2019-07-23 22:32 UTC (permalink / raw)
To: Daniel Baluta
Cc: Marco Felsch, Shawn Guo, Mark Rutland, Dong Aisheng, Peng Fan,
Anson Huang, devicetree, linux-kernel, NXP Linux Team, S.j. Wang,
paul.olaru, Sascha Hauer, Leonard Crestez, Fabio Estevam,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
sound-open-firmware
On Tue, Jul 23, 2019 at 2:41 AM Daniel Baluta <daniel.baluta@nxp.com> wrote:
>
> This describes the DSP device tree node.
>
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
> .../devicetree/bindings/dsp/fsl,dsp.yaml | 87 +++++++++++++++++++
> 1 file changed, 87 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
>
> diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
> new file mode 100644
> index 000000000000..d112486eda0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
> @@ -0,0 +1,87 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/freescale/fsl,dsp.yaml#
This needs updating to match the path.
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX8 DSP core
> +
> +maintainers:
> + - Daniel Baluta <daniel.baluta@nxp.com>
> +
> +description: |
> + Some boards from i.MX8 family contain a DSP core used for
> + advanced pre- and post- audio processing.
> +
> +properties:
> + compatible:
> + enum:
> + - fsl,imx8qxp-dsp
> +
> + reg:
> + description: Should contain register location and length
> +
> + clocks:
> + items:
> + - description: ipg clock
> + - description: ocram clock
> + - description: core clock
> +
> + clock-names:
> + items:
> + - const: ipg
> + - const: ocram
> + - const: core
> +
> + power-domains:
> + description:
> + List of phandle and PM domain specifier as documented in
> + Documentation/devicetree/bindings/power/power_domain.txt
> + maxItems: 4
Need a blank line here.
With those 2 fixes:
Reviewed-by: Rob Herring <robh@kernel.org>
> + mboxes:
> + description:
> + List of <&phandle type channel> - 2 channels for TXDB, 2 channels for RXDB
> + (see mailbox/fsl,mu.txt)
> + maxItems: 4
> +
> + mbox-names:
> + items:
> + - const: txdb0
> + - const: txdb1
> + - const: rxdb0
> + - const: rxdb1
> +
> + memory-region:
> + description:
> + phandle to a node describing reserved memory (System RAM memory)
> + used by DSP (see bindings/reserved-memory/reserved-memory.txt)
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - power-domains
> + - mboxes
> + - mbox-names
> + - memory-region
> +
> +examples:
> + - |
> + #include <dt-bindings/firmware/imx/rsrc.h>
> + #include <dt-bindings/clock/imx8-clock.h>
> + dsp@596e8000 {
> + compatbile = "fsl,imx8qxp-dsp";
> + reg = <0x596e8000 0x88000>;
> + clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>,
> + <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>,
> + <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>;
> + clock-names = "ipg", "ocram", "core";
> + power-domains = <&pd IMX_SC_R_MU_13A>,
> + <&pd IMX_SC_R_MU_13B>,
> + <&pd IMX_SC_R_DSP>,
> + <&pd IMX_SC_R_DSP_RAM>;
> + mbox-names = "txdb0", "txdb1", "rxdb0", "rxdb1";
> + mboxes = <&lsio_mu13 2 0>, <&lsio_mu13 2 1>, <&lsio_mu13 3 0>, <&lsio_mu13 3 1>;
> + };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 19+ messages in thread