[PATCH v2 0/2] Add generic FSL CPU DAI driver

[PATCH v2 0/2] Add generic FSL CPU DAI driver

[Daniel Baluta]

From: Daniel Baluta <daniel.baluta@nxp.com>

On platforms with a DSP we need to split the resource handling between
Application Processor (AP) and DSP. On platforms where the DSP
doesn't have an easy access to resources, the AP will take care of
configuring them. Resources handled by this generic driver are:
clocks, power domains, pinctrl.

Changes since v1:
	- added dt-bindings patch
	- add missing signed-off-by
	- do not hardcode the name of DAI driver but derive it from
	newly introduced dai_index property.

Daniel Baluta (2):
  dt-bindings: sound: Add FSL CPU DAI bindings
  ASoC: fsl: Add generic CPU DAI driver

 .../devicetree/bindings/sound/fsl,dai.yaml    |  97 ++++++
 sound/soc/fsl/Kconfig                         |   8 +
 sound/soc/fsl/Makefile                        |   2 +
 sound/soc/fsl/fsl_dai.c                       | 288 ++++++++++++++++++
 4 files changed, 395 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,dai.yaml
 create mode 100644 sound/soc/fsl/fsl_dai.c

-- 
2.17.1

[PATCH v2 1/2] dt-bindings: sound: Add FSL CPU DAI bindings

[Daniel Baluta]

From: Daniel Baluta <daniel.baluta@nxp.com>

Add dt bindings for he Generic FSL CPU DAI.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 .../devicetree/bindings/sound/fsl,dai.yaml    | 97 +++++++++++++++++++
 1 file changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,dai.yaml

diff --git a/Documentation/devicetree/bindings/sound/fsl,dai.yaml b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
new file mode 100644
index 000000000000..e6426af67d30
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
@@ -0,0 +1,97 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/fsl,dai.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic CPU FSL DAI driver for resource management
+
+maintainers:
+  - Daniel Baluta <daniel.baluta@nxp.com>
+
+description: |
+  On platforms with a DSP we need to split the resource handling between
+  Application Processor (AP) and DSP. On platforms where the DSP doesn't
+  have an easy access to resources, the AP will take care of
+  configuring them. Resources handled by this generic driver are: clocks,
+  power domains, pinctrl.
+
+properties:
+  '#sound-dai-cells':
+    const: 0
+
+  compatible:
+    enum:
+      - fsl,esai-dai
+      - fsl,sai-dai
+
+  clocks:
+    oneOf:
+      - items: # for ESAI
+          - description: Core clock used to access registers.
+          - description: ESAI baud clock for ESAI controller used to derive
+                         HCK, SCK and FS.
+          - description: The system clock derived from ahb clock used to derive
+                         HCK, SCK and FS.
+          - description: SPBA clock is required when ESAI is placed as a bus
+                         slave of the SP Bus and when two or more bus masters
+                         (CPU, DMA or DSP) try to access it. This property is
+                         optional depending on SoC design.
+      - items: # for SAI
+          - description: Bus clock for accessing registers
+          - description: Master clock 0 for providing bit clock and frame clock
+          - description: Master clock 1 for providing bit clock and frame clock
+          - description: Master clock 2 for providing bit clock and frame clock
+          - description: Master clock 3 for providing bit clock and frame clock
+
+  clock-names:
+    oneOf:
+      - items: # for ESAI
+          - const: core
+          - const: extal
+          - const: fsys
+          - const: spba
+      - items: # for SAI
+          - const: bus
+          - const: mclk0
+          - const: mclk1
+          - const: mclk2
+          - const: mclk3
+
+  pinctrl-0:
+    description: Should specify pin control groups used for this controller.
+
+  pinctrl-names:
+    const: default
+
+  power-domains:
+    $ref: '/schemas/types.yaml#/definitions/phandle-array'
+    description:
+      List of phandles and PM domain specifiers, as defined by bindings of the
+      PM domain provider.
+
+  fsl,dai-index:
+    $ref: '/schemas/types.yaml#/definitions/uint32'
+    description: Physical DAI index, must match the index from topology file
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - fsl,dai-index
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8-clock.h>
+    esai0_port: esai-port {
+         #sound-dai-cells = <0>;
+        compatible = "fsl,esai-dai";
+
+        fsl,dai-index = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_esai0>;
+
+        clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>,  <&esai0_lpcg 1>,
+            <&clk_dummy>;
+        clock-names = "core", "extal", "fsys", "spba";
+    };
-- 
2.17.1

[PATCH v2 2/2] ASoC: fsl: Add generic CPU DAI driver

[Daniel Baluta]

From: Daniel Baluta <daniel.baluta@nxp.com>

On i.MX8 platforms that have a DSP the DAI handling is taken care
of by two entities:
	* Application Processor (AP), which runs Linux
	* DSP, which runs a firmware (typically Sound Open Firmware)

The DSP has access to DAI IP registers, but it cannot easily handle
resources like:
	* clocks
	* power domain management
	* pinctrl

For this reason we introduce a generic FSL DAI driver which will take
care of the resources above.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 sound/soc/fsl/Kconfig   |   8 ++
 sound/soc/fsl/Makefile  |   2 +
 sound/soc/fsl/fsl_dai.c | 288 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 298 insertions(+)
 create mode 100644 sound/soc/fsl/fsl_dai.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 65e8cd4be930..bea8ab2c24f9 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -75,6 +75,14 @@ config SND_SOC_FSL_ESAI
 	  This option is only useful for out-of-tree drivers since
 	  in-tree drivers select it automatically.
 
+config SND_SOC_FSL_DAI
+	tristate "Generic FSL DAI support for Sound Open Firmware"
+	help
+	  Say Y if you want to enable generic FSL DAI support to be used
+	  with Sound Open Firmware. This module takes care of enabling
+	  clocks, power domain, pinctrl for FSL DAIs. The rest of DAI
+	  control is taken care of by SOF firmware.
+
 config SND_SOC_FSL_MICFIL
 	tristate "Pulse Density Modulation Microphone Interface (MICFIL) module support"
 	select REGMAP_MMIO
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 8cde88c72d93..e4ed253b6657 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -20,6 +20,7 @@ snd-soc-fsl-ssi-y := fsl_ssi.o
 snd-soc-fsl-ssi-$(CONFIG_DEBUG_FS) += fsl_ssi_dbg.o
 snd-soc-fsl-spdif-objs := fsl_spdif.o
 snd-soc-fsl-esai-objs := fsl_esai.o
+snd-soc-fsl-dai-objs := fsl_dai.o
 snd-soc-fsl-micfil-objs := fsl_micfil.o
 snd-soc-fsl-utils-objs := fsl_utils.o
 snd-soc-fsl-dma-objs := fsl_dma.o
@@ -32,6 +33,7 @@ obj-$(CONFIG_SND_SOC_FSL_SAI) += snd-soc-fsl-sai.o
 obj-$(CONFIG_SND_SOC_FSL_SSI) += snd-soc-fsl-ssi.o
 obj-$(CONFIG_SND_SOC_FSL_SPDIF) += snd-soc-fsl-spdif.o
 obj-$(CONFIG_SND_SOC_FSL_ESAI) += snd-soc-fsl-esai.o
+obj-$(CONFIG_SND_SOC_FSL_DAI) += snd-soc-fsl-dai.o
 obj-$(CONFIG_SND_SOC_FSL_MICFIL) += snd-soc-fsl-micfil.o
 obj-$(CONFIG_SND_SOC_FSL_UTILS) += snd-soc-fsl-utils.o
 obj-$(CONFIG_SND_SOC_FSL_MQS) += snd-soc-fsl-mqs.o
diff --git a/sound/soc/fsl/fsl_dai.c b/sound/soc/fsl/fsl_dai.c
new file mode 100644
index 000000000000..804e04f87841
--- /dev/null
+++ b/sound/soc/fsl/fsl_dai.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Freescale Generic DAI driver for DSP
+//
+// Copyright 2019 NXP
+// Author: Daniel Baluta <daniel.baluta@nxp.com>
+
+#include <linux/clk.h>
+#include <linux/module.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+
+enum fsl_dai_type {
+	FSL_DAI_TYPE_NONE,
+	FSL_DAI_TYPE_SAI,
+	FSL_DAI_TYPE_ESAI,
+};
+
+#define FSL_DAI_ESAI_CLK_NUM	4
+static const char *esai_clks[FSL_DAI_ESAI_CLK_NUM] = {
+	"core",
+	"extal",
+	"fsys",
+	"spba",
+};
+
+#define FSL_DAI_SAI_CLK_NUM	5
+static const char *sai_clks[FSL_DAI_SAI_CLK_NUM] = {
+	"bus",
+	"mclk0",
+	"mclk1",
+	"mclk2",
+	"mclk3",
+};
+
+struct fsl_dai {
+	struct platform_device *pdev;
+
+	/* DAI clocks */
+	struct clk **clks;
+	const char **clk_names;
+	int num_clks;
+
+	/* Power Domain handling */
+	int num_domains;
+	struct device **pd_dev;
+	struct device_link **link;
+
+	/* DAIS */
+	struct snd_soc_dai_driver *dai_drv;
+	int num_drv;
+};
+
+static struct snd_soc_dai_driver fsl_dai = {
+};
+
+static const struct snd_soc_component_driver fsl_dai_component = {
+	.name = "fsl-dai",
+};
+
+static int fsl_dai_init_clocks(struct fsl_dai *dai_priv)
+{
+	struct device *dev = &dai_priv->pdev->dev;
+	int i;
+
+	dai_priv->clks = devm_kcalloc(dev, dai_priv->num_clks,
+				      sizeof(*dai_priv->clks), GFP_KERNEL);
+	if (!dai_priv->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < dai_priv->num_clks; i++) {
+		dai_priv->clks[i] = devm_clk_get(dev, dai_priv->clk_names[i]);
+		if (IS_ERR(dai_priv->clks[i])) {
+			dev_dbg(dev, "Failed to get clk %s\n",
+				dai_priv->clk_names[i]);
+			dai_priv->clks[i] = NULL;
+		}
+	}
+
+	return 0;
+}
+
+int fsl_get_dai_type(struct fsl_dai *dai_priv)
+{
+	struct device_node *np = dai_priv->pdev->dev.of_node;
+
+	if (of_device_is_compatible(np, "fsl,esai-dai"))
+		return FSL_DAI_TYPE_ESAI;
+
+	if (of_device_is_compatible(np, "fsl,sai-dai"))
+		return FSL_DAI_TYPE_SAI;
+
+	return FSL_DAI_TYPE_NONE;
+}
+
+static int fsl_dai_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct fsl_dai *priv;
+	char *dai_name;
+	int dai_type, dai_index;
+	int ret;
+	int i;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->pdev = pdev;
+
+	dev_set_drvdata(&pdev->dev, priv);
+
+	ret = of_property_read_u32(np, "fsl,dai-index", &dai_index);
+	if (ret) {
+		dev_err(&pdev->dev, "dai-index missing or invalid\n");
+		return ret;
+	}
+
+	dai_type = fsl_get_dai_type(priv);
+	switch (dai_type) {
+	case FSL_DAI_TYPE_ESAI:
+		priv->clk_names = esai_clks;
+		priv->num_clks = FSL_DAI_ESAI_CLK_NUM;
+		priv->dai_drv = &fsl_dai;
+		priv->num_drv = 1;
+		dai_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "esai%d",
+					  dai_index);
+		if (!dai_name)
+			return -ENOMEM;
+		break;
+	case FSL_DAI_TYPE_SAI:
+		priv->clk_names = sai_clks;
+		priv->num_clks = FSL_DAI_SAI_CLK_NUM;
+		priv->dai_drv = &fsl_dai;
+		priv->num_drv = 1;
+		dai_name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "sai%d",
+					  dai_index);
+		if (!dai_name)
+			return -ENOMEM;
+		break;
+	default:
+		dev_err(&pdev->dev, "Invalid DAI type %d\n", dai_type);
+		return -EINVAL;
+	}
+
+	fsl_dai.name = dai_name;
+
+	ret = fsl_dai_init_clocks(priv);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Error at init clocks\n");
+		return ret;
+	}
+
+	priv->num_domains = of_count_phandle_with_args(np, "power-domains",
+						       "#power-domain-cells");
+	if (priv->num_domains < 0) {
+		dev_err(&pdev->dev, "no power-domains property in %pOF\n", np);
+		return priv->num_domains;
+	}
+
+	priv->pd_dev = devm_kmalloc_array(&pdev->dev, priv->num_domains,
+					  sizeof(*priv->pd_dev), GFP_KERNEL);
+	if (!priv->pd_dev)
+		return -ENOMEM;
+
+	priv->link = devm_kmalloc_array(&pdev->dev, priv->num_domains,
+					sizeof(*priv->link), GFP_KERNEL);
+	if (!priv->link)
+		return -ENOMEM;
+
+	for (i = 0; i < priv->num_domains; i++) {
+		priv->pd_dev[i] = dev_pm_domain_attach_by_id(&pdev->dev, i);
+		if (IS_ERR(priv->pd_dev[i])) {
+			ret = PTR_ERR(priv->pd_dev[i]);
+			goto unroll_pm;
+		}
+
+		priv->link[i] = device_link_add(&pdev->dev, priv->pd_dev[i],
+						DL_FLAG_STATELESS |
+						DL_FLAG_PM_RUNTIME |
+						DL_FLAG_RPM_ACTIVE);
+		if (!priv->link[i]) {
+			ret = -EINVAL;
+			dev_pm_domain_detach(priv->pd_dev[i], false);
+			goto unroll_pm;
+		}
+	}
+
+	pm_runtime_enable(&pdev->dev);
+
+	ret = devm_snd_soc_register_component(&pdev->dev, &fsl_dai_component,
+					      &fsl_dai, 1);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Failed to register DAI ret = %d\n", ret);
+		return ret;
+	}
+	return 0;
+
+unroll_pm:
+	while (--i >= 0) {
+		device_link_del(priv->link[i]);
+		dev_pm_domain_detach(priv->pd_dev[i], false);
+	}
+	return ret;
+}
+
+static int fsl_dai_remove(struct platform_device *pdev)
+{
+	struct fsl_dai *priv = platform_get_drvdata(pdev);
+	int i;
+
+	pm_runtime_disable(&priv->pdev->dev);
+
+	for (i = 0; i < priv->num_domains; i++) {
+		device_link_del(priv->link[i]);
+		dev_pm_domain_detach(priv->pd_dev[i], false);
+	}
+
+	return 0;
+}
+
+static const struct of_device_id fsl_dai_dt_ids[] = {
+	{ .compatible = "fsl,esai-dai", },
+	{ .compatible = "fsl,sai-dai", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, fsl_dai_dt_ids);
+
+#ifdef CONFIG_PM
+static int fsl_dai_runtime_resume(struct device *dev)
+{
+	struct fsl_dai *priv = dev_get_drvdata(dev);
+	int i, ret;
+
+	for (i = 0; i < priv->num_clks; i++) {
+		ret = clk_prepare_enable(priv->clks[i]);
+		if (ret < 0) {
+			dev_err(dev, "Failed to enable clk %s\n",
+				priv->clk_names[i]);
+			goto out;
+		}
+	}
+	return 0;
+out:
+	while (--i >= 0)
+		clk_disable_unprepare(priv->clks[i]);
+
+	return ret;
+}
+
+static int fsl_dai_runtime_suspend(struct device *dev)
+{
+	struct fsl_dai *priv = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < priv->num_clks; i++)
+		clk_disable_unprepare(priv->clks[i]);
+
+	return 0;
+}
+#endif /* CONFIG_PM */
+
+static const struct dev_pm_ops fsl_dai_pm_ops = {
+	SET_RUNTIME_PM_OPS(fsl_dai_runtime_suspend,
+			   fsl_dai_runtime_resume, NULL)
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+				pm_runtime_force_resume)
+};
+
+static struct platform_driver fsl_dai_driver = {
+	.probe = fsl_dai_probe,
+	.remove = fsl_dai_remove,
+	.driver = {
+		.name = "fsl-dai",
+		.pm = &fsl_dai_pm_ops,
+		.of_match_table = fsl_dai_dt_ids,
+	},
+};
+
+module_platform_driver(fsl_dai_driver);
+
+MODULE_ALIAS("platform:fsl-dai");
+
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@nxp.com>");
+MODULE_DESCRIPTION("FSL Generic DAI driver for DSP");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

Re: [PATCH v2 1/2] dt-bindings: sound: Add FSL CPU DAI bindings

[Rob Herring]

On Fri, Mar 06, 2020 at 01:13:52PM +0200, Daniel Baluta wrote:
> From: Daniel Baluta <daniel.baluta@nxp.com>
> 
> Add dt bindings for he Generic FSL CPU DAI.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  .../devicetree/bindings/sound/fsl,dai.yaml    | 97 +++++++++++++++++++
>  1 file changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,dai.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,dai.yaml b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
> new file mode 100644
> index 000000000000..e6426af67d30
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,dai.yaml
> @@ -0,0 +1,97 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings please:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/fsl,dai.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic CPU FSL DAI driver for resource management

Bindings are for h/w devices, not drivers.

> +
> +maintainers:
> +  - Daniel Baluta <daniel.baluta@nxp.com>
> +
> +description: |
> +  On platforms with a DSP we need to split the resource handling between
> +  Application Processor (AP) and DSP. On platforms where the DSP doesn't
> +  have an easy access to resources, the AP will take care of
> +  configuring them. Resources handled by this generic driver are: clocks,
> +  power domains, pinctrl.

The DT should define a DSP node with resources that are part of the 
DSP. What setup the AP has to do should be implied by the compatible 
string and possibly what resources are described.

