[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

The DAI link format should be specified for the whole link rather than just
one component on the link. So move the format specification for the HDMI
audio link from the CPU component to the link itself.

Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
in this case there is no functional difference between only specifying the
the format for the CPU side or for the whole link, but the later it will
allow us to remove support for just specifying the format for one component.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1.
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 6d949f1..75de26c 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
 	.cpu_dai = {
 		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},
 	.codec_dai = {
 		.name = "sh_mobile_hdmi-hifi",
-- 
1.8.0

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

The DAI link format should be specified for the whole link rather than just
one component on the link. So move the format specification for the HDMI
audio link from the CPU component to the link itself.

Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
in this case there is no functional difference between only specifying the
the format for the CPU side or for the whole link, but the later it will
allow us to remove support for just specifying the format for one component.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1.
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 6d949f1..75de26c 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
 	.cpu_dai = {
 		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},
 	.codec_dai = {
 		.name = "sh_mobile_hdmi-hifi",
-- 
1.8.0

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

The DAI link format should be specified for the whole link rather than just
one component on the link. So move the format specification for the HDMI
audio link from the CPU component to the link itself.

Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
in this case there is no functional difference between only specifying the
the format for the CPU side or for the whole link, but the later it will
allow us to remove support for just specifying the format for one component.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
No changes since v1.
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 6d949f1..75de26c 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
 	.cpu_dai = {
 		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},
 	.codec_dai = {
 		.name = "sh_mobile_hdmi-hifi",
-- 
1.8.0

[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion

[Lars-Peter Clausen]

According to the sh7724 hardware user manual (Rev.2.00 Jan 2013) page 1851
to 1856 the FSI bit-clock is inverted to the bit-clock as specified by the
I2S standard. This means the bit clock inversion bit should be set for a
normal I2S clock and should not be set for an inverted I2S clock.

Similarly when operating in left-justfied mode both the frame-clock and the
bit-clock need to be inverted to be standards compliant.

This means also that the exta clock inversion setting in the armadillo800eva
machine driver for CPU component should now be removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
I don't have the hardware but I'd like to get rid of the extra
SND_SOC_DAIFMT_IB_NF in simple-card platform data, so we can remove the fmt
field. Kuninori can you check if this works?

Changes since v1:
	* Fix whitespace
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 1 -
 sound/soc/sh/fsi.c                             | 7 +++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
 	.platform	= "sh_fsi2",
 	.daifmt		= SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
 	.cpu_dai = {
-		.fmt	= SND_SOC_DAIFMT_IB_NF,
 		.name	= "fsia-dai",
 	},
 	.codec_dai = {
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index b87b22e..6783d3f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1594,6 +1594,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 {
+	/*
+	 * FSI bit clock is inverted to the I2S specification, so we invert it
+	 * when a non-inverted I2S clock was requested and vice versa.
+	 */
+	fsi->bit_clk_inv = !fsi->bit_clk_inv;
+
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
 		fsi->fmt = CR_I2S;
@@ -1602,6 +1608,7 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 	case SND_SOC_DAIFMT_LEFT_J:
 		fsi->fmt = CR_PCM;
 		fsi->chan_num = 2;
+		fsi->lr_clk_inv = !fsi->lr_clk_inv;
 		break;
 	default:
 		return -EINVAL;
-- 
1.8.0

[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion

[Lars-Peter Clausen]

According to the sh7724 hardware user manual (Rev.2.00 Jan 2013) page 1851
to 1856 the FSI bit-clock is inverted to the bit-clock as specified by the
I2S standard. This means the bit clock inversion bit should be set for a
normal I2S clock and should not be set for an inverted I2S clock.

Similarly when operating in left-justfied mode both the frame-clock and the
bit-clock need to be inverted to be standards compliant.

This means also that the exta clock inversion setting in the armadillo800eva
machine driver for CPU component should now be removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
I don't have the hardware but I'd like to get rid of the extra
SND_SOC_DAIFMT_IB_NF in simple-card platform data, so we can remove the fmt
field. Kuninori can you check if this works?

Changes since v1:
	* Fix whitespace
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 1 -
 sound/soc/sh/fsi.c                             | 7 +++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
 	.platform	= "sh_fsi2",
 	.daifmt		= SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
 	.cpu_dai = {
-		.fmt	= SND_SOC_DAIFMT_IB_NF,
 		.name	= "fsia-dai",
 	},
 	.codec_dai = {
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index b87b22e..6783d3f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1594,6 +1594,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 {
+	/*
+	 * FSI bit clock is inverted to the I2S specification, so we invert it
+	 * when a non-inverted I2S clock was requested and vice versa.
+	 */
+	fsi->bit_clk_inv = !fsi->bit_clk_inv;
+
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
 		fsi->fmt = CR_I2S;
@@ -1602,6 +1608,7 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 	case SND_SOC_DAIFMT_LEFT_J:
 		fsi->fmt = CR_PCM;
 		fsi->chan_num = 2;
+		fsi->lr_clk_inv = !fsi->lr_clk_inv;
 		break;
 	default:
 		return -EINVAL;
-- 
1.8.0

[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion

[Lars-Peter Clausen]

According to the sh7724 hardware user manual (Rev.2.00 Jan 2013) page 1851
to 1856 the FSI bit-clock is inverted to the bit-clock as specified by the
I2S standard. This means the bit clock inversion bit should be set for a
normal I2S clock and should not be set for an inverted I2S clock.

Similarly when operating in left-justfied mode both the frame-clock and the
bit-clock need to be inverted to be standards compliant.

This means also that the exta clock inversion setting in the armadillo800eva
machine driver for CPU component should now be removed.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
I don't have the hardware but I'd like to get rid of the extra
SND_SOC_DAIFMT_IB_NF in simple-card platform data, so we can remove the fmt
field. Kuninori can you check if this works?

Changes since v1:
	* Fix whitespace
---
 arch/arm/mach-shmobile/board-armadillo800eva.c | 1 -
 sound/soc/sh/fsi.c                             | 7 +++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
 	.platform	= "sh_fsi2",
 	.daifmt		= SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
 	.cpu_dai = {
-		.fmt	= SND_SOC_DAIFMT_IB_NF,
 		.name	= "fsia-dai",
 	},
 	.codec_dai = {
diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
index b87b22e..6783d3f 100644
--- a/sound/soc/sh/fsi.c
+++ b/sound/soc/sh/fsi.c
@@ -1594,6 +1594,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
 
 static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 {
+	/*
+	 * FSI bit clock is inverted to the I2S specification, so we invert it
+	 * when a non-inverted I2S clock was requested and vice versa.
+	 */
+	fsi->bit_clk_inv = !fsi->bit_clk_inv;
+
 	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
 	case SND_SOC_DAIFMT_I2S:
 		fsi->fmt = CR_I2S;
@@ -1602,6 +1608,7 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
 	case SND_SOC_DAIFMT_LEFT_J:
 		fsi->fmt = CR_PCM;
 		fsi->chan_num = 2;
+		fsi->lr_clk_inv = !fsi->lr_clk_inv;
 		break;
 	default:
 		return -EINVAL;
-- 
1.8.0

[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

[Lars-Peter Clausen]

Having to set different formats on the CPU side and the CODEC side of a DAI
link is usually indication that something is terribly wrong and in most
cases is a result of a broken driver that implements a set_fmt() callback
which does not follow the specification. In the past this feature has been
used to work around broken drivers, rather than fixing them. We don't really
want to encourage this, so remove support for setting different formats on
both ends of the link.

Along the way switch to static DAI format setup by setting the the dai_fmt
field of the snd_soc_dai_link rather than calling snd_soc_dai_fmt().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
	* dev_dbg(dai_fmt)
---
 include/sound/simple_card.h     |  1 -
 sound/soc/generic/simple-card.c | 30 ++++++++----------------------
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 1255ddb..b9b4f28 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -16,7 +16,6 @@
 
 struct asoc_simple_dai {
 	const char *name;
-	unsigned int fmt;
 	unsigned int sysclk;
 	int slots;
 	int slot_width;
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index f7c6734..3efd947 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -125,14 +125,6 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 {
 	int ret;
 
-	if (set->fmt) {
-		ret = snd_soc_dai_set_fmt(dai, set->fmt);
-		if (ret && ret != -ENOTSUPP) {
-			dev_err(dai->dev, "simple-card: set_fmt error\n");
-			goto err;
-		}
-	}
-
 	if (set->sysclk) {
 		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
 		if (ret && ret != -ENOTSUPP) {
@@ -269,12 +261,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 					 struct device_node *codec,
 					 char *prefix, int idx)
 {
+	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *bitclkmaster = NULL;
 	struct device_node *framemaster = NULL;
-	struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
-	struct asoc_simple_dai *cpu_dai = &dai_props->cpu_dai;
-	struct asoc_simple_dai *codec_dai = &dai_props->codec_dai;
 	unsigned int daifmt;
 
 	daifmt = snd_soc_of_parse_daifmt(node, prefix,
@@ -289,8 +279,7 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		 */
 		dev_dbg(dev, "Revert to legacy daifmt parsing\n");
 
-		cpu_dai->fmt = codec_dai->fmt -			snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+		daifmt = snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
 			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
 	} else {
 		if (codec = bitclkmaster)
@@ -299,11 +288,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		else
 			daifmt |= (codec = framemaster) ?
 				SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
-
-		cpu_dai->fmt	= daifmt;
-		codec_dai->fmt	= daifmt;
 	}
 
+	dai_link->dai_fmt = daifmt;
+
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
 
@@ -379,13 +367,12 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	dai_link->init = asoc_simple_card_dai_init;
 
 	dev_dbg(dev, "\tname : %s\n", dai_link->stream_name);
-	dev_dbg(dev, "\tcpu : %s / %04x / %d\n",
+	dev_dbg(dev, "\tformat : %04x\n", dai_link->dai_fmt);
+	dev_dbg(dev, "\tcpu : %s / %d\n",
 		dai_link->cpu_dai_name,
-		dai_props->cpu_dai.fmt,
 		dai_props->cpu_dai.sysclk);
-	dev_dbg(dev, "\tcodec : %s / %04x / %d\n",
+	dev_dbg(dev, "\tcodec : %s / %d\n",
 		dai_link->codec_dai_name,
-		dai_props->codec_dai.fmt,
 		dai_props->codec_dai.sysclk);
 
 	/*
@@ -572,14 +559,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 		dai_link->codec_name	= cinfo->codec;
 		dai_link->cpu_dai_name	= cinfo->cpu_dai.name;
 		dai_link->codec_dai_name = cinfo->codec_dai.name;
+		dai_link->dai_fmt	= cinfo->daifmt;
 		dai_link->init		= asoc_simple_card_dai_init;
 		memcpy(&priv->dai_props->cpu_dai, &cinfo->cpu_dai,
 					sizeof(priv->dai_props->cpu_dai));
 		memcpy(&priv->dai_props->codec_dai, &cinfo->codec_dai,
 					sizeof(priv->dai_props->codec_dai));
 
-		priv->dai_props->cpu_dai.fmt	|= cinfo->daifmt;
-		priv->dai_props->codec_dai.fmt	|= cinfo->daifmt;
 	}
 
 	snd_soc_card_set_drvdata(&priv->snd_card, priv);
-- 
1.8.0

[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

[Lars-Peter Clausen]

Having to set different formats on the CPU side and the CODEC side of a DAI
link is usually indication that something is terribly wrong and in most
cases is a result of a broken driver that implements a set_fmt() callback
which does not follow the specification. In the past this feature has been
used to work around broken drivers, rather than fixing them. We don't really
want to encourage this, so remove support for setting different formats on
both ends of the link.

Along the way switch to static DAI format setup by setting the the dai_fmt
field of the snd_soc_dai_link rather than calling snd_soc_dai_fmt().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
	* dev_dbg(dai_fmt)
---
 include/sound/simple_card.h     |  1 -
 sound/soc/generic/simple-card.c | 30 ++++++++----------------------
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 1255ddb..b9b4f28 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -16,7 +16,6 @@
 
 struct asoc_simple_dai {
 	const char *name;
-	unsigned int fmt;
 	unsigned int sysclk;
 	int slots;
 	int slot_width;
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index f7c6734..3efd947 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -125,14 +125,6 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 {
 	int ret;
 
-	if (set->fmt) {
-		ret = snd_soc_dai_set_fmt(dai, set->fmt);
-		if (ret && ret != -ENOTSUPP) {
-			dev_err(dai->dev, "simple-card: set_fmt error\n");
-			goto err;
-		}
-	}
-
 	if (set->sysclk) {
 		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
 		if (ret && ret != -ENOTSUPP) {
@@ -269,12 +261,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 					 struct device_node *codec,
 					 char *prefix, int idx)
 {
+	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *bitclkmaster = NULL;
 	struct device_node *framemaster = NULL;
-	struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
-	struct asoc_simple_dai *cpu_dai = &dai_props->cpu_dai;
-	struct asoc_simple_dai *codec_dai = &dai_props->codec_dai;
 	unsigned int daifmt;
 
 	daifmt = snd_soc_of_parse_daifmt(node, prefix,
@@ -289,8 +279,7 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		 */
 		dev_dbg(dev, "Revert to legacy daifmt parsing\n");
 
-		cpu_dai->fmt = codec_dai->fmt =
-			snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+		daifmt = snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
 			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
 	} else {
 		if (codec == bitclkmaster)
@@ -299,11 +288,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		else
 			daifmt |= (codec == framemaster) ?
 				SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
-
-		cpu_dai->fmt	= daifmt;
-		codec_dai->fmt	= daifmt;
 	}
 
+	dai_link->dai_fmt = daifmt;
+
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
 
@@ -379,13 +367,12 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	dai_link->init = asoc_simple_card_dai_init;
 
 	dev_dbg(dev, "\tname : %s\n", dai_link->stream_name);
-	dev_dbg(dev, "\tcpu : %s / %04x / %d\n",
+	dev_dbg(dev, "\tformat : %04x\n", dai_link->dai_fmt);
+	dev_dbg(dev, "\tcpu : %s / %d\n",
 		dai_link->cpu_dai_name,
-		dai_props->cpu_dai.fmt,
 		dai_props->cpu_dai.sysclk);
-	dev_dbg(dev, "\tcodec : %s / %04x / %d\n",
+	dev_dbg(dev, "\tcodec : %s / %d\n",
 		dai_link->codec_dai_name,
-		dai_props->codec_dai.fmt,
 		dai_props->codec_dai.sysclk);
 
 	/*
@@ -572,14 +559,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 		dai_link->codec_name	= cinfo->codec;
 		dai_link->cpu_dai_name	= cinfo->cpu_dai.name;
 		dai_link->codec_dai_name = cinfo->codec_dai.name;
+		dai_link->dai_fmt	= cinfo->daifmt;
 		dai_link->init		= asoc_simple_card_dai_init;
 		memcpy(&priv->dai_props->cpu_dai, &cinfo->cpu_dai,
 					sizeof(priv->dai_props->cpu_dai));
 		memcpy(&priv->dai_props->codec_dai, &cinfo->codec_dai,
 					sizeof(priv->dai_props->codec_dai));
 
-		priv->dai_props->cpu_dai.fmt	|= cinfo->daifmt;
-		priv->dai_props->codec_dai.fmt	|= cinfo->daifmt;
 	}
 
 	snd_soc_card_set_drvdata(&priv->snd_card, priv);
-- 
1.8.0

[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

[Lars-Peter Clausen]

Having to set different formats on the CPU side and the CODEC side of a DAI
link is usually indication that something is terribly wrong and in most
cases is a result of a broken driver that implements a set_fmt() callback
which does not follow the specification. In the past this feature has been
used to work around broken drivers, rather than fixing them. We don't really
want to encourage this, so remove support for setting different formats on
both ends of the link.

Along the way switch to static DAI format setup by setting the the dai_fmt
field of the snd_soc_dai_link rather than calling snd_soc_dai_fmt().

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Changes since v1:
	* dev_dbg(dai_fmt)
---
 include/sound/simple_card.h     |  1 -
 sound/soc/generic/simple-card.c | 30 ++++++++----------------------
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 1255ddb..b9b4f28 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -16,7 +16,6 @@
 
 struct asoc_simple_dai {
 	const char *name;
-	unsigned int fmt;
 	unsigned int sysclk;
 	int slots;
 	int slot_width;
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index f7c6734..3efd947 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -125,14 +125,6 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 {
 	int ret;
 
-	if (set->fmt) {
-		ret = snd_soc_dai_set_fmt(dai, set->fmt);
-		if (ret && ret != -ENOTSUPP) {
-			dev_err(dai->dev, "simple-card: set_fmt error\n");
-			goto err;
-		}
-	}
-
 	if (set->sysclk) {
 		ret = snd_soc_dai_set_sysclk(dai, 0, set->sysclk, 0);
 		if (ret && ret != -ENOTSUPP) {
@@ -269,12 +261,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 					 struct device_node *codec,
 					 char *prefix, int idx)
 {
+	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
 	struct device *dev = simple_priv_to_dev(priv);
 	struct device_node *bitclkmaster = NULL;
 	struct device_node *framemaster = NULL;
-	struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
-	struct asoc_simple_dai *cpu_dai = &dai_props->cpu_dai;
-	struct asoc_simple_dai *codec_dai = &dai_props->codec_dai;
 	unsigned int daifmt;
 
 	daifmt = snd_soc_of_parse_daifmt(node, prefix,
@@ -289,8 +279,7 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		 */
 		dev_dbg(dev, "Revert to legacy daifmt parsing\n");
 
-		cpu_dai->fmt = codec_dai->fmt =
-			snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
+		daifmt = snd_soc_of_parse_daifmt(codec, NULL, NULL, NULL) |
 			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
 	} else {
 		if (codec == bitclkmaster)
@@ -299,11 +288,10 @@ static int asoc_simple_card_parse_daifmt(struct device_node *node,
 		else
 			daifmt |= (codec == framemaster) ?
 				SND_SOC_DAIFMT_CBS_CFM : SND_SOC_DAIFMT_CBS_CFS;
-
-		cpu_dai->fmt	= daifmt;
-		codec_dai->fmt	= daifmt;
 	}
 
+	dai_link->dai_fmt = daifmt;
+
 	of_node_put(bitclkmaster);
 	of_node_put(framemaster);
 
@@ -379,13 +367,12 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	dai_link->init = asoc_simple_card_dai_init;
 
 	dev_dbg(dev, "\tname : %s\n", dai_link->stream_name);
-	dev_dbg(dev, "\tcpu : %s / %04x / %d\n",
+	dev_dbg(dev, "\tformat : %04x\n", dai_link->dai_fmt);
+	dev_dbg(dev, "\tcpu : %s / %d\n",
 		dai_link->cpu_dai_name,
-		dai_props->cpu_dai.fmt,
 		dai_props->cpu_dai.sysclk);
-	dev_dbg(dev, "\tcodec : %s / %04x / %d\n",
+	dev_dbg(dev, "\tcodec : %s / %d\n",
 		dai_link->codec_dai_name,
-		dai_props->codec_dai.fmt,
 		dai_props->codec_dai.sysclk);
 
 	/*
@@ -572,14 +559,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 		dai_link->codec_name	= cinfo->codec;
 		dai_link->cpu_dai_name	= cinfo->cpu_dai.name;
 		dai_link->codec_dai_name = cinfo->codec_dai.name;
+		dai_link->dai_fmt	= cinfo->daifmt;
 		dai_link->init		= asoc_simple_card_dai_init;
 		memcpy(&priv->dai_props->cpu_dai, &cinfo->cpu_dai,
 					sizeof(priv->dai_props->cpu_dai));
 		memcpy(&priv->dai_props->codec_dai, &cinfo->codec_dai,
 					sizeof(priv->dai_props->codec_dai));
 
-		priv->dai_props->cpu_dai.fmt	|= cinfo->daifmt;
-		priv->dai_props->codec_dai.fmt	|= cinfo->daifmt;
 	}
 
 	snd_soc_card_set_drvdata(&priv->snd_card, priv);
-- 
1.8.0

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Wed, Jan 21, 2015 at 02:38:10PM +0100, Lars-Peter Clausen wrote:
> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.

Morimoto-san,

could you provide a review of this patch?

> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Wed, Jan 21, 2015 at 02:38:10PM +0100, Lars-Peter Clausen wrote:
> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.

Morimoto-san,

could you provide a review of this patch?

> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Wed, Jan 21, 2015 at 02:38:10PM +0100, Lars-Peter Clausen wrote:
> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.

Morimoto-san,

could you provide a review of this patch?

> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

Thank you for your patch

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.
> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",

I guess if you want to use common .daifmt, it should be same as "codec" side.
(cpu side "S" means, codec / daifmt side "M")
So, this mean above should be

@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   < 	.cpu_dai = {
		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},

c7a507eea1db1430476289f525f9c853d5d485e8
(ASoC: fsi: fixup SND_SOC_DAIFMT_CBx_CFx flags)
can be good example ?
(I wonder what happen on armadillo800eva ?)

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

Thank you for your patch

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.
> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",

I guess if you want to use common .daifmt, it should be same as "codec" side.
(cpu side "S" means, codec / daifmt side "M")
So, this mean above should be

@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
 	.cpu_dai = {
		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},

c7a507eea1db1430476289f525f9c853d5d485e8
(ASoC: fsi: fixup SND_SOC_DAIFMT_CBx_CFx flags)
can be good example ?
(I wonder what happen on armadillo800eva ?)

Best regards
---
Kuninori Morimoto

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

Thank you for your patch

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> No changes since v1.
> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 6d949f1..75de26c 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>  	.card		= "FSI2B-HDMI",
>  	.codec		= "sh-mobile-hdmi",
>  	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>  	.cpu_dai = {
>  		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>  	},
>  	.codec_dai = {
>  		.name = "sh_mobile_hdmi-hifi",

I guess if you want to use common .daifmt, it should be same as "codec" side.
(cpu side "S" means, codec / daifmt side "M")
So, this mean above should be

@@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
 	.card		= "FSI2B-HDMI",
 	.codec		= "sh-mobile-hdmi",
 	.platform	= "sh_fsi2",
+	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
 	.cpu_dai = {
		.name	= "fsib-dai",
-		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
 	},

c7a507eea1db1430476289f525f9c853d5d485e8
(ASoC: fsi: fixup SND_SOC_DAIFMT_CBx_CFx flags)
can be good example ?
(I wonder what happen on armadillo800eva ?)

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

On 01/27/2015 08:40 AM, Kuninori Morimoto wrote:
>
> Hi Lars
>
> Thank you for your patch
>
>> The DAI link format should be specified for the whole link rather than just
>> one component on the link. So move the format specification for the HDMI
>> audio link from the CPU component to the link itself.
>>
>> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
>> in this case there is no functional difference between only specifying the
>> the format for the CPU side or for the whole link, but the later it will
>> allow us to remove support for just specifying the format for one component.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>> No changes since v1.
>> ---
>>   arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> index 6d949f1..75de26c 100644
>> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
>> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>>   	.card		= "FSI2B-HDMI",
>>   	.codec		= "sh-mobile-hdmi",
>>   	.platform	= "sh_fsi2",
>> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>>   	.cpu_dai = {
>>   		.name	= "fsib-dai",
>> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>>   	},
>>   	.codec_dai = {
>>   		.name = "sh_mobile_hdmi-hifi",
>
> I guess if you want to use common .daifmt, it should be same as "codec" side.
> (cpu side "S" means, codec / daifmt side "M")
> So, this mean above should be
>
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>   	.card		= "FSI2B-HDMI",
>   	.codec		= "sh-mobile-hdmi",
>   	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <>   	.cpu_dai = {
> 		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>   	},

Right now the CPU is configured as master, with your proposed change it 
would be configured as slave. These flags are always from the CODEC's 
perspective.

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

On 01/27/2015 08:40 AM, Kuninori Morimoto wrote:
>
> Hi Lars
>
> Thank you for your patch
>
>> The DAI link format should be specified for the whole link rather than just
>> one component on the link. So move the format specification for the HDMI
>> audio link from the CPU component to the link itself.
>>
>> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
>> in this case there is no functional difference between only specifying the
>> the format for the CPU side or for the whole link, but the later it will
>> allow us to remove support for just specifying the format for one component.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>> No changes since v1.
>> ---
>>   arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> index 6d949f1..75de26c 100644
>> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
>> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>>   	.card		= "FSI2B-HDMI",
>>   	.codec		= "sh-mobile-hdmi",
>>   	.platform	= "sh_fsi2",
>> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>>   	.cpu_dai = {
>>   		.name	= "fsib-dai",
>> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>>   	},
>>   	.codec_dai = {
>>   		.name = "sh_mobile_hdmi-hifi",
>
> I guess if you want to use common .daifmt, it should be same as "codec" side.
> (cpu side "S" means, codec / daifmt side "M")
> So, this mean above should be
>
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>   	.card		= "FSI2B-HDMI",
>   	.codec		= "sh-mobile-hdmi",
>   	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
>   	.cpu_dai = {
> 		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>   	},

Right now the CPU is configured as master, with your proposed change it 
would be configured as slave. These flags are always from the CODEC's 
perspective.

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Lars-Peter Clausen]

On 01/27/2015 08:40 AM, Kuninori Morimoto wrote:
>
> Hi Lars
>
> Thank you for your patch
>
>> The DAI link format should be specified for the whole link rather than just
>> one component on the link. So move the format specification for the HDMI
>> audio link from the CPU component to the link itself.
>>
>> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
>> in this case there is no functional difference between only specifying the
>> the format for the CPU side or for the whole link, but the later it will
>> allow us to remove support for just specifying the format for one component.
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> ---
>> No changes since v1.
>> ---
>>   arch/arm/mach-shmobile/board-armadillo800eva.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> index 6d949f1..75de26c 100644
>> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
>> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
>> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>>   	.card		= "FSI2B-HDMI",
>>   	.codec		= "sh-mobile-hdmi",
>>   	.platform	= "sh_fsi2",
>> +	.daifmt		= SND_SOC_DAIFMT_CBS_CFS,
>>   	.cpu_dai = {
>>   		.name	= "fsib-dai",
>> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>>   	},
>>   	.codec_dai = {
>>   		.name = "sh_mobile_hdmi-hifi",
>
> I guess if you want to use common .daifmt, it should be same as "codec" side.
> (cpu side "S" means, codec / daifmt side "M")
> So, this mean above should be
>
> @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
>   	.card		= "FSI2B-HDMI",
>   	.codec		= "sh-mobile-hdmi",
>   	.platform	= "sh_fsi2",
> +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
>   	.cpu_dai = {
> 		.name	= "fsib-dai",
> -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
>   	},

Right now the CPU is configured as master, with your proposed change it 
would be configured as slave. These flags are always from the CODEC's 
perspective.

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

> > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > (cpu side "S" means, codec / daifmt side "M")
> > So, this mean above should be
> >
> > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> >   	.card		= "FSI2B-HDMI",
> >   	.codec		= "sh-mobile-hdmi",
> >   	.platform	= "sh_fsi2",
> > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <> >   	.cpu_dai = {
> > 		.name	= "fsib-dai",
> > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> >   	},
> 
> Right now the CPU is configured as master, with your proposed change it 
> would be configured as slave. These flags are always from the CODEC's 
> perspective.

I see, your patch was correct for CPU = Master

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

> > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > (cpu side "S" means, codec / daifmt side "M")
> > So, this mean above should be
> >
> > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> >   	.card		= "FSI2B-HDMI",
> >   	.codec		= "sh-mobile-hdmi",
> >   	.platform	= "sh_fsi2",
> > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
> >   	.cpu_dai = {
> > 		.name	= "fsib-dai",
> > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> >   	},
> 
> Right now the CPU is configured as master, with your proposed change it 
> would be configured as slave. These flags are always from the CODEC's 
> perspective.

I see, your patch was correct for CPU = Master

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars

> > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > (cpu side "S" means, codec / daifmt side "M")
> > So, this mean above should be
> >
> > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> >   	.card		= "FSI2B-HDMI",
> >   	.codec		= "sh-mobile-hdmi",
> >   	.platform	= "sh_fsi2",
> > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
> >   	.cpu_dai = {
> > 		.name	= "fsib-dai",
> > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> >   	},
> 
> Right now the CPU is configured as master, with your proposed change it 
> would be configured as slave. These flags are always from the CODEC's 
> perspective.

I see, your patch was correct for CPU = Master

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Tue, Jan 27, 2015 at 11:52:47PM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars
> 
> > > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > > (cpu side "S" means, codec / daifmt side "M")
> > > So, this mean above should be
> > >
> > > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> > >   	.card		= "FSI2B-HDMI",
> > >   	.codec		= "sh-mobile-hdmi",
> > >   	.platform	= "sh_fsi2",
> > > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <> > >   	.cpu_dai = {
> > > 		.name	= "fsib-dai",
> > > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> > >   	},
> > 
> > Right now the CPU is configured as master, with your proposed change it 
> > would be configured as slave. These flags are always from the CODEC's 
> > perspective.
> 
> I see, your patch was correct for CPU = Master

Morimoto-san, could you clarify if you would like me to pick up
this patch or not? (I am a bit confused.)

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Tue, Jan 27, 2015 at 11:52:47PM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars
> 
> > > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > > (cpu side "S" means, codec / daifmt side "M")
> > > So, this mean above should be
> > >
> > > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> > >   	.card		= "FSI2B-HDMI",
> > >   	.codec		= "sh-mobile-hdmi",
> > >   	.platform	= "sh_fsi2",
> > > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
> > >   	.cpu_dai = {
> > > 		.name	= "fsib-dai",
> > > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> > >   	},
> > 
> > Right now the CPU is configured as master, with your proposed change it 
> > would be configured as slave. These flags are always from the CODEC's 
> > perspective.
> 
> I see, your patch was correct for CPU = Master

Morimoto-san, could you clarify if you would like me to pick up
this patch or not? (I am a bit confused.)

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Tue, Jan 27, 2015 at 11:52:47PM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars
> 
> > > I guess if you want to use common .daifmt, it should be same as "codec" side.
> > > (cpu side "S" means, codec / daifmt side "M")
> > > So, this mean above should be
> > >
> > > @@ -1040,9 +1040,9 @@ static struct asoc_simple_card_info fsi2_hdmi_info = {
> > >   	.card		= "FSI2B-HDMI",
> > >   	.codec		= "sh-mobile-hdmi",
> > >   	.platform	= "sh_fsi2",
> > > +	.daifmt		= SND_SOC_DAIFMT_CBM_CFM,   <=
> > >   	.cpu_dai = {
> > > 		.name	= "fsib-dai",
> > > -		.fmt	= SND_SOC_DAIFMT_CBS_CFS,
> > >   	},
> > 
> > Right now the CPU is configured as master, with your proposed change it 
> > would be configured as slave. These flags are always from the CODEC's 
> > perspective.
> 
> I see, your patch was correct for CPU = Master

Morimoto-san, could you clarify if you would like me to pick up
this patch or not? (I am a bit confused.)

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars, Simon

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

I tested these 3 patches on armadillo800eva

	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

2/3 patch changed behavior for clock inversion on FSI driver.
FSI + wm8978 on armadillo800eva worked without any issues,
but, I don't know how much effect it has for other board.
We used this inversion flags on each board for historical reasons (copy-paste),
but, almost all these were not needed (except some picky board) on FSI.
Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
And unfortunately, Renesas don't use FSI anymore.
So, I think keeping current FSI driver as-is is more safety for old boards.
armadillo800eva works well with below patch (as 2/3 patch)

--------------
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
        .platform       = "sh_fsi2",
        .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
        .cpu_dai = {
-               .fmt    = SND_SOC_DAIFMT_IB_NF,
                .name   = "fsia-dai",
        },
        .codec_dai = {
--------------

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars, Simon

> The DAI link format should be specified for the whole link rather than just
> one component on the link. So move the format specification for the HDMI
> audio link from the CPU component to the link itself.
> 
> Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> in this case there is no functional difference between only specifying the
> the format for the CPU side or for the whole link, but the later it will
> allow us to remove support for just specifying the format for one component.
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---

I tested these 3 patches on armadillo800eva

	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats

Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

2/3 patch changed behavior for clock inversion on FSI driver.
FSI + wm8978 on armadillo800eva worked without any issues,
but, I don't know how much effect it has for other board.
We used this inversion flags on each board for historical reasons (copy-paste),
but, almost all these were not needed (except some picky board) on FSI.
Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
And unfortunately, Renesas don't use FSI anymore.
So, I think keeping current FSI driver as-is is more safety for old boards.
armadillo800eva works well with below patch (as 2/3 patch)

--------------
diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
index 75de26c..36aaeb1 100644
--- a/arch/arm/mach-shmobile/board-armadillo800eva.c
+++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
@@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
        .platform       = "sh_fsi2",
        .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
        .cpu_dai = {
-               .fmt    = SND_SOC_DAIFMT_IB_NF,
                .name   = "fsia-dai",
        },
        .codec_dai = {
--------------

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Fri, Jan 30, 2015 at 05:18:56AM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars, Simon
> 
> > The DAI link format should be specified for the whole link rather than just
> > one component on the link. So move the format specification for the HDMI
> > audio link from the CPU component to the link itself.
> > 
> > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > in this case there is no functional difference between only specifying the
> > the format for the CPU side or for the whole link, but the later it will
> > allow us to remove support for just specifying the format for one component.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > ---
> 
> I tested these 3 patches on armadillo800eva
> 
> 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> 2/3 patch changed behavior for clock inversion on FSI driver.
> FSI + wm8978 on armadillo800eva worked without any issues,
> but, I don't know how much effect it has for other board.
> We used this inversion flags on each board for historical reasons (copy-paste),
> but, almost all these were not needed (except some picky board) on FSI.
> Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> And unfortunately, Renesas don't use FSI anymore.
> So, I think keeping current FSI driver as-is is more safety for old boards.
> armadillo800eva works well with below patch (as 2/3 patch)


Thanks, I am inclined to agree that it is best to leave things unchanged.

> 
> --------------
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> --------------
> 

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Fri, Jan 30, 2015 at 05:18:56AM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars, Simon
> 
> > The DAI link format should be specified for the whole link rather than just
> > one component on the link. So move the format specification for the HDMI
> > audio link from the CPU component to the link itself.
> > 
> > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > in this case there is no functional difference between only specifying the
> > the format for the CPU side or for the whole link, but the later it will
> > allow us to remove support for just specifying the format for one component.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > ---
> 
> I tested these 3 patches on armadillo800eva
> 
> 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> 2/3 patch changed behavior for clock inversion on FSI driver.
> FSI + wm8978 on armadillo800eva worked without any issues,
> but, I don't know how much effect it has for other board.
> We used this inversion flags on each board for historical reasons (copy-paste),
> but, almost all these were not needed (except some picky board) on FSI.
> Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> And unfortunately, Renesas don't use FSI anymore.
> So, I think keeping current FSI driver as-is is more safety for old boards.
> armadillo800eva works well with below patch (as 2/3 patch)


Thanks, I am inclined to agree that it is best to leave things unchanged.

> 
> --------------
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> --------------
> 

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Simon Horman]

On Fri, Jan 30, 2015 at 05:18:56AM +0000, Kuninori Morimoto wrote:
> 
> Hi Lars, Simon
> 
> > The DAI link format should be specified for the whole link rather than just
> > one component on the link. So move the format specification for the HDMI
> > audio link from the CPU component to the link itself.
> > 
> > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > in this case there is no functional difference between only specifying the
> > the format for the CPU side or for the whole link, but the later it will
> > allow us to remove support for just specifying the format for one component.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > ---
> 
> I tested these 3 patches on armadillo800eva
> 
> 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> 
> Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> 2/3 patch changed behavior for clock inversion on FSI driver.
> FSI + wm8978 on armadillo800eva worked without any issues,
> but, I don't know how much effect it has for other board.
> We used this inversion flags on each board for historical reasons (copy-paste),
> but, almost all these were not needed (except some picky board) on FSI.
> Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> And unfortunately, Renesas don't use FSI anymore.
> So, I think keeping current FSI driver as-is is more safety for old boards.
> armadillo800eva works well with below patch (as 2/3 patch)


Thanks, I am inclined to agree that it is best to leave things unchanged.

> 
> --------------
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> --------------
> 

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars, Mark

> > > The DAI link format should be specified for the whole link rather than just
> > > one component on the link. So move the format specification for the HDMI
> > > audio link from the CPU component to the link itself.
> > > 
> > > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > > in this case there is no functional difference between only specifying the
> > > the format for the CPU side or for the whole link, but the later it will
> > > allow us to remove support for just specifying the format for one component.
> > > 
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > ---
> > 
> > I tested these 3 patches on armadillo800eva
> > 
> > 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> > 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> > 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> > 
> > Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > 2/3 patch changed behavior for clock inversion on FSI driver.
> > FSI + wm8978 on armadillo800eva worked without any issues,
> > but, I don't know how much effect it has for other board.
> > We used this inversion flags on each board for historical reasons (copy-paste),
> > but, almost all these were not needed (except some picky board) on FSI.
> > Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> > And unfortunately, Renesas don't use FSI anymore.
> > So, I think keeping current FSI driver as-is is more safety for old boards.
> > armadillo800eva works well with below patch (as 2/3 patch)
> 
> 
> Thanks, I am inclined to agree that it is best to leave things unchanged.
> 
> > 
> > --------------
> > diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > index 75de26c..36aaeb1 100644
> > --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> > +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
> >         .platform       = "sh_fsi2",
> >         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
> >         .cpu_dai = {
> > -               .fmt    = SND_SOC_DAIFMT_IB_NF,
> >                 .name   = "fsia-dai",
> >         },
> >         .codec_dai = {
> > --------------

As I mentioned above (and, Simon agreed), I think we can
agree if [2/3] patch was exchanged

Best regards
---
Kuninori Morimoto

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Lars, Mark

> > > The DAI link format should be specified for the whole link rather than just
> > > one component on the link. So move the format specification for the HDMI
> > > audio link from the CPU component to the link itself.
> > > 
> > > Since the sh-mobile-hdmi DAI driver doesn't implement the set_fmt() callback
> > > in this case there is no functional difference between only specifying the
> > > the format for the CPU side or for the whole link, but the later it will
> > > allow us to remove support for just specifying the format for one component.
> > > 
> > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> > > ---
> > 
> > I tested these 3 patches on armadillo800eva
> > 
> > 	[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format
> > 	[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion
> > 	[PATCH v2 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
> > 
> > Tested-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > 2/3 patch changed behavior for clock inversion on FSI driver.
> > FSI + wm8978 on armadillo800eva worked without any issues,
> > but, I don't know how much effect it has for other board.
> > We used this inversion flags on each board for historical reasons (copy-paste),
> > but, almost all these were not needed (except some picky board) on FSI.
> > Maybe Lars's 2/3 patch is correct, but, it is difficult to check/confirm for all boards.
> > And unfortunately, Renesas don't use FSI anymore.
> > So, I think keeping current FSI driver as-is is more safety for old boards.
> > armadillo800eva works well with below patch (as 2/3 patch)
> 
> 
> Thanks, I am inclined to agree that it is best to leave things unchanged.
> 
> > 
> > --------------
> > diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > index 75de26c..36aaeb1 100644
> > --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> > +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> > @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
> >         .platform       = "sh_fsi2",
> >         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
> >         .cpu_dai = {
> > -               .fmt    = SND_SOC_DAIFMT_IB_NF,
> >                 .name   = "fsia-dai",
> >         },
> >         .codec_dai = {
> > --------------

As I mentioned above (and, Simon agreed), I think we can
agree if [2/3] patch was exchanged

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Mark Brown]

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

On Fri, Feb 27, 2015 at 12:23:26AM +0000, Kuninori Morimoto wrote:

> As I mentioned above (and, Simon agreed), I think we can
> agree if [2/3] patch was exchanged

Do you mean that patch 2/3 should be replaced with soemthing else - if
so what should it be replaced by?  I've not seen an updated version of
this series...

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Mark Brown]

On Fri, Feb 27, 2015 at 12:23:26AM +0000, Kuninori Morimoto wrote:

> As I mentioned above (and, Simon agreed), I think we can
> agree if [2/3] patch was exchanged

Do you mean that patch 2/3 should be replaced with soemthing else - if
so what should it be replaced by?  I've not seen an updated version of
this series...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150322/d61f25d2/attachment.sig>

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Mark

> > As I mentioned above (and, Simon agreed), I think we can
> > agree if [2/3] patch was exchanged
> 
> Do you mean that patch 2/3 should be replaced with soemthing else - if
> so what should it be replaced by?  I've not seen an updated version of
> this series...
> [2 Digital signature <application/pgp-signature (7bit)>]
> No public key for 24D68B725D5487D0 created at 2015-03-23T03:52:37+0900 using RSA

I guess I added my recommend patch on previous mail.
If it was not understandable, I can send full-patch-set instead of Lars.

Best regards
---
Kuninori Morimoto

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Mark

> > As I mentioned above (and, Simon agreed), I think we can
> > agree if [2/3] patch was exchanged
> 
> Do you mean that patch 2/3 should be replaced with soemthing else - if
> so what should it be replaced by?  I've not seen an updated version of
> this series...
> [2 Digital signature <application/pgp-signature (7bit)>]
> No public key for 24D68B725D5487D0 created at 2015-03-23T03:52:37+0900 using RSA

I guess I added my recommend patch on previous mail.
If it was not understandable, I can send full-patch-set instead of Lars.

Best regards
---
Kuninori Morimoto

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Mark Brown]

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

On Mon, Mar 23, 2015 at 07:00:11AM +0000, Kuninori Morimoto wrote:

> I guess I added my recommend patch on previous mail.
> If it was not understandable, I can send full-patch-set instead of Lars.

Ah, in a reply in the middle of the thread?  That does get hard to
follow sometimes, if there's general agreement on what should go in if
someone could do a complete resend of the code everyone agrees on that
would be helpful.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Mark Brown]

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

On Mon, Mar 23, 2015 at 07:00:11AM +0000, Kuninori Morimoto wrote:

> I guess I added my recommend patch on previous mail.
> If it was not understandable, I can send full-patch-set instead of Lars.

Ah, in a reply in the middle of the thread?  That does get hard to
follow sometimes, if there's general agreement on what should go in if
someone could do a complete resend of the code everyone agrees on that
would be helpful.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Mark Brown]

On Mon, Mar 23, 2015 at 07:00:11AM +0000, Kuninori Morimoto wrote:

> I guess I added my recommend patch on previous mail.
> If it was not understandable, I can send full-patch-set instead of Lars.

Ah, in a reply in the middle of the thread?  That does get hard to
follow sometimes, if there's general agreement on what should go in if
someone could do a complete resend of the code everyone agrees on that
would be helpful.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150323/e5bfe815/attachment.sig>

Re: [PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Mark

> > I guess I added my recommend patch on previous mail.
> > If it was not understandable, I can send full-patch-set instead of Lars.
> 
> Ah, in a reply in the middle of the thread?  That does get hard to
> follow sometimes, if there's general agreement on what should go in if
> someone could do a complete resend of the code everyone agrees on that
> would be helpful.

OK, I will send full-patch-set for it.

[PATCH v2 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format

[Kuninori Morimoto]

Hi Mark

> > I guess I added my recommend patch on previous mail.
> > If it was not understandable, I can send full-patch-set instead of Lars.
> 
> Ah, in a reply in the middle of the thread?  That does get hard to
> follow sometimes, if there's general agreement on what should go in if
> someone could do a complete resend of the code everyone agrees on that
> would be helpful.

OK, I will send full-patch-set for it.

Re: [PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion

[Geert Uytterhoeven]

On Wed, Jan 21, 2015 at 2:38 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> According to the sh7724 hardware user manual (Rev.2.00 Jan 2013) page 1851
> to 1856 the FSI bit-clock is inverted to the bit-clock as specified by the
> I2S standard. This means the bit clock inversion bit should be set for a
> normal I2S clock and should not be set for an inverted I2S clock.
>
> Similarly when operating in left-justfied mode both the frame-clock and the
> bit-clock need to be inverted to be standards compliant.
>
> This means also that the exta clock inversion setting in the armadillo800eva
> machine driver for CPU component should now be removed.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> I don't have the hardware but I'd like to get rid of the extra
> SND_SOC_DAIFMT_IB_NF in simple-card platform data, so we can remove the fmt
> field. Kuninori can you check if this works?
>
> Changes since v1:
>         * Fix whitespace
> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 1 -

The board files are gone by now.

>  sound/soc/sh/fsi.c                             | 7 +++++++

Is this part still applicable?

>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
> index b87b22e..6783d3f 100644
> --- a/sound/soc/sh/fsi.c
> +++ b/sound/soc/sh/fsi.c
> @@ -1594,6 +1594,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
>
>  static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
>  {
> +       /*
> +        * FSI bit clock is inverted to the I2S specification, so we invert it
> +        * when a non-inverted I2S clock was requested and vice versa.
> +        */
> +       fsi->bit_clk_inv = !fsi->bit_clk_inv;
> +
>         switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>         case SND_SOC_DAIFMT_I2S:
>                 fsi->fmt = CR_I2S;
> @@ -1602,6 +1608,7 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
>         case SND_SOC_DAIFMT_LEFT_J:
>                 fsi->fmt = CR_PCM;
>                 fsi->chan_num = 2;
> +               fsi->lr_clk_inv = !fsi->lr_clk_inv;
>                 break;
>         default:
>                 return -EINVAL;
> --
> 1.8.0

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v2 2/3] ASoC: sh: fsi: Fix clock inversion

[Geert Uytterhoeven]

On Wed, Jan 21, 2015 at 2:38 PM, Lars-Peter Clausen <lars@metafoo.de> wrote:
> According to the sh7724 hardware user manual (Rev.2.00 Jan 2013) page 1851
> to 1856 the FSI bit-clock is inverted to the bit-clock as specified by the
> I2S standard. This means the bit clock inversion bit should be set for a
> normal I2S clock and should not be set for an inverted I2S clock.
>
> Similarly when operating in left-justfied mode both the frame-clock and the
> bit-clock need to be inverted to be standards compliant.
>
> This means also that the exta clock inversion setting in the armadillo800eva
> machine driver for CPU component should now be removed.
>
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> ---
> I don't have the hardware but I'd like to get rid of the extra
> SND_SOC_DAIFMT_IB_NF in simple-card platform data, so we can remove the fmt
> field. Kuninori can you check if this works?
>
> Changes since v1:
>         * Fix whitespace
> ---
>  arch/arm/mach-shmobile/board-armadillo800eva.c | 1 -

The board files are gone by now.

>  sound/soc/sh/fsi.c                             | 7 +++++++

Is this part still applicable?

>  2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-shmobile/board-armadillo800eva.c b/arch/arm/mach-shmobile/board-armadillo800eva.c
> index 75de26c..36aaeb1 100644
> --- a/arch/arm/mach-shmobile/board-armadillo800eva.c
> +++ b/arch/arm/mach-shmobile/board-armadillo800eva.c
> @@ -1015,7 +1015,6 @@ static struct asoc_simple_card_info fsi_wm8978_info = {
>         .platform       = "sh_fsi2",
>         .daifmt         = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_CBM_CFM,
>         .cpu_dai = {
> -               .fmt    = SND_SOC_DAIFMT_IB_NF,
>                 .name   = "fsia-dai",
>         },
>         .codec_dai = {
> diff --git a/sound/soc/sh/fsi.c b/sound/soc/sh/fsi.c
> index b87b22e..6783d3f 100644
> --- a/sound/soc/sh/fsi.c
> +++ b/sound/soc/sh/fsi.c
> @@ -1594,6 +1594,12 @@ static int fsi_dai_trigger(struct snd_pcm_substream *substream, int cmd,
>
>  static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
>  {
> +       /*
> +        * FSI bit clock is inverted to the I2S specification, so we invert it
> +        * when a non-inverted I2S clock was requested and vice versa.
> +        */
> +       fsi->bit_clk_inv = !fsi->bit_clk_inv;
> +
>         switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
>         case SND_SOC_DAIFMT_I2S:
>                 fsi->fmt = CR_I2S;
> @@ -1602,6 +1608,7 @@ static int fsi_set_fmt_dai(struct fsi_priv *fsi, unsigned int fmt)
>         case SND_SOC_DAIFMT_LEFT_J:
>                 fsi->fmt = CR_PCM;
>                 fsi->chan_num = 2;
> +               fsi->lr_clk_inv = !fsi->lr_clk_inv;
>                 break;
>         default:
>                 return -EINVAL;
> --
> 1.8.0

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds