[PATCH 0/2] ASoC: simple-card: Introduce playback-only/capture-only DAI link flags

[PATCH 0/2] ASoC: simple-card: Introduce playback-only/capture-only DAI link flags

[Daniel Baluta]

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

This patch series introduces capture-only/playback-only DAI link
properties for simple-card.

Daniel Baluta (2):
  ASoC: simple-card: Introduce playback-only/capture only DAI link flags
  ASoC: dt-bindings: simple-card: Document new DAI flags
    playback-only/capture-only

 .../bindings/sound/simple-card.yaml           |  8 ++++++
 include/sound/simple_card_utils.h             |  5 ++++
 sound/soc/generic/simple-card-utils.c         | 27 +++++++++++++++++++
 sound/soc/generic/simple-card.c               | 10 +++++++
 4 files changed, 50 insertions(+)

-- 
2.25.1

[PATCH 1/2] ASoC: simple-card: Introduce playback-only/capture only DAI link flags

[Daniel Baluta]

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

We need this to signal that DAI link supports only 1 direction that
can only be either playback or capture.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 include/sound/simple_card_utils.h     |  5 +++++
 sound/soc/generic/simple-card-utils.c | 27 +++++++++++++++++++++++++++
 sound/soc/generic/simple-card.c       | 10 ++++++++++
 3 files changed, 42 insertions(+)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index d1a95bc33c56..47d90edaf6fe 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -135,6 +135,11 @@ int asoc_simple_parse_daifmt(struct device *dev,
 			     struct device_node *codec,
 			     char *prefix,
 			     unsigned int *retfmt);