Or maybe the audio portion of the DSP is a child node...

> +
> +properties:
> +  '#sound-dai-cells':
> +    const: 0
> +
> +  compatible:
> +    enum:
> +      - fsl,esai-dai
> +      - fsl,sai-dai

Not very specific. There's only 2 versions of the DSP and ways it is 
integrated?

> +
> +  clocks:
> +    oneOf:
> +      - items: # for ESAI
> +          - description: Core clock used to access registers.
> +          - description: ESAI baud clock for ESAI controller used to derive
> +                         HCK, SCK and FS.
> +          - description: The system clock derived from ahb clock used to derive
> +                         HCK, SCK and FS.
> +          - description: SPBA clock is required when ESAI is placed as a bus
> +                         slave of the SP Bus and when two or more bus masters
> +                         (CPU, DMA or DSP) try to access it. This property is
> +                         optional depending on SoC design.
> +      - items: # for SAI
> +          - description: Bus clock for accessing registers
> +          - description: Master clock 0 for providing bit clock and frame clock
> +          - description: Master clock 1 for providing bit clock and frame clock
> +          - description: Master clock 2 for providing bit clock and frame clock
> +          - description: Master clock 3 for providing bit clock and frame clock
> +
> +  clock-names:
> +    oneOf:
> +      - items: # for ESAI
> +          - const: core
> +          - const: extal
> +          - const: fsys
> +          - const: spba
> +      - items: # for SAI
> +          - const: bus
> +          - const: mclk0
> +          - const: mclk1
> +          - const: mclk2
> +          - const: mclk3
> +
> +  pinctrl-0:
> +    description: Should specify pin control groups used for this controller.
> +
> +  pinctrl-names:
> +    const: default

