* [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver
@ 2019-07-23 8:40 ` Daniel Baluta
0 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-07-23 8:40 UTC (permalink / raw)
To: m.felsch, shawnguo
Cc: mark.rutland, aisheng.dong, peng.fan, anson.huang, devicetree,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
i.MX8QXP boards feature an Hifi4 DSP from Tensilica.
This patch series aims on adding the DT node describing the DSP,
but it also contains the Linux SOF DSP driver that will use the DT node
for easier review.
Note that we switched to the new yaml format for bindings documentation.
The DSP will run SOF Firmware [1]. Patches 1,2,3 are adding support
for Linux DSP driver are already sent for review to SOF folks [2]
[1] https://github.com/thesofproject/sof
[2] https://github.com/thesofproject/linux/pull/1048/commits
Changes since v1:
- removed 'clk: imx8: Add DSP related clocks' as it was already
applied by Shawn
- add patches adding support for Linux DSP driver to make things
clear for review
- add maxItems property for PM in DT bindings doc
Daniel Baluta (5):
ASoC: SOF: imx: Add i.MX8 HW support
ASoC: SOF: topology: Add dummy support for i.MX8 DAIs
ASoC: SOF: Add DT DSP device support
arm64: dts: imx8qxp: Add DSP DT node
dt-bindings: dsp: fsl: Add DSP core binding support
.../devicetree/bindings/dsp/fsl,dsp.yaml | 87 ++++
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 +
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 32 ++
include/sound/sof/dai.h | 2 +
include/uapi/sound/sof/tokens.h | 8 +
sound/soc/sof/Kconfig | 10 +
sound/soc/sof/Makefile | 4 +
sound/soc/sof/imx/Kconfig | 21 +
sound/soc/sof/imx/Makefile | 7 +
sound/soc/sof/imx/imx8.c | 464 ++++++++++++++++++
sound/soc/sof/sof-dt-dev.c | 159 ++++++
sound/soc/sof/topology.c | 30 ++
12 files changed, 828 insertions(+)
create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
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
create mode 100644 sound/soc/sof/sof-dt-dev.c
--
2.17.1
_______________________________________________
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] 43+ messages in thread
* [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 8:40 ` Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
@ 2019-07-23 8:41 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 8:41 ` Daniel Baluta
@ 2019-07-23 15:01 ` Pierre-Louis Bossart
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
@ 2019-07-23 15:01 ` Pierre-Louis Bossart
0 siblings, 0 replies; 43+ 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");
>
_______________________________________________
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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
2019-07-23 15:01 ` Pierre-Louis Bossart
(?)
@ 2019-07-24 6:54 ` Daniel Baluta
-1 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-07-24 6:54 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: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] 43+ 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
0 siblings, 0 replies; 43+ 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.
_______________________________________________
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] 43+ 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
0 siblings, 0 replies; 43+ 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] 43+ 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
-1 siblings, 0 replies; 43+ 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] 43+ 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
0 siblings, 0 replies; 43+ 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, 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
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
_______________________________________________
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] 43+ 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
-1 siblings, 0 replies; 43+ 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-firmware
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] 43+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
@ 2019-07-24 8:58 ` Daniel Baluta
0 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-07-24 8:58 UTC (permalink / raw)
To: Lucas Stach
Cc: Mark Rutland, Aisheng Dong, Peng Fan, sound-open-firmware,
Anson Huang, Devicetree List, Daniel Baluta, S.j. Wang,
Pierre-Louis Bossart, Marco Felsch, Paul Olaru, Rob Herring,
dl-linux-imx, Pengutronix Kernel Team, Shawn Guo,
Leonard Crestez, Fabio Estevam, Linux Kernel Mailing List,
linux-arm-kernel
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!
_______________________________________________
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] 43+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 1/5] ASoC: SOF: imx: Add i.MX8 HW support
@ 2019-07-24 8:58 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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] 43+ messages in thread
* [PATCH v2 2/5] ASoC: SOF: topology: Add dummy support for i.MX8 DAIs
2019-07-23 8:40 ` Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* [PATCH v2 2/5] ASoC: SOF: topology: Add dummy support for i.MX8 DAIs
@ 2019-07-23 8:41 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 8:40 ` Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
@ 2019-07-23 8:41 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 8:41 ` Daniel Baluta
@ 2019-07-23 15:11 ` Pierre-Louis Bossart
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
@ 2019-07-23 15:11 ` Pierre-Louis Bossart
0 siblings, 0 replies; 43+ 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");
>
_______________________________________________
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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 15:11 ` Pierre-Louis Bossart
(?)
@ 2019-07-24 7:04 ` Daniel Baluta
-1 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-07-24 7:04 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.
>
> > 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] 43+ 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
0 siblings, 0 replies; 43+ 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.
_______________________________________________
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] 43+ 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
0 siblings, 0 replies; 43+ 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] 43+ 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
-1 siblings, 0 replies; 43+ 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] 43+ 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
0 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-08-07 14:39 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, 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.
_______________________________________________
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] 43+ 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
-1 siblings, 0 replies; 43+ 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] 43+ 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
0 siblings, 0 replies; 43+ messages in thread
From: Pierre-Louis Bossart @ 2019-08-07 15:22 UTC (permalink / raw)
To: Daniel Baluta
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
>>>> +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
_______________________________________________
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] 43+ 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
-1 siblings, 0 replies; 43+ 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] 43+ 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
0 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-08-07 15:28 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: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.
_______________________________________________
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] 43+ 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
-1 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-08-08 6:27 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: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
^ permalink raw reply [flat|nested] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
@ 2019-08-08 6:27 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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] 43+ messages in thread
* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
2019-07-23 15:11 ` Pierre-Louis Bossart
@ 2019-08-07 15:29 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ 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
0 siblings, 0 replies; 43+ messages in thread
From: Daniel Baluta @ 2019-08-07 15:29 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.
One more thing: So this should be 'depends on SND_SOC_SOF_DT' right?
_______________________________________________
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] 43+ 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
-1 siblings, 0 replies; 43+ 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, 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 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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
@ 2019-08-07 16:01 ` Pierre-Louis Bossart
0 siblings, 0 replies; 43+ 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.
_______________________________________________
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] 43+ messages in thread* Re: [Sound-open-firmware] [PATCH v2 3/5] ASoC: SOF: Add DT DSP device support
@ 2019-08-07 16:01 ` Pierre-Louis Bossart
0 siblings, 0 replies; 43+ 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] 43+ messages in thread
* [PATCH v2 4/5] arm64: dts: imx8qxp: Add DSP DT node
2019-07-23 8:40 ` Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* [PATCH v2 4/5] arm64: dts: imx8qxp: Add DSP DT node
@ 2019-07-23 8:41 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
2019-07-23 8:40 ` Daniel Baluta
@ 2019-07-23 8:41 ` Daniel Baluta
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
@ 2019-07-23 8:41 ` Daniel Baluta
0 siblings, 0 replies; 43+ 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,
Daniel Baluta, shengjiu.wang, linux-kernel, paul.olaru, robh+dt,
linux-imx, kernel, leonard.crestez, festevam, linux-arm-kernel,
sound-open-firmware
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 43+ messages in thread* Re: [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
2019-07-23 8:41 ` Daniel Baluta
(?)
@ 2019-07-23 22:32 ` Rob Herring
-1 siblings, 0 replies; 43+ 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] 43+ messages in thread* Re: [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
@ 2019-07-23 22:32 ` Rob Herring
0 siblings, 0 replies; 43+ messages in thread
From: Rob Herring @ 2019-07-23 22:32 UTC (permalink / raw)
To: Daniel Baluta
Cc: Mark Rutland, Dong Aisheng, Peng Fan, Anson Huang, devicetree,
Fabio Estevam, S.j. Wang, Marco Felsch, linux-kernel, paul.olaru,
NXP Linux Team, Sascha Hauer, Leonard Crestez, Shawn Guo,
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
>
_______________________________________________
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] 43+ messages in thread* Re: [PATCH v2 5/5] dt-bindings: dsp: fsl: Add DSP core binding support
@ 2019-07-23 22:32 ` Rob Herring
0 siblings, 0 replies; 43+ 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] 43+ messages in thread