[PATCH v2 0/2] Support Tegra I2S client format conversion

[PATCH v2 0/2] Support Tegra I2S client format conversion

[Sameer Pujar]

From: Mohan Kumar <mkumard@nvidia.com>

The AHUB HW modules are interconnected with CIF which are capable of
supporting Channel and Sample bit format conversion. Due to this, the
I2S Client can have different Channel and Sample bit from the hw_params()
and this config is passed from CIF port of I2S DT node which can help to
perform this conversion.

- First change to split simple_fixup_sample_fmt to support returning
  sample format value
- Second patch to support Tegra I2S client channel and sample format
  programming based on CIF port from DT node.


Changelog
=========
  v2:
    - Fix sparse warning related to incorrect type assignment for
      variable 'sample_format' in hw_params() call. The defect was
      reported by "kernel test robot <lkp@intel.com>" and details
      can be found below:

      https://lore.kernel.org/oe-kbuild-all/202405211805.UMAdiH0d-lkp@intel.com/

Mohan Kumar (2):
  ASoC: simple-card-utils: Split simple_fixup_sample_fmt func
  ASoC: tegra: I2S client convert formats handling

 include/sound/simple_card_utils.h     |  2 +
 sound/soc/generic/simple-card-utils.c | 26 +++++++---
 sound/soc/tegra/tegra210_i2s.c        | 71 +++++++++++++++++++++++++--
 sound/soc/tegra/tegra210_i2s.h        |  2 +
 4 files changed, 91 insertions(+), 10 deletions(-)

-- 
2.45.1

[PATCH v2 1/2] ASoC: simple-card-utils: Split simple_fixup_sample_fmt func

[Sameer Pujar]

From: Mohan Kumar <mkumard@nvidia.com>

Split the simple_fixup_sample_fmt() into two functions by adding
one more function named simple_util_get_sample_fmt() to return
the sample format value.

This is useful for drivers that wish to simply get the sample format
without setting the mask.

Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 include/sound/simple_card_utils.h     |  2 ++
 sound/soc/generic/simple-card-utils.c | 26 ++++++++++++++++++++------
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index ad67957b7b48..2c2279d082ec 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -174,6 +174,8 @@ void simple_util_parse_convert(struct device_node *np, char *prefix,
 			       struct simple_util_data *data);
 bool simple_util_is_convert_required(const struct simple_util_data *data);
 
+int simple_util_get_sample_fmt(struct simple_util_data *data);
+
 int simple_util_parse_routing(struct snd_soc_card *card,
 				      char *prefix);
 int simple_util_parse_widgets(struct snd_soc_card *card,
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 81077d16d22f..f1f5a1c025fc 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -13,12 +13,11 @@
 #include <sound/pcm_params.h>
 #include <sound/simple_card_utils.h>
 
-static void simple_fixup_sample_fmt(struct simple_util_data *data,
-					 struct snd_pcm_hw_params *params)
+int simple_util_get_sample_fmt(struct simple_util_data *data)
 {
 	int i;
-	struct snd_mask *mask = hw_param_mask(params,
-					      SNDRV_PCM_HW_PARAM_FORMAT);
+	int val = -EINVAL;
+
 	struct {
 		char *fmt;
 		u32 val;
@@ -33,11 +32,26 @@ static void simple_fixup_sample_fmt(struct simple_util_data *data,
 	for (i = 0; i < ARRAY_SIZE(of_sample_fmt_table); i++) {
 		if (!strcmp(data->convert_sample_format,
 			    of_sample_fmt_table[i].fmt)) {
-			snd_mask_none(mask);
-			snd_mask_set(mask, of_sample_fmt_table[i].val);
+			val = of_sample_fmt_table[i].val;
 			break;
 		}
 	}
+	return val;
+}
+EXPORT_SYMBOL_GPL(simple_util_get_sample_fmt);
+
+static void simple_fixup_sample_fmt(struct simple_util_data *data,
+				    struct snd_pcm_hw_params *params)
+{
+	int val;
+	struct snd_mask *mask = hw_param_mask(params,
+					      SNDRV_PCM_HW_PARAM_FORMAT);
+
+	val = simple_util_get_sample_fmt(data);
+	if (val >= 0) {
+		snd_mask_none(mask);
+		snd_mask_set(mask, val);
+	}
 }
 
 void simple_util_parse_convert(struct device_node *np,
-- 
2.45.1

Re: [PATCH v2 1/2] ASoC: simple-card-utils: Split simple_fixup_sample_fmt func

[Thierry Reding]

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

On Mon May 27, 2024 at 2:56 PM CEST, Sameer Pujar wrote:
> From: Mohan Kumar <mkumard@nvidia.com>
>
> Split the simple_fixup_sample_fmt() into two functions by adding
> one more function named simple_util_get_sample_fmt() to return
> the sample format value.
>
> This is useful for drivers that wish to simply get the sample format
> without setting the mask.
>
> Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  include/sound/simple_card_utils.h     |  2 ++
>  sound/soc/generic/simple-card-utils.c | 26 ++++++++++++++++++++------
>  2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
> index ad67957b7b48..2c2279d082ec 100644
> --- a/include/sound/simple_card_utils.h
> +++ b/include/sound/simple_card_utils.h
> @@ -174,6 +174,8 @@ void simple_util_parse_convert(struct device_node *np, char *prefix,
>  			       struct simple_util_data *data);
>  bool simple_util_is_convert_required(const struct simple_util_data *data);
>  
> +int simple_util_get_sample_fmt(struct simple_util_data *data);
> +
>  int simple_util_parse_routing(struct snd_soc_card *card,
>  				      char *prefix);
>  int simple_util_parse_widgets(struct snd_soc_card *card,
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index 81077d16d22f..f1f5a1c025fc 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -13,12 +13,11 @@
>  #include <sound/pcm_params.h>
>  #include <sound/simple_card_utils.h>
>  
> -static void simple_fixup_sample_fmt(struct simple_util_data *data,
> -					 struct snd_pcm_hw_params *params)
> +int simple_util_get_sample_fmt(struct simple_util_data *data)
>  {
>  	int i;
> -	struct snd_mask *mask = hw_param_mask(params,
> -					      SNDRV_PCM_HW_PARAM_FORMAT);
> +	int val = -EINVAL;
> +
>  	struct {
>  		char *fmt;
>  		u32 val;
> @@ -33,11 +32,26 @@ static void simple_fixup_sample_fmt(struct simple_util_data *data,
>  	for (i = 0; i < ARRAY_SIZE(of_sample_fmt_table); i++) {
>  		if (!strcmp(data->convert_sample_format,
>  			    of_sample_fmt_table[i].fmt)) {
> -			snd_mask_none(mask);
> -			snd_mask_set(mask, of_sample_fmt_table[i].val);
> +			val = of_sample_fmt_table[i].val;
>  			break;
>  		}
>  	}
> +	return val;
> +}
> +EXPORT_SYMBOL_GPL(simple_util_get_sample_fmt);
> +
> +static void simple_fixup_sample_fmt(struct simple_util_data *data,
> +				    struct snd_pcm_hw_params *params)
> +{
> +	int val;
> +	struct snd_mask *mask = hw_param_mask(params,
> +					      SNDRV_PCM_HW_PARAM_FORMAT);
> +
> +	val = simple_util_get_sample_fmt(data);
> +	if (val >= 0) {

Maybe nothing that we need to worry about, but this could be potentially
a problem if the snd_pcm_format_t ever outgrows the non-negative number
space. snd_pcm_format_t is defined to be an int as well, so any very
large number would wrap into a negative value and then would be
considered an error in this check.

Then again, if that ever were to happen, we'd likely get a compiler
error for the snd_pcm_format_t overflowing, so we'd probably notice and
rewrite at that point.

So I suppose:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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

[PATCH v2 2/2] ASoC: tegra: I2S client convert formats handling

[Sameer Pujar]

From: Mohan Kumar <mkumard@nvidia.com>

The AHUB HW modules are interconnected with CIF which are capable of
supporting Channel and Sample bit format conversion. Due to this, the
I2S Client can have different Channel and Sample bit from the hw_params()
and this config is passed from CIF port of I2S DT node which can help to
perform this conversion.

For e.g. HFP usecase consists of BT SCO with 1ch and 8k audio data
which needs to be converted and mixed with external codec playback and
capture path which is of 2ch and 48k format.

For HFP Playback:
  The path includes mono to stereo and 8k to 48k conversion
    _ _ _ _       _ _ _          _ _ _        _ _ _ _ _       _ _ _ _ _ _
   |       |1ch  |      | 1ch   |     |2ch   | SFC     | 2ch |           |
   |BT SCO |---->| I2Sx |------>| CIF |----->| 8k->48k |---->| Codec I2Sy|
   |_ _ _ _|     |_ _ __|client |_ _ _|audio |_ _ _ _ _|     |_ _ _ _ _ _|

For HFP Capture:
  The path includes stereo to mono and 48k to 8k conversion
    _ _ _ _ _ _       _ _ _ _ _        _ _ _         _ _ _ _       _ _ _ _
   |           | 2ch | SFC     | 2ch  |     | 1ch   |       | 1ch |       |
   | codec I2Sy|---->| 48k->8k |----->| CIF |------>| I2Sx  |---->| BT SCO|
   |_ _ _ _ _ _|     |_ _ _ _ _| audio|_ _ _|client |_ _ _ _|     |_ _ _ _|

For above two path, I2S client channel uses existing DT binding to pass
channels and format conversion in I2Sx CIF Port.

Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
Signed-off-by: Sameer Pujar <spujar@nvidia.com>
---
 sound/soc/tegra/tegra210_i2s.c | 71 ++++++++++++++++++++++++++++++++--
 sound/soc/tegra/tegra210_i2s.h |  2 +
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/sound/soc/tegra/tegra210_i2s.c b/sound/soc/tegra/tegra210_i2s.c
index ba7fdd7405ac..fe4fde844d86 100644
--- a/sound/soc/tegra/tegra210_i2s.c
+++ b/sound/soc/tegra/tegra210_i2s.c
@@ -8,11 +8,13 @@
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/of_graph.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <sound/core.h>
 #include <sound/pcm_params.h>
+#include <sound/simple_card_utils.h>
 #include <sound/soc.h>
 #include "tegra210_i2s.h"
 #include "tegra_cif.h"
@@ -603,6 +605,7 @@ static int tegra210_i2s_hw_params(struct snd_pcm_substream *substream,
 	struct tegra210_i2s *i2s = snd_soc_dai_get_drvdata(dai);
 	unsigned int sample_size, channels, srate, val, reg, path;
 	struct tegra_cif_conf cif_conf;
+	snd_pcm_format_t sample_format;
 
 	memset(&cif_conf, 0, sizeof(struct tegra_cif_conf));
 
@@ -615,28 +618,51 @@ static int tegra210_i2s_hw_params(struct snd_pcm_substream *substream,
 
 	cif_conf.audio_ch = channels;
 	cif_conf.client_ch = channels;
+	if (i2s->client_channels)
+		cif_conf.client_ch = i2s->client_channels;
 
+	/* AHUB CIF Audio bits configs */
 	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_S8:
+		cif_conf.audio_bits = TEGRA_ACIF_BITS_8;
+		break;
+	case SNDRV_PCM_FORMAT_S16_LE:
+		cif_conf.audio_bits = TEGRA_ACIF_BITS_16;
+		break;
+	case SNDRV_PCM_FORMAT_S32_LE:
+		cif_conf.audio_bits = TEGRA_ACIF_BITS_32;
+		break;
+	default:
+		dev_err(dev, "unsupported params audio bit format!\n");
+		return -EOPNOTSUPP;
+	}
+
+	sample_format = params_format(params);
+	if (i2s->client_sample_format >= 0)
+		sample_format = (snd_pcm_format_t)i2s->client_sample_format;
+
+	/*
+	 * Format of the I2S for sending/receiving the audio
+	 * to/from external device.
+	 */
+	switch (sample_format) {
 	case SNDRV_PCM_FORMAT_S8:
 		val = I2S_BITS_8;
 		sample_size = 8;
-		cif_conf.audio_bits = TEGRA_ACIF_BITS_8;
 		cif_conf.client_bits = TEGRA_ACIF_BITS_8;
 		break;
 	case SNDRV_PCM_FORMAT_S16_LE:
 		val = I2S_BITS_16;
 		sample_size = 16;
-		cif_conf.audio_bits = TEGRA_ACIF_BITS_16;
 		cif_conf.client_bits = TEGRA_ACIF_BITS_16;
 		break;
 	case SNDRV_PCM_FORMAT_S32_LE:
 		val = I2S_BITS_32;
 		sample_size = 32;
-		cif_conf.audio_bits = TEGRA_ACIF_BITS_32;
 		cif_conf.client_bits = TEGRA_ACIF_BITS_32;
 		break;
 	default:
-		dev_err(dev, "unsupported format!\n");
+		dev_err(dev, "unsupported client bit format!\n");
 		return -EOPNOTSUPP;
 	}
 
@@ -872,6 +898,40 @@ static const struct regmap_config tegra210_i2s_regmap_config = {
 	.cache_type		= REGCACHE_FLAT,
 };
 
+/*
+ * The AHUB HW modules are interconnected with CIF which are capable of
+ * supporting Channel and Sample bit format conversion. This needs different
+ * CIF Audio and client configuration. As one of the config comes from
+ * params_channels() or params_format(), the extra configuration is passed from
+ * CIF Port of DT I2S node which can help to perform this conversion.
+ *
+ *    4ch          audio = 4ch      client = 2ch       2ch
+ *   -----> ADMAIF -----------> CIF -------------> I2S ---->
+ */
+static void tegra210_parse_client_convert(struct device *dev)
+{
+	struct tegra210_i2s *i2s = dev_get_drvdata(dev);
+	struct device_node *ports, *ep;
+	struct simple_util_data data = {};
+	int cif_port = 0;
+
+	ports = of_get_child_by_name(dev->of_node, "ports");
+	if (ports) {
+		ep = of_graph_get_endpoint_by_regs(ports, cif_port, -1);
+		if (ep) {
+			simple_util_parse_convert(ep, NULL, &data);
+			of_node_put(ep);
+		}
+		of_node_put(ports);
+	}
+
+	if (data.convert_channels)
+		i2s->client_channels = data.convert_channels;
+
+	if (data.convert_sample_format)
+		i2s->client_sample_format = simple_util_get_sample_fmt(&data);
+}
+
 static int tegra210_i2s_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -887,6 +947,7 @@ static int tegra210_i2s_probe(struct platform_device *pdev)
 	i2s->tx_mask = DEFAULT_I2S_SLOT_MASK;
 	i2s->rx_mask = DEFAULT_I2S_SLOT_MASK;
 	i2s->loopback = false;
+	i2s->client_sample_format = -EINVAL;
 
 	dev_set_drvdata(dev, i2s);
 
@@ -916,6 +977,8 @@ static int tegra210_i2s_probe(struct platform_device *pdev)
 		return PTR_ERR(i2s->regmap);
 	}
 
+	tegra210_parse_client_convert(dev);
+
 	regcache_cache_only(i2s->regmap, true);
 
 	err = devm_snd_soc_register_component(dev, &tegra210_i2s_cmpnt,
diff --git a/sound/soc/tegra/tegra210_i2s.h b/sound/soc/tegra/tegra210_i2s.h
index 030d70c45e18..fe478f3d8435 100644
--- a/sound/soc/tegra/tegra210_i2s.h
+++ b/sound/soc/tegra/tegra210_i2s.h
@@ -112,6 +112,8 @@ struct tegra210_i2s {
 	struct clk *clk_i2s;
 	struct clk *clk_sync_input;
 	struct regmap *regmap;
+	int client_sample_format;
+	unsigned int client_channels;
 	unsigned int stereo_to_mono[I2S_PATHS];
 	unsigned int mono_to_stereo[I2S_PATHS];
 	unsigned int dai_fmt;
-- 
2.45.1

Re: [PATCH v2 2/2] ASoC: tegra: I2S client convert formats handling

[Thierry Reding]

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

On Mon May 27, 2024 at 2:56 PM CEST, Sameer Pujar wrote:
> From: Mohan Kumar <mkumard@nvidia.com>
>
> The AHUB HW modules are interconnected with CIF which are capable of
> supporting Channel and Sample bit format conversion. Due to this, the
> I2S Client can have different Channel and Sample bit from the hw_params()
> and this config is passed from CIF port of I2S DT node which can help to
> perform this conversion.
>
> For e.g. HFP usecase consists of BT SCO with 1ch and 8k audio data
> which needs to be converted and mixed with external codec playback and
> capture path which is of 2ch and 48k format.
>
> For HFP Playback:
>   The path includes mono to stereo and 8k to 48k conversion
>     _ _ _ _       _ _ _          _ _ _        _ _ _ _ _       _ _ _ _ _ _
>    |       |1ch  |      | 1ch   |     |2ch   | SFC     | 2ch |           |
>    |BT SCO |---->| I2Sx |------>| CIF |----->| 8k->48k |---->| Codec I2Sy|
>    |_ _ _ _|     |_ _ __|client |_ _ _|audio |_ _ _ _ _|     |_ _ _ _ _ _|
>
> For HFP Capture:
>   The path includes stereo to mono and 48k to 8k conversion
>     _ _ _ _ _ _       _ _ _ _ _        _ _ _         _ _ _ _       _ _ _ _
>    |           | 2ch | SFC     | 2ch  |     | 1ch   |       | 1ch |       |
>    | codec I2Sy|---->| 48k->8k |----->| CIF |------>| I2Sx  |---->| BT SCO|
>    |_ _ _ _ _ _|     |_ _ _ _ _| audio|_ _ _|client |_ _ _ _|     |_ _ _ _|
>
> For above two path, I2S client channel uses existing DT binding to pass
> channels and format conversion in I2Sx CIF Port.
>
> Signed-off-by: Mohan Kumar <mkumard@nvidia.com>
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  sound/soc/tegra/tegra210_i2s.c | 71 ++++++++++++++++++++++++++++++++--
>  sound/soc/tegra/tegra210_i2s.h |  2 +
>  2 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/tegra/tegra210_i2s.c b/sound/soc/tegra/tegra210_i2s.c
> index ba7fdd7405ac..fe4fde844d86 100644
> --- a/sound/soc/tegra/tegra210_i2s.c
> +++ b/sound/soc/tegra/tegra210_i2s.c
> @@ -8,11 +8,13 @@
>  #include <linux/device.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> +#include <linux/of_graph.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/regmap.h>
>  #include <sound/core.h>
>  #include <sound/pcm_params.h>
> +#include <sound/simple_card_utils.h>
>  #include <sound/soc.h>
>  #include "tegra210_i2s.h"
>  #include "tegra_cif.h"
> @@ -603,6 +605,7 @@ static int tegra210_i2s_hw_params(struct snd_pcm_substream *substream,
>  	struct tegra210_i2s *i2s = snd_soc_dai_get_drvdata(dai);
>  	unsigned int sample_size, channels, srate, val, reg, path;
>  	struct tegra_cif_conf cif_conf;
> +	snd_pcm_format_t sample_format;
>  
>  	memset(&cif_conf, 0, sizeof(struct tegra_cif_conf));
>  
> @@ -615,28 +618,51 @@ static int tegra210_i2s_hw_params(struct snd_pcm_substream *substream,
>  
>  	cif_conf.audio_ch = channels;
>  	cif_conf.client_ch = channels;
> +	if (i2s->client_channels)
> +		cif_conf.client_ch = i2s->client_channels;
>  
> +	/* AHUB CIF Audio bits configs */
>  	switch (params_format(params)) {
> +	case SNDRV_PCM_FORMAT_S8:
> +		cif_conf.audio_bits = TEGRA_ACIF_BITS_8;
> +		break;
> +	case SNDRV_PCM_FORMAT_S16_LE:
> +		cif_conf.audio_bits = TEGRA_ACIF_BITS_16;
> +		break;
> +	case SNDRV_PCM_FORMAT_S32_LE:
> +		cif_conf.audio_bits = TEGRA_ACIF_BITS_32;
> +		break;
> +	default:
> +		dev_err(dev, "unsupported params audio bit format!\n");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	sample_format = params_format(params);

Maybe you can move this before the switch and then reuse sample_format
in the switch?

> +	if (i2s->client_sample_format >= 0)
> +		sample_format = (snd_pcm_format_t)i2s->client_sample_format;
> +
> +	/*
> +	 * Format of the I2S for sending/receiving the audio
> +	 * to/from external device.
> +	 */
> +	switch (sample_format) {
>  	case SNDRV_PCM_FORMAT_S8:
>  		val = I2S_BITS_8;
>  		sample_size = 8;
> -		cif_conf.audio_bits = TEGRA_ACIF_BITS_8;
>  		cif_conf.client_bits = TEGRA_ACIF_BITS_8;
>  		break;
>  	case SNDRV_PCM_FORMAT_S16_LE:
>  		val = I2S_BITS_16;
>  		sample_size = 16;
> -		cif_conf.audio_bits = TEGRA_ACIF_BITS_16;
>  		cif_conf.client_bits = TEGRA_ACIF_BITS_16;
>  		break;
>  	case SNDRV_PCM_FORMAT_S32_LE:
>  		val = I2S_BITS_32;
>  		sample_size = 32;
> -		cif_conf.audio_bits = TEGRA_ACIF_BITS_32;
>  		cif_conf.client_bits = TEGRA_ACIF_BITS_32;
>  		break;
>  	default:
> -		dev_err(dev, "unsupported format!\n");
> +		dev_err(dev, "unsupported client bit format!\n");
>  		return -EOPNOTSUPP;
>  	}
>  
> @@ -872,6 +898,40 @@ static const struct regmap_config tegra210_i2s_regmap_config = {
>  	.cache_type		= REGCACHE_FLAT,
>  };
>  
> +/*
> + * The AHUB HW modules are interconnected with CIF which are capable of
> + * supporting Channel and Sample bit format conversion. This needs different
> + * CIF Audio and client configuration. As one of the config comes from
> + * params_channels() or params_format(), the extra configuration is passed from
> + * CIF Port of DT I2S node which can help to perform this conversion.
> + *
> + *    4ch          audio = 4ch      client = 2ch       2ch
> + *   -----> ADMAIF -----------> CIF -------------> I2S ---->
> + */
> +static void tegra210_parse_client_convert(struct device *dev)
> +{
> +	struct tegra210_i2s *i2s = dev_get_drvdata(dev);
> +	struct device_node *ports, *ep;
> +	struct simple_util_data data = {};
> +	int cif_port = 0;
> +
> +	ports = of_get_child_by_name(dev->of_node, "ports");
> +	if (ports) {
> +		ep = of_graph_get_endpoint_by_regs(ports, cif_port, -1);
> +		if (ep) {
> +			simple_util_parse_convert(ep, NULL, &data);
> +			of_node_put(ep);
> +		}
> +		of_node_put(ports);

You could maybe try to use the "new" __free() anotation here to have
this cleaned up automatically. It'd look something like this:

	struct device_node *ports __free(device_node), *ep __free(device_node);

And then you should be able to omit both of those of_node_put() calls.
It may not be worth doing in this case because the declaration becomes
unwieldy and there aren't a lot of these calls.

> +	}
> +
> +	if (data.convert_channels)
> +		i2s->client_channels = data.convert_channels;
> +
> +	if (data.convert_sample_format)
> +		i2s->client_sample_format = simple_util_get_sample_fmt(&data);
> +}
> +
>  static int tegra210_i2s_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -887,6 +947,7 @@ static int tegra210_i2s_probe(struct platform_device *pdev)
>  	i2s->tx_mask = DEFAULT_I2S_SLOT_MASK;
>  	i2s->rx_mask = DEFAULT_I2S_SLOT_MASK;
>  	i2s->loopback = false;
> +	i2s->client_sample_format = -EINVAL;
>  
>  	dev_set_drvdata(dev, i2s);
>  
> @@ -916,6 +977,8 @@ static int tegra210_i2s_probe(struct platform_device *pdev)
>  		return PTR_ERR(i2s->regmap);
>  	}
>  
> +	tegra210_parse_client_convert(dev);
> +
>  	regcache_cache_only(i2s->regmap, true);
>  
>  	err = devm_snd_soc_register_component(dev, &tegra210_i2s_cmpnt,
> diff --git a/sound/soc/tegra/tegra210_i2s.h b/sound/soc/tegra/tegra210_i2s.h
> index 030d70c45e18..fe478f3d8435 100644
> --- a/sound/soc/tegra/tegra210_i2s.h
> +++ b/sound/soc/tegra/tegra210_i2s.h
> @@ -112,6 +112,8 @@ struct tegra210_i2s {
>  	struct clk *clk_i2s;
>  	struct clk *clk_sync_input;
>  	struct regmap *regmap;
> +	int client_sample_format;

Given that snd_pcm_format_t is defined to be int, can you store that in
here directly? That way you only need to cast when getting the sample
format.

Anyway, very small nitpicks, so either way:

Reviewed-by: Thierry Reding <treding@nvidia.com>

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