pinctrl properties are implicitly allowed an don't have to be listed 
here.

> +
> +  power-domains:
> +    $ref: '/schemas/types.yaml#/definitions/phandle-array'

Don't need a type.

> +    description:
> +      List of phandles and PM domain specifiers, as defined by bindings of the
> +      PM domain provider.

Don't need to re-define common properties.

You do need to say how many power domains (maxItems: 1?).

> +
> +  fsl,dai-index:
> +    $ref: '/schemas/types.yaml#/definitions/uint32'
> +    description: Physical DAI index, must match the index from topology file

Sorry, we don't do indexes in DT.

What's a topology file?

> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - fsl,dai-index
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8-clock.h>
> +    esai0_port: esai-port {
> +         #sound-dai-cells = <0>;
> +        compatible = "fsl,esai-dai";
> +
> +        fsl,dai-index = <0>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&pinctrl_esai0>;
> +
> +        clocks = <&esai0_lpcg 1>, <&esai0_lpcg 0>,  <&esai0_lpcg 1>,
> +            <&clk_dummy>;
> +        clock-names = "core", "extal", "fsys", "spba";
> +    };
> -- 
> 2.17.1
> 

Re: [PATCH v2 1/2] dt-bindings: sound: Add FSL CPU DAI bindings

[Daniel Baluta]

Thanks Rob for review. See my comments inline:

<snip>

> > +# SPDX-License-Identifier: GPL-2.0
>
> Dual license new bindings please:
>
> (GPL-2.0-only OR BSD-2-Clause)

Ok, will do.

>
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/fsl,dai.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Generic CPU FSL DAI driver for resource management
>
> Bindings are for h/w devices, not drivers.

Indeed. I think I will change it to something like this.

title: 'FSL CPU DAI for resource management'

The explanation are already in patch 2/2 of this series but let e
explain again what I'm
trying to do here and let me know if this makes sense to you.

Digital Audio Interface device (DAI) are configured by the firmware
running on the DSP. The only
trouble we have is that we cannot easily handle 'resources' like:
clocks, pinctrl, power domains from
firmware.

This is because our architecture is like this:

M core [running System Controller Firmware]
            |
            |
A core [Linux]<----> DSP core [SOF firmware]

In theory, it is possible for DSP core to communicate with M core, but
this needs a huge
amount of work in order to make it work. We have this on our plans for
the future,
but we are now trying to do resource management from A core because
the infrastructure is already in place.

So, the curent driver introduced in this series acts like a Generic
resource driver for DAI device. We can
have multiple types of DAIs but most of them need the same types of
resources (clocks, pinctrl, pm) sof
for this reason I made it generic.


>
> > +
> > +maintainers:
> > +  - Daniel Baluta <daniel.baluta@nxp.com>
> > +
> > +description: |
> > +  On platforms with a DSP we need to split the resource handling between
> > +  Application Processor (AP) and DSP. On platforms where the DSP doesn't
> > +  have an easy access to resources, the AP will take care of
> > +  configuring them. Resources handled by this generic driver are: clocks,
> > +  power domains, pinctrl.
>
> The DT should define a DSP node with resources that are part of the
> DSP. What setup the AP has to do should be implied by the compatible
> string and possibly what resources are described.

We already have a DSP node: Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
but I thought that the resources attached to DAIs are separated from
the resources
attached to the DSP device.

In the great scheme of ALSA we usually have things like this:

FE         <----->       BE

In the SOF world FE are defined by topology framework. Back ends are
defined by the machine driver:

On the BE side we have:
- codec  -> this is the specific code
- platform -> this is the DSP
- cpu -> this is our Generic DAI device

Now, I'm wondering if we can get rid of cpu here and make platform
node (dsp) take care of every
resource (this looks not natural).

Perhaps Mark, Liam or Pierre can help me with this.


>
> Or maybe the audio portion of the DSP is a child node...
>
> > +
> > +properties:
> > +  '#sound-dai-cells':
> > +    const: 0
> > +
> > +  compatible:
> > +    enum:
> > +      - fsl,esai-dai
> > +      - fsl,sai-dai
>
> Not very specific. There's only 2 versions of the DSP and ways it is
> integrated?

As I said above this is not about the DSP, but about the Digital Audio
Intraface. On i.MX
NXP boards we have two types of DAIs: SAI and ESAI.

<snip>

> > +  pinctrl-0:
> > +    description: Should specify pin control groups used for this controller.
> > +
> > +  pinctrl-names:
> > +    const: default
>
> pinctrl properties are implicitly allowed an don't have to be listed
> here.

Great.

>
> > +
> > +  power-domains:
> > +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
>
> Don't need a type.
>
> > +    description:
> > +      List of phandles and PM domain specifiers, as defined by bindings of the
> > +      PM domain provider.
>
> Don't need to re-define common properties.
>
> You do need to say how many power domains (maxItems: 1?).

We support multiple power domains, so technically there is no upper
limit. What should I put here in this case?
>
> > +
> > +  fsl,dai-index:
> > +    $ref: '/schemas/types.yaml#/definitions/uint32'
> > +    description: Physical DAI index, must match the index from topology file
>
> Sorry, we don't do indexes in DT.
>
> What's a topology file?

Topology files are binary blobs that contain the description of an
audio pipeline. They are built
are written in a specific format and compiled with alsa-tplg tools in userspace.

Then loaded via firmware interface inside the kernel.

https://www.alsa-project.org/wiki/ALSA_topology


thanks,
Daniel.

Re: [PATCH v2 2/2] ASoC: fsl: Add generic CPU DAI driver

[Lars-Peter Clausen]

