[PATCH 1/2] ASoC: dt-bindings: imx-hdmi: Add binding doc for hdmi machine driver

[PATCH 1/2] ASoC: dt-bindings: imx-hdmi: Add binding doc for hdmi machine driver

[Shengjiu Wang]

Imx-hdmi is a new added machine driver for supporting hdmi devices
on i.MX platforms. There is HDMI IP or external HDMI modules connect
with SAI or AUD2HTX interface.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 .../bindings/sound/imx-audio-hdmi.yaml        | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/imx-audio-hdmi.yaml

diff --git a/Documentation/devicetree/bindings/sound/imx-audio-hdmi.yaml b/Documentation/devicetree/bindings/sound/imx-audio-hdmi.yaml
new file mode 100644
index 000000000000..d5474f83ac2c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/imx-audio-hdmi.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/imx-audio-hdmi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX audio complex with HDMI
+
+maintainers:
+  - Shengjiu Wang <shengjiu.wang@nxp.com>
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx-audio-hdmi
+      - fsl,imx-audio-sii902x
+
+  model:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: User specified audio sound card name
+
+  audio-cpu:
+    description: The phandle of an CPU DAI controller
+
+  hdmi-out:
+    description: |
+      This is a boolean property. If present, the transmitting function
+      of HDMI will be enabled, indicating there's a physical HDMI out
+      connector or jack on the board or it's connecting to some other IP
+      block, such as an HDMI encoder or display-controller.
+
+  hdmi-in:
+    description: |
+      This is a boolean property. If present, the receiving function of
+      HDMI will be enabled, indicating there is a physical HDMI in
+      connector/jack on the board.
+
+required:
+  - compatible
+  - model
+  - audio-cpu
+
+additionalProperties: false
+
+examples:
+  - |
+    sound-hdmi {
+        compatible = "fsl,imx-audio-hdmi";
+        model = "audio-hdmi";
+        audio-cpu = <&aud2htx>;
+        hdmi-out;
+    };
-- 
2.27.0

[PATCH 2/2] ASoC: fsl: Add imx-hdmi machine driver

[Shengjiu Wang]

The driver is initially designed for sound card using HDMI
interface on i.MX platform. There is internal HDMI IP or
external HDMI modules connect with SAI or AUD2HTX interface.
It supports both transmitter and receiver devices.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/Kconfig    |  12 ++
 sound/soc/fsl/Makefile   |   2 +
 sound/soc/fsl/imx-hdmi.c | 235 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 249 insertions(+)
 create mode 100644 sound/soc/fsl/imx-hdmi.c

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 24710decd38a..84db0b7b9d59 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -305,6 +305,18 @@ config SND_SOC_IMX_AUDMIX
 	  Say Y if you want to add support for SoC audio on an i.MX board with
 	  an Audio Mixer.
 
+config SND_SOC_IMX_HDMI
+	tristate "SoC Audio support for i.MX boards with HDMI port"
+	select SND_SOC_FSL_SAI
+	select SND_SOC_FSL_AUD2HTX
+	select SND_SOC_HDMI_CODEC
+	help
+	  ALSA SoC Audio support with HDMI feature for Freescale SoCs that have
+	  SAI/AUD2HTX and connect with internal HDMI IP or external module
+	  SII902X.
+	  Say Y if you want to add support for SoC audio on an i.MX board with
+	  IMX HDMI.
+
 endif # SND_IMX_SOC
 
 endmenu
diff --git a/sound/soc/fsl/Makefile b/sound/soc/fsl/Makefile
index 0b20e038b65b..8c5fa8a859c0 100644
--- a/sound/soc/fsl/Makefile
+++ b/sound/soc/fsl/Makefile
@@ -65,9 +65,11 @@ snd-soc-imx-es8328-objs := imx-es8328.o
 snd-soc-imx-sgtl5000-objs := imx-sgtl5000.o
 snd-soc-imx-spdif-objs := imx-spdif.o
 snd-soc-imx-audmix-objs := imx-audmix.o
+snd-soc-imx-hdmi-objs := imx-hdmi.o
 
 obj-$(CONFIG_SND_SOC_EUKREA_TLV320) += snd-soc-eukrea-tlv320.o
 obj-$(CONFIG_SND_SOC_IMX_ES8328) += snd-soc-imx-es8328.o
 obj-$(CONFIG_SND_SOC_IMX_SGTL5000) += snd-soc-imx-sgtl5000.o
 obj-$(CONFIG_SND_SOC_IMX_SPDIF) += snd-soc-imx-spdif.o
 obj-$(CONFIG_SND_SOC_IMX_AUDMIX) += snd-soc-imx-audmix.o
+obj-$(CONFIG_SND_SOC_IMX_HDMI) += snd-soc-imx-hdmi.o
diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
new file mode 100644
index 000000000000..ac164514b1b2
--- /dev/null
+++ b/sound/soc/fsl/imx-hdmi.c
@@ -0,0 +1,235 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2017-2020 NXP
+
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <sound/jack.h>
+#include <sound/pcm_params.h>
+#include <sound/hdmi-codec.h>
+#include "fsl_sai.h"
+
+/**
+ * struct cpu_priv - CPU private data
+ * @sysclk_freq: SYSCLK rates for set_sysclk()
+ * @sysclk_dir: SYSCLK directions for set_sysclk()
+ * @sysclk_id: SYSCLK ids for set_sysclk()
+ * @slot_width: Slot width of each frame
+ *
+ * Note: [1] for tx and [0] for rx
+ */
+struct cpu_priv {
+	unsigned long sysclk_freq[2];
+	u32 sysclk_dir[2];
+	u32 sysclk_id[2];
+	u32 slot_width;
+};
+
+struct imx_hdmi_data {
+	struct snd_soc_dai_link dai;
+	struct snd_soc_card card;
+	struct snd_soc_jack hdmi_jack;
+	struct snd_soc_jack_pin hdmi_jack_pin;
+	struct cpu_priv cpu_priv;
+	u32 dai_fmt;
+};
+
+static int imx_hdmi_hw_params(struct snd_pcm_substream *substream,
+			      struct snd_pcm_hw_params *params)
+{
+	struct snd_soc_pcm_runtime *rtd = substream->private_data;
+	struct imx_hdmi_data *data = snd_soc_card_get_drvdata(rtd->card);
+	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
+	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_card *card = rtd->card;
+	struct device *dev = card->dev;
+	int ret;
+
+	/* set cpu DAI configuration */
+	ret = snd_soc_dai_set_sysclk(cpu_dai, data->cpu_priv.sysclk_id[tx],
+				     8 * data->cpu_priv.slot_width * params_rate(params),
+				     tx ? SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN);
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(dev, "failed to set cpu sysclk: %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0, 2, data->cpu_priv.slot_width);
+	if (ret && ret != -ENOTSUPP) {
+		dev_err(dev, "failed to set cpu dai tdm slot: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct snd_soc_ops imx_hdmi_ops = {
+	.hw_params = imx_hdmi_hw_params,
+};
+
+static const struct snd_soc_dapm_widget imx_hdmi_widgets[] = {
+	SND_SOC_DAPM_LINE("HDMI Jack", NULL),
+};
+
+static int imx_hdmi_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_card *card = rtd->card;
+	struct snd_soc_dai *codec_dai = asoc_rtd_to_codec(rtd, 0);
+	struct snd_soc_component *component = codec_dai->component;
+	struct imx_hdmi_data *data = snd_soc_card_get_drvdata(card);
+	int ret;
+
+	data->hdmi_jack_pin.pin = "HDMI Jack";
+	data->hdmi_jack_pin.mask = SND_JACK_LINEOUT;
+	/* enable jack detection */
+	ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+				    &data->hdmi_jack, &data->hdmi_jack_pin, 1);
+	if (ret) {
+		dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_component_set_jack(component, &data->hdmi_jack, NULL);
+	if (ret && ret != -EOPNOTSUPP) {
+		dev_err(card->dev, "Can't set HDMI Jack %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+};
+
+static int imx_hdmi_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct snd_soc_dai_link_component *dlc;
+	struct platform_device *cpu_pdev;
+	struct device_node *cpu_np;
+	struct imx_hdmi_data *data;
+	int ret;
+
+	dlc = devm_kzalloc(&pdev->dev, 3 * sizeof(*dlc), GFP_KERNEL);
+	if (!dlc)
+		return -ENOMEM;
+
+	cpu_np = of_parse_phandle(np, "audio-cpu", 0);
+	if (!cpu_np) {
+		dev_err(&pdev->dev, "cpu dai phandle missing or invalid\n");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	cpu_pdev = of_find_device_by_node(cpu_np);
+	if (!cpu_pdev) {
+		dev_err(&pdev->dev, "failed to find SAI platform device\n");
+		ret = -EINVAL;
+		goto fail;
+	}
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data) {
+		ret = -ENOMEM;
+		goto fail;
+	}
+
+	data->dai.cpus = &dlc[0];
+	data->dai.num_cpus = 1;
+	data->dai.platforms = &dlc[1];
+	data->dai.num_platforms = 1;
+	data->dai.codecs = &dlc[2];
+	data->dai.num_codecs = 1;
+
+	data->dai.name = "i.MX HDMI";
+	data->dai.stream_name = "i.MX HDMI";
+	data->dai.cpus->dai_name = dev_name(&cpu_pdev->dev);
+	data->dai.platforms->of_node = cpu_np;
+	data->dai.ops = &imx_hdmi_ops;
+	data->dai.playback_only = true;
+	data->dai.capture_only = false;
+	data->dai.init = imx_hdmi_init;
+
+	if (of_node_name_eq(cpu_np, "sai")) {
+		data->cpu_priv.sysclk_id[1] = FSL_SAI_CLK_MAST1;
+		data->cpu_priv.sysclk_id[0] = FSL_SAI_CLK_MAST1;
+	}
+
+	if (of_device_is_compatible(np, "fsl,imx-audio-sii902x")) {
+		data->dai_fmt = SND_SOC_DAIFMT_LEFT_J;
+		data->cpu_priv.slot_width = 24;
+	} else {
+		data->dai_fmt = SND_SOC_DAIFMT_I2S;
+		data->cpu_priv.slot_width = 32;
+	}
+
+	if (of_property_read_bool(np, "hdmi-out")) {
+		data->dai.playback_only = true;
+		data->dai.capture_only = false;
+		data->dai.codecs->dai_name = "i2s-hifi";
+		data->dai.codecs->name = "hdmi-audio-codec.1";
+		data->dai.dai_fmt = data->dai_fmt |
+				    SND_SOC_DAIFMT_NB_NF |
+				    SND_SOC_DAIFMT_CBS_CFS;
+	}
+
+	if (of_property_read_bool(np, "hdmi-in")) {
+		data->dai.playback_only = false;
+		data->dai.capture_only = true;
+		data->dai.codecs->dai_name = "i2s-hifi";
+		data->dai.codecs->name = "hdmi-audio-codec.2";
+		data->dai.dai_fmt = data->dai_fmt |
+				    SND_SOC_DAIFMT_NB_NF |
+				    SND_SOC_DAIFMT_CBM_CFM;
+	}
+
+	if ((data->dai.playback_only && data->dai.capture_only) ||
+	    (!data->dai.playback_only && !data->dai.capture_only)) {
+		dev_err(&pdev->dev, "Wrongly enable HDMI DAI link\n");
+		goto fail;
+	}
+
+	data->card.dapm_widgets = imx_hdmi_widgets;
+	data->card.num_dapm_widgets = ARRAY_SIZE(imx_hdmi_widgets);
+	data->card.dev = &pdev->dev;
+	data->card.owner = THIS_MODULE;
+	ret = snd_soc_of_parse_card_name(&data->card, "model");
+	if (ret)
+		goto fail;
+
+	data->card.num_links = 1;
+	data->card.dai_link = &data->dai;
+
+	platform_set_drvdata(pdev, &data->card);
+	snd_soc_card_set_drvdata(&data->card, data);
+	ret = devm_snd_soc_register_card(&pdev->dev, &data->card);
+	if (ret) {
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+		goto fail;
+	}
+
+fail:
+	if (cpu_np)
+		of_node_put(cpu_np);
+
+	return ret;
+}
+
+static const struct of_device_id imx_hdmi_dt_ids[] = {
+	{ .compatible = "fsl,imx-audio-hdmi", },
+	{ .compatible = "fsl,imx-audio-sii902x", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, imx_hdmi_dt_ids);
+
+static struct platform_driver imx_hdmi_driver = {
+	.driver = {
+		.name = "imx-hdmi",
+		.owner = THIS_MODULE,
+		.pm = &snd_soc_pm_ops,
+		.of_match_table = imx_hdmi_dt_ids,
+	},
+	.probe = imx_hdmi_probe,
+};
+module_platform_driver(imx_hdmi_driver);
+
+MODULE_AUTHOR("Freescale Semiconductor, Inc.");
+MODULE_DESCRIPTION("Freescale i.MX hdmi audio ASoC machine driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx-hdmi");
-- 
2.27.0

Re: [PATCH 2/2] ASoC: fsl: Add imx-hdmi machine driver

[Nicolin Chen]

On Fri, Nov 27, 2020 at 01:30:21PM +0800, Shengjiu Wang wrote:
> The driver is initially designed for sound card using HDMI
> interface on i.MX platform. There is internal HDMI IP or
> external HDMI modules connect with SAI or AUD2HTX interface.
> It supports both transmitter and receiver devices.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/Kconfig    |  12 ++
>  sound/soc/fsl/Makefile   |   2 +
>  sound/soc/fsl/imx-hdmi.c | 235 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 249 insertions(+)
>  create mode 100644 sound/soc/fsl/imx-hdmi.c

> diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
> new file mode 100644
> index 000000000000..ac164514b1b2
> --- /dev/null
> +++ b/sound/soc/fsl/imx-hdmi.c

> +static int imx_hdmi_hw_params(struct snd_pcm_substream *substream,
> +			      struct snd_pcm_hw_params *params)
> +{
> +	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> +	struct imx_hdmi_data *data = snd_soc_card_get_drvdata(rtd->card);
> +	bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> +	struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> +	struct snd_soc_card *card = rtd->card;
> +	struct device *dev = card->dev;
> +	int ret;
> +
> +	/* set cpu DAI configuration */
> +	ret = snd_soc_dai_set_sysclk(cpu_dai, data->cpu_priv.sysclk_id[tx],
> +				     8 * data->cpu_priv.slot_width * params_rate(params),

Looks like fixed 2 slots being used, judging by the set_tdm_slot
call below. Then...why "8 *"? Probably need a line of comments?

> +				     tx ? SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN);
> +	if (ret && ret != -ENOTSUPP) {
> +		dev_err(dev, "failed to set cpu sysclk: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0, 2, data->cpu_priv.slot_width);

May have a local variable to cache slot_width.

> +static int imx_hdmi_probe(struct platform_device *pdev)

> +	data->dai.name = "i.MX HDMI";
> +	data->dai.stream_name = "i.MX HDMI";
> +	data->dai.cpus->dai_name = dev_name(&cpu_pdev->dev);
> +	data->dai.platforms->of_node = cpu_np;
> +	data->dai.ops = &imx_hdmi_ops;
> +	data->dai.playback_only = true;
> +	data->dai.capture_only = false;
> +	data->dai.init = imx_hdmi_init;
> +
> +
> +	if (of_property_read_bool(np, "hdmi-out")) {
> +		data->dai.playback_only = true;
> +		data->dai.capture_only = false;
> +		data->dai.codecs->dai_name = "i2s-hifi";
> +		data->dai.codecs->name = "hdmi-audio-codec.1";
> +		data->dai.dai_fmt = data->dai_fmt |
> +				    SND_SOC_DAIFMT_NB_NF |
> +				    SND_SOC_DAIFMT_CBS_CFS;
> +	}
> +
> +	if (of_property_read_bool(np, "hdmi-in")) {
> +		data->dai.playback_only = false;
> +		data->dai.capture_only = true;
> +		data->dai.codecs->dai_name = "i2s-hifi";
> +		data->dai.codecs->name = "hdmi-audio-codec.2";
> +		data->dai.dai_fmt = data->dai_fmt |
> +				    SND_SOC_DAIFMT_NB_NF |
> +				    SND_SOC_DAIFMT_CBM_CFM;
> +	}
> +
> +	if ((data->dai.playback_only && data->dai.capture_only) ||
> +	    (!data->dai.playback_only && !data->dai.capture_only)) {
> +		dev_err(&pdev->dev, "Wrongly enable HDMI DAI link\n");
> +		goto fail;
> +	}

Seems that this condition check can never be true, given that:
1. By default: playback_only=true && capture_only=false
2. Conditionally overwritten: playback_only=true && capture_only=false
3. Conditionally overwritten: playback_only=false && capture_only=true

If I understand it correctly, probably should be something like:
	bool hdmi_out = of_property_read_bool(np, "hdmi-out");
	bool hdmi_in = of_property_read_bool(np, "hdmi-in");

	if ((hdmi_out && hdmi_in) || (!hdmi_out || !hdmi_in))
		// "Invalid HDMI DAI link"; goto fail;

	if (hdmi_out) {
		// ...
	} else if (hdmi_in) {
		// ...
	} else // No need of this line if two properties are exclusive

> +	data->card.num_links = 1;
> +	data->card.dai_link = &data->dai;
> +
> +	platform_set_drvdata(pdev, &data->card);

Why pass card pointer?

Re: [PATCH 2/2] ASoC: fsl: Add imx-hdmi machine driver

[Shengjiu Wang]

On Thu, Dec 3, 2020 at 4:23 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Fri, Nov 27, 2020 at 01:30:21PM +0800, Shengjiu Wang wrote:
> > The driver is initially designed for sound card using HDMI
> > interface on i.MX platform. There is internal HDMI IP or
> > external HDMI modules connect with SAI or AUD2HTX interface.
> > It supports both transmitter and receiver devices.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  sound/soc/fsl/Kconfig    |  12 ++
> >  sound/soc/fsl/Makefile   |   2 +
> >  sound/soc/fsl/imx-hdmi.c | 235 +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 249 insertions(+)
> >  create mode 100644 sound/soc/fsl/imx-hdmi.c
>
> > diff --git a/sound/soc/fsl/imx-hdmi.c b/sound/soc/fsl/imx-hdmi.c
> > new file mode 100644
> > index 000000000000..ac164514b1b2
> > --- /dev/null
> > +++ b/sound/soc/fsl/imx-hdmi.c
>
> > +static int imx_hdmi_hw_params(struct snd_pcm_substream *substream,
> > +                           struct snd_pcm_hw_params *params)
> > +{
> > +     struct snd_soc_pcm_runtime *rtd = substream->private_data;
> > +     struct imx_hdmi_data *data = snd_soc_card_get_drvdata(rtd->card);
> > +     bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
> > +     struct snd_soc_dai *cpu_dai = asoc_rtd_to_cpu(rtd, 0);
> > +     struct snd_soc_card *card = rtd->card;
> > +     struct device *dev = card->dev;
> > +     int ret;
> > +
> > +     /* set cpu DAI configuration */
> > +     ret = snd_soc_dai_set_sysclk(cpu_dai, data->cpu_priv.sysclk_id[tx],
> > +                                  8 * data->cpu_priv.slot_width * params_rate(params),
>
> Looks like fixed 2 slots being used, judging by the set_tdm_slot
> call below. Then...why "8 *"? Probably need a line of comments?

The master clock always 256 * rate, when slot_width=32.  so use
the 8 * slot_width.  will add comments.

>
> > +                                  tx ? SND_SOC_CLOCK_OUT : SND_SOC_CLOCK_IN);
> > +     if (ret && ret != -ENOTSUPP) {
> > +             dev_err(dev, "failed to set cpu sysclk: %d\n", ret);
> > +             return ret;
> > +     }
> > +
> > +     ret = snd_soc_dai_set_tdm_slot(cpu_dai, 0, 0, 2, data->cpu_priv.slot_width);
>
> May have a local variable to cache slot_width.

ok.

>
> > +static int imx_hdmi_probe(struct platform_device *pdev)
>
> > +     data->dai.name = "i.MX HDMI";
> > +     data->dai.stream_name = "i.MX HDMI";
> > +     data->dai.cpus->dai_name = dev_name(&cpu_pdev->dev);
> > +     data->dai.platforms->of_node = cpu_np;
> > +     data->dai.ops = &imx_hdmi_ops;
> > +     data->dai.playback_only = true;
> > +     data->dai.capture_only = false;
> > +     data->dai.init = imx_hdmi_init;
> > +
> > +
> > +     if (of_property_read_bool(np, "hdmi-out")) {
> > +             data->dai.playback_only = true;
> > +             data->dai.capture_only = false;
> > +             data->dai.codecs->dai_name = "i2s-hifi";
> > +             data->dai.codecs->name = "hdmi-audio-codec.1";
> > +             data->dai.dai_fmt = data->dai_fmt |
> > +                                 SND_SOC_DAIFMT_NB_NF |
> > +                                 SND_SOC_DAIFMT_CBS_CFS;
> > +     }
> > +
> > +     if (of_property_read_bool(np, "hdmi-in")) {
> > +             data->dai.playback_only = false;
> > +             data->dai.capture_only = true;
> > +             data->dai.codecs->dai_name = "i2s-hifi";
> > +             data->dai.codecs->name = "hdmi-audio-codec.2";
> > +             data->dai.dai_fmt = data->dai_fmt |
> > +                                 SND_SOC_DAIFMT_NB_NF |
> > +                                 SND_SOC_DAIFMT_CBM_CFM;
> > +     }
> > +
> > +     if ((data->dai.playback_only && data->dai.capture_only) ||
> > +         (!data->dai.playback_only && !data->dai.capture_only)) {
> > +             dev_err(&pdev->dev, "Wrongly enable HDMI DAI link\n");
> > +             goto fail;
> > +     }
>
> Seems that this condition check can never be true, given that:
> 1. By default: playback_only=true && capture_only=false
> 2. Conditionally overwritten: playback_only=true && capture_only=false
> 3. Conditionally overwritten: playback_only=false && capture_only=true
>
> If I understand it correctly, probably should be something like:
>         bool hdmi_out = of_property_read_bool(np, "hdmi-out");
>         bool hdmi_in = of_property_read_bool(np, "hdmi-in");
>
>         if ((hdmi_out && hdmi_in) || (!hdmi_out || !hdmi_in))
>                 // "Invalid HDMI DAI link"; goto fail;
>
>         if (hdmi_out) {
>                 // ...
>         } else if (hdmi_in) {
>                 // ...
>         } else // No need of this line if two properties are exclusive
>

Good catch, will update it.

> > +     data->card.num_links = 1;
> > +     data->card.dai_link = &data->dai;
> > +
> > +     platform_set_drvdata(pdev, &data->card);
>
> Why pass card pointer?

Seems it duplicates with dev_set_drvdata(card->dev, card);
in snd_soc_register_card.  will remove it.

best regards
wang shengjiu