+int asoc_simple_parse_link_direction(struct device *dev,
+				     struct device_node *node,
+				     char *prefix,
+				     bool *is_playback_only,
+				     bool *is_capture_only);
 int asoc_simple_parse_tdm_width_map(struct device *dev, struct device_node *np,
 				    struct asoc_simple_dai *dai);
 
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 5b18a4af022f..e04d2995cf0b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -115,6 +115,33 @@ int asoc_simple_parse_daifmt(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(asoc_simple_parse_daifmt);
 
+int asoc_simple_parse_link_direction(struct device *dev, struct device_node *node, char *prefix,
+				     bool *playback_only, bool *capture_only)
+{
+	bool is_playback_only = false;
+	bool is_capture_only = false;
+
+	if (!prefix)
+		prefix = "";
+
+	if (of_property_read_bool(node, "playback-only"))
+		is_playback_only = true;
+
+	if (of_property_read_bool(node, "capture-only"))
+		is_capture_only = true;
+
+	if (is_playback_only && is_capture_only) {
+		dev_err(dev, "Invalid configuration, both playback-only / capture-only are set\n");
+		return -EINVAL;
+	}
+
+	*playback_only = is_playback_only;
+	*capture_only = is_capture_only;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_link_direction);
+
 int asoc_simple_parse_tdm_width_map(struct device *dev, struct device_node *np,
 				    struct asoc_simple_dai *dai)
 {
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 190f11366e84..1fb34a51636d 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -195,6 +195,7 @@ static int simple_link_init(struct asoc_simple_priv *priv,
 {
 	struct device *dev = simple_priv_to_dev(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, li->link);
+	bool is_playback_only, is_capture_only;
 	int ret;
 
 	ret = asoc_simple_parse_daifmt(dev, node, codec,
@@ -202,6 +203,15 @@ static int simple_link_init(struct asoc_simple_priv *priv,
 	if (ret < 0)
 		return 0;
 
+	ret = asoc_simple_parse_link_direction(dev, node, prefix,
+					       &is_playback_only,
+					       &is_capture_only);
+	if (ret < 0)
+		return 0;
+
+	dai_link->playback_only = is_playback_only;
+	dai_link->capture_only = is_capture_only;
+
 	dai_link->init			= asoc_simple_dai_init;
 	dai_link->ops			= &simple_ops;
 
-- 
2.25.1

Re: [PATCH 1/2] ASoC: simple-card: Introduce playback-only/capture only DAI link flags

[Kuninori Morimoto]

Hi Daniel

Thank you for your patch.
This is not a big deal, but...

> We need this to signal that DAI link supports only 1 direction that
> can only be either playback or capture.
(snip)
> +	if (of_property_read_bool(node, "playback-only"))
> +		is_playback_only = true;
> +
> +	if (of_property_read_bool(node, "capture-only"))
> +		is_capture_only = true;

More simply

	is_playback_only = of_property_read_bool(node, "playback-only");
	is_capture_only  = of_property_read_bool(node, "capture-only");

> +	ret = asoc_simple_parse_link_direction(dev, node, prefix,
> +					       &is_playback_only,
> +					       &is_capture_only);
> +	if (ret < 0)
> +		return 0;
> +
> +	dai_link->playback_only = is_playback_only;
> +	dai_link->capture_only = is_capture_only;

It doesn't overwrite when error case, so
More simply

	ret = asoc_simple_parse_link_direction(dev, node, prefix,
						&dai_link->playback_only,
						&dai_link->capture_only);


Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/2] ASoC: simple-card: Introduce playback-only/capture only DAI link flags

[Daniel Baluta]

On Wed, Aug 2, 2023 at 2:31 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
>
>
> Hi Daniel
>
> Thank you for your patch.
> This is not a big deal, but...
>
> > We need this to signal that DAI link supports only 1 direction that
> > can only be either playback or capture.
> (snip)
> > +     if (of_property_read_bool(node, "playback-only"))
> > +             is_playback_only = true;
> > +
> > +     if (of_property_read_bool(node, "capture-only"))
> > +             is_capture_only = true;
>
> More simply
>
>         is_playback_only = of_property_read_bool(node, "playback-only");
>         is_capture_only  = of_property_read_bool(node, "capture-only");


Good point. Will fix in v2.

>
> > +     ret = asoc_simple_parse_link_direction(dev, node, prefix,
> > +                                            &is_playback_only,
> > +                                            &is_capture_only);
> > +     if (ret < 0)
> > +             return 0;
> > +
> > +     dai_link->playback_only = is_playback_only;
> > +     dai_link->capture_only = is_capture_only;
>
> It doesn't overwrite when error case, so
> More simply
>
>         ret = asoc_simple_parse_link_direction(dev, node, prefix,
>                                                 &dai_link->playback_only,
>                                                 &dai_link->capture_only);

Can do this because dai_link->playback_only is a bitfield.

[PATCH 2/2] ASoC: dt-bindings: simple-card: Document new DAI flags playback-only/capture-only

[Daniel Baluta]

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

Document new playback-only and capture-only flags which can be used when
dai link can only support just one direction: playback or capture but
not both.

Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
---
 Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
index 59ac2d1d1ccf..1bf331f095a4 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.yaml
+++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
@@ -27,6 +27,14 @@ definitions:
     description: dai-link uses bit clock inversion
     $ref: /schemas/types.yaml#/definitions/flag
 
+  playback-only:
+    description: dai-link is used only for playback
+    $ref: /schemas/types.yaml#/definitions/flag
+
+  capture-only:
+    description: dai-link is used only for capture
+    $ref: /schemas/types.yaml#/definitions/flag
+
   dai-tdm-slot-num:
     description: see tdm-slot.txt.
     $ref: /schemas/types.yaml#/definitions/uint32
-- 
2.25.1

Re: [PATCH 2/2] ASoC: dt-bindings: simple-card: Document new DAI flags playback-only/capture-only

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 746 bytes --]

On Tue, Aug 01, 2023 at 11:24:33AM +0300, Daniel Baluta wrote:
> From: Daniel Baluta <daniel.baluta@nxp.com>
> 
> Document new playback-only and capture-only flags which can be used when
> dai link can only support just one direction: playback or capture but
> not both.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)

Please add new features to audio-graph-card2 - we're trying to deprecate
simple-card, audio-graph-card2 is a superset with more flexibility.
It's not the end of the world to also support things in simple-card but
it definitely shouldn't have any capabilities that the newer card lacks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/2] ASoC: dt-bindings: simple-card: Document new DAI flags playback-only/capture-only

[Rob Herring]

On Tue, Aug 01, 2023 at 11:24:33AM +0300, Daniel Baluta wrote:
> From: Daniel Baluta <daniel.baluta@nxp.com>
> 
> Document new playback-only and capture-only flags which can be used when
> dai link can only support just one direction: playback or capture but
> not both.
> 
> Signed-off-by: Daniel Baluta <daniel.baluta@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/simple-card.yaml | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.yaml b/Documentation/devicetree/bindings/sound/simple-card.yaml
> index 59ac2d1d1ccf..1bf331f095a4 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.yaml
> +++ b/Documentation/devicetree/bindings/sound/simple-card.yaml
> @@ -27,6 +27,14 @@ definitions:
>      description: dai-link uses bit clock inversion
>      $ref: /schemas/types.yaml#/definitions/flag
>  
> +  playback-only:
> +    description: dai-link is used only for playback
> +    $ref: /schemas/types.yaml#/definitions/flag
> +
> +  capture-only:
> +    description: dai-link is used only for capture
> +    $ref: /schemas/types.yaml#/definitions/flag

Wouldn't this be implicit based on limitations in the either the cpu or 
codec DAI?

Rob

Re: [PATCH 2/2] ASoC: dt-bindings: simple-card: Document new DAI flags playback-only/capture-only

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

On Fri, Aug 11, 2023 at 01:12:36PM -0600, Rob Herring wrote:
> On Tue, Aug 01, 2023 at 11:24:33AM +0300, Daniel Baluta wrote:

> > +  playback-only:
> > +    description: dai-link is used only for playback
> > +    $ref: /schemas/types.yaml#/definitions/flag

> > +  capture-only:
> > +    description: dai-link is used only for capture
> > +    $ref: /schemas/types.yaml#/definitions/flag

> Wouldn't this be implicit based on limitations in the either the cpu or 
> codec DAI?

You can see cases where people just don't connect some of the signals
for whatever reason so even if the two devices could do bidrectional
audio the board can't, and there are also cases like the at91sam9g20ek
where the DAI is connected for bidrectional audio but there's not
actually any audio inputs you can connect (even loopbacks) for one of
the directions so it's best to just mask things out from the user.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]