On 3/6/20 12:13 PM, Daniel Baluta wrote:
> +static int fsl_dai_probe(struct platform_device *pdev)
> +{
> [...]
> +	ret = of_property_read_u32(np, "fsl,dai-index", &dai_index);
> +	if (ret) {
> +		dev_err(&pdev->dev, "dai-index missing or invalid\n");
> +		return ret;
> +	}
Maybe this can follow a more standard approach using DT aliases. Just 
like we assign IDs to things like SPI or I2C masters.
> +
> +	fsl_dai.name = dai_name;
This breaks as soon as there is more than one DAI in the system since 
you are sharing a global struct between them.
[...]

Re: [PATCH v2 1/2] dt-bindings: sound: Add FSL CPU DAI bindings

[Rob Herring]

On Mon, Mar 16, 2020 at 7:01 AM Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> Thanks Rob for review. See my comments inline:
>
> <snip>
>
> > > +# SPDX-License-Identifier: GPL-2.0
> >
> > Dual license new bindings please:
> >
> > (GPL-2.0-only OR BSD-2-Clause)
>
> Ok, will do.
>
> >
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/sound/fsl,dai.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Generic CPU FSL DAI driver for resource management
> >
> > Bindings are for h/w devices, not drivers.
>
> Indeed. I think I will change it to something like this.
>
> title: 'FSL CPU DAI for resource management'
>
> The explanation are already in patch 2/2 of this series but let e
> explain again what I'm
> trying to do here and let me know if this makes sense to you.
>
> Digital Audio Interface device (DAI) are configured by the firmware
> running on the DSP. The only
> trouble we have is that we cannot easily handle 'resources' like:
> clocks, pinctrl, power domains from
> firmware.
>
> This is because our architecture is like this:
>
> M core [running System Controller Firmware]
>             |
>             |
> A core [Linux]<----> DSP core [SOF firmware]
>
> In theory, it is possible for DSP core to communicate with M core, but
> this needs a huge
> amount of work in order to make it work. We have this on our plans for
> the future,
> but we are now trying to do resource management from A core because
> the infrastructure is already in place.

When you change things in the future, Linux gets to keep supporting
both ways of doing things? I'd rather just support one way.

> So, the curent driver introduced in this series acts like a Generic
> resource driver for DAI device. We can
> have multiple types of DAIs but most of them need the same types of
> resources (clocks, pinctrl, pm) sof
> for this reason I made it generic.
>
>
> >
> > > +
> > > +maintainers:
> > > +  - Daniel Baluta <daniel.baluta@nxp.com>
> > > +
> > > +description: |
> > > +  On platforms with a DSP we need to split the resource handling between
> > > +  Application Processor (AP) and DSP. On platforms where the DSP doesn't
> > > +  have an easy access to resources, the AP will take care of
> > > +  configuring them. Resources handled by this generic driver are: clocks,
> > > +  power domains, pinctrl.
> >
> > The DT should define a DSP node with resources that are part of the
> > DSP. What setup the AP has to do should be implied by the compatible
> > string and possibly what resources are described.
>
> We already have a DSP node: Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
> but I thought that the resources attached to DAIs are separated from
> the resources
> attached to the DSP device.

I'd agree if the DAI was fully described in the DT.

> In the great scheme of ALSA we usually have things like this:
>
> FE         <----->       BE
>
> In the SOF world FE are defined by topology framework. Back ends are
> defined by the machine driver:
>
> On the BE side we have:
> - codec  -> this is the specific code
> - platform -> this is the DSP
> - cpu -> this is our Generic DAI device
>
> Now, I'm wondering if we can get rid of cpu here and make platform
> node (dsp) take care of every
> resource (this looks not natural).

I would think about how the DT will look when the DSP manages all
these resources itself and how the kernel drivers evolve. I think
perhaps if you can get rid of the DT part and just define the
resources in the driver, then the future transition would be easier.

> Perhaps Mark, Liam or Pierre can help me with this.
>
>
> >
> > Or maybe the audio portion of the DSP is a child node...
> >
> > > +
> > > +properties:
> > > +  '#sound-dai-cells':
> > > +    const: 0
> > > +
> > > +  compatible:
> > > +    enum:
> > > +      - fsl,esai-dai
> > > +      - fsl,sai-dai
> >
> > Not very specific. There's only 2 versions of the DSP and ways it is
> > integrated?
>
> As I said above this is not about the DSP, but about the Digital Audio
> Intraface. On i.MX
> NXP boards we have two types of DAIs: SAI and ESAI.
>
> <snip>
>
> > > +  pinctrl-0:
> > > +    description: Should specify pin control groups used for this controller.
> > > +
> > > +  pinctrl-names:
> > > +    const: default
> >
> > pinctrl properties are implicitly allowed an don't have to be listed
> > here.
>
> Great.
>
> >
> > > +
> > > +  power-domains:
> > > +    $ref: '/schemas/types.yaml#/definitions/phandle-array'
> >
> > Don't need a type.
> >
> > > +    description:
> > > +      List of phandles and PM domain specifiers, as defined by bindings of the
> > > +      PM domain provider.
> >
> > Don't need to re-define common properties.
> >
> > You do need to say how many power domains (maxItems: 1?).
>
> We support multiple power domains, so technically there is no upper
> limit. What should I put here in this case?

There's an upper limit in the h/w so there should be some sort of limit.


> > > +  fsl,dai-index:
> > > +    $ref: '/schemas/types.yaml#/definitions/uint32'
> > > +    description: Physical DAI index, must match the index from topology file
> >
> > Sorry, we don't do indexes in DT.
> >
> > What's a topology file?
>
> Topology files are binary blobs that contain the description of an
> audio pipeline. They are built
> are written in a specific format and compiled with alsa-tplg tools in userspace.
>
> Then loaded via firmware interface inside the kernel.

Sounds like a kernel-userspace issue that has nothing to do with DT.
How do other platforms deal with mulitple DAIs?

Rob