[PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

Add ability to remove rate constraints from generic ASoC AC'97 CODEC
driver via passed platform data, make it selectable in config.

This way this driver can be used for platforms which don't need
specialized AC'97 CODEC drivers while at the same avoiding
code duplication from implementing equivalent functionality in
a controller driver.

There is no change in behavior when no platform data is passed.

Signed-off-by: Maciej Szmigiero <mail@maciej.szmigiero.name>

diff --git a/include/sound/soc-ac97.h b/include/sound/soc-ac97.h
new file mode 100644
index 0000000..ceb4e2f
--- /dev/null
+++ b/include/sound/soc-ac97.h
@@ -0,0 +1,17 @@
+/*
+ * Platform data for generic ASoC AC97 CODEC driver.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#ifndef __LINUX_SND__SOC_AC97_H
+#define __LINUX_SND__SOC_AC97_H
+
+struct snd_soc_ac97_codec_platform_data {
+	bool playback_rate_constrained;
+	bool capture_rate_constrained;
+};
+
+#endif /* __LINUX_SND__SOC_AC97_H */
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0bddd92..92d6d39 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -16,7 +16,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_88PM860X if MFD_88PM860X
 	select SND_SOC_L3
 	select SND_SOC_AB8500_CODEC if ABX500_CORE
-	select SND_SOC_AC97_CODEC if SND_SOC_AC97_BUS
+	select SND_SOC_AC97_CODEC
 	select SND_SOC_AD1836 if SPI_MASTER
 	select SND_SOC_AD193X_SPI if SPI_MASTER
 	select SND_SOC_AD193X_I2C if I2C
@@ -210,8 +210,9 @@ config SND_SOC_AB8500_CODEC
 	tristate
 
 config SND_SOC_AC97_CODEC
-	tristate
+	tristate "Build generic ASoC AC97 CODEC driver"
 	select SND_AC97_CODEC
+	select SND_SOC_AC97_BUS
 
 config SND_SOC_AD1836
 	tristate
diff --git a/sound/soc/codecs/ac97.c b/sound/soc/codecs/ac97.c
index d0ac723..c3fb845 100644
--- a/sound/soc/codecs/ac97.c
+++ b/sound/soc/codecs/ac97.c
@@ -20,9 +20,16 @@
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/ac97_codec.h>
+#include <sound/soc-ac97.h>
 #include <sound/initval.h>
 #include <sound/soc.h>
 
+struct ac97_private {
+	struct snd_ac97 *ac97;
+	bool playback_rate_constrained;
+	bool capture_rate_constrained;
+};
+
 static const struct snd_soc_dapm_widget ac97_widgets[] = {
 	SND_SOC_DAPM_INPUT("RX"),
 	SND_SOC_DAPM_OUTPUT("TX"),
@@ -33,22 +40,53 @@ static const struct snd_soc_dapm_route ac97_routes[] = {
 	{ "TX", NULL, "AC97 Playback" },
 };
 
+static const unsigned int default_rates[] = {
+	8000, 11025, 22050, 44100, 48000
+};
+
+static const struct snd_pcm_hw_constraint_list default_rate_constraints = {
+	.count	= ARRAY_SIZE(default_rates),
+	.list	= default_rates,
+};
+
+static int ac97_startup(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct ac97_private *ac97 = snd_soc_codec_get_drvdata(codec);
+	bool constrained;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		constrained = ac97->playback_rate_constrained;
+	else
+		constrained = ac97->capture_rate_constrained;
+
+	if (!constrained)
+		return 0;
+
+	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+		snd_pcm_hw_constraint_list(substream->runtime, 0,
+			SNDRV_PCM_HW_PARAM_RATE, &default_rate_constraints);
+	else
+		snd_pcm_hw_constraint_list(substream->runtime, 0,
+			SNDRV_PCM_HW_PARAM_RATE, &default_rate_constraints);
+
+	return 0;
+}
+
 static int ac97_prepare(struct snd_pcm_substream *substream,
 			struct snd_soc_dai *dai)
 {
 	struct snd_soc_codec *codec = dai->codec;
-	struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec);
+	struct ac97_private *ac97 = snd_soc_codec_get_drvdata(codec);
 
 	int reg = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
 		  AC97_PCM_FRONT_DAC_RATE : AC97_PCM_LR_ADC_RATE;
-	return snd_ac97_set_rate(ac97, reg, substream->runtime->rate);
+	return snd_ac97_set_rate(ac97->ac97, reg, substream->runtime->rate);
 }
 
-#define STD_AC97_RATES (SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 |\
-		SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_44100 |\
-		SNDRV_PCM_RATE_48000)
-
 static const struct snd_soc_dai_ops ac97_dai_ops = {
+	.startup	= ac97_startup,
 	.prepare	= ac97_prepare,
 };
 
@@ -58,20 +96,21 @@ static struct snd_soc_dai_driver ac97_dai = {
 		.stream_name = "AC97 Playback",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = STD_AC97_RATES,
+		.rates = SNDRV_PCM_RATE_KNOT,
 		.formats = SND_SOC_STD_AC97_FMTS,},
 	.capture = {
 		.stream_name = "AC97 Capture",
 		.channels_min = 1,
 		.channels_max = 2,
-		.rates = STD_AC97_RATES,
+		.rates = SNDRV_PCM_RATE_KNOT,
 		.formats = SND_SOC_STD_AC97_FMTS,},
 	.ops = &ac97_dai_ops,
 };
 
 static int ac97_soc_probe(struct snd_soc_codec *codec)
 {
-	struct snd_ac97 *ac97;
+	struct ac97_private *ac97 = snd_soc_codec_get_drvdata(codec);
+
 	struct snd_ac97_bus *ac97_bus;
 	struct snd_ac97_template ac97_template;
 	int ret;
@@ -83,31 +122,28 @@ static int ac97_soc_probe(struct snd_soc_codec *codec)
 		return ret;
 
 	memset(&ac97_template, 0, sizeof(struct snd_ac97_template));
-	ret = snd_ac97_mixer(ac97_bus, &ac97_template, &ac97);
+	ret = snd_ac97_mixer(ac97_bus, &ac97_template, &ac97->ac97);
 	if (ret < 0)
 		return ret;
 
-	snd_soc_codec_set_drvdata(codec, ac97);
-
 	return 0;
 }
 
 #ifdef CONFIG_PM
 static int ac97_soc_suspend(struct snd_soc_codec *codec)
 {
-	struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec);
+	struct ac97_private *ac97 = snd_soc_codec_get_drvdata(codec);
 
-	snd_ac97_suspend(ac97);
+	snd_ac97_suspend(ac97->ac97);
 
 	return 0;
 }
 
 static int ac97_soc_resume(struct snd_soc_codec *codec)
 {
+	struct ac97_private *ac97 = snd_soc_codec_get_drvdata(codec);
 
-	struct snd_ac97 *ac97 = snd_soc_codec_get_drvdata(codec);
-
-	snd_ac97_resume(ac97);
+	snd_ac97_resume(ac97->ac97);
 
 	return 0;
 }
@@ -129,6 +165,27 @@ static struct snd_soc_codec_driver soc_codec_dev_ac97 = {
 
 static int ac97_probe(struct platform_device *pdev)
 {
+	struct ac97_private *ac97 =
+		devm_kzalloc(&pdev->dev, sizeof(struct ac97_private),
+				GFP_KERNEL);
+	struct snd_soc_ac97_codec_platform_data *pdata =
+		pdev->dev.platform_data;
+
+	if (!ac97)
+		return -ENOMEM;
+
+	if (pdata) {
+		ac97->playback_rate_constrained =
+			pdata->playback_rate_constrained;
+		ac97->capture_rate_constrained =
+			pdata->capture_rate_constrained;
+	} else {
+		ac97->playback_rate_constrained = 1;
+		ac97->capture_rate_constrained = 1;
+	}
+
+	platform_set_drvdata(pdev, ac97);
+
 	return snd_soc_register_codec(&pdev->dev,
 			&soc_codec_dev_ac97, &ac97_dai, 1);
 }

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Mark Brown]

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

On Wed, Mar 11, 2015 at 12:28:19AM +0100, Maciej S. Szmigiero wrote:
> Add ability to remove rate constraints from generic ASoC AC'97 CODEC
> driver via passed platform data, make it selectable in config.

Please use subject lines matching the style for the subsystem.  This is
helpful for identifying relevant patches and not getting your messages
deleted unread...

> This way this driver can be used for platforms which don't need
> specialized AC'97 CODEC drivers while at the same avoiding
> code duplication from implementing equivalent functionality in
> a controller driver.

I'm sorry but this just doesn't explain what this patch is intended to
accomplish.  If we can talk to the AC'97 CODEC at all we can already
identify whatever constraints it has by looking at the ID registers so
it's not clear when or why a platform would need to use this.  It feels
like there is some underlying problem that you're trying to address.

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

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

Thanks for looking into the patch.

W dniu 22.03.2015 19:27, Mark Brown pisze:
> On Wed, Mar 11, 2015 at 12:28:19AM +0100, Maciej S. Szmigiero wrote:
>> Add ability to remove rate constraints from generic ASoC AC'97 CODEC
>> driver via passed platform data, make it selectable in config.
> 
> Please use subject lines matching the style for the subsystem.  This is
> helpful for identifying relevant patches and not getting your messages
> deleted unread...

I assume "[PATCH] ASoC: driver: subject" format would be right?

>> This way this driver can be used for platforms which don't need
>> specialized AC'97 CODEC drivers while at the same avoiding
>> code duplication from implementing equivalent functionality in
>> a controller driver.
> 
> I'm sorry but this just doesn't explain what this patch is intended to
> accomplish.  If we can talk to the AC'97 CODEC at all we can already
> identify whatever constraints it has by looking at the ID registers so
> it's not clear when or why a platform would need to use this.  It feels
> like there is some underlying problem that you're trying to address.

This patch itself is supposed to allow using this CODEC driver with
CODECs that support other rates that these that were already hard-coded
in the driver (8000, 11025, 22050, 44100, 48000).

In general sense what I want to accomplish is to add sound support for
UDOO board to the mainline kernel.
It uses i.MX6 SSI core as controller with VT1613 AC'97 codec.

While it would be possible to simply add ac97 bus enumeration to
either fsl_ssi driver or fsl-asoc-card sound card driver it looks to me
that even in this case a platform device for codec would need to be
registered anyway.

This is because as far I see ASoC CODEC DAIs in links are identified
either by OF node or name of their platform device.
I've already tried to accomplish all of this via adding OF node of
AC'97 codec but such patch was rejected.

If the ac97 bus enumeration is added to fsl_ssi driver then there is
also a problem of communicating a name of ac97 platform device
to sound card driver so it can then reference it in DAI links
that it builds.
If such name would be hardcoded then there would be a possibility to
use only one such CODEC in the system
(this SoC theoretically allows up to three).

And, naturally, this would result in a small code duplication with
regard to this generic driver.

Best regards,
Maciej Szmigiero

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

Thanks for looking into the patch.

W dniu 22.03.2015 19:27, Mark Brown pisze:
> On Wed, Mar 11, 2015 at 12:28:19AM +0100, Maciej S. Szmigiero wrote:
>> Add ability to remove rate constraints from generic ASoC AC'97 CODEC
>> driver via passed platform data, make it selectable in config.
> 
> Please use subject lines matching the style for the subsystem.  This is
> helpful for identifying relevant patches and not getting your messages
> deleted unread...

I assume "[PATCH] ASoC: driver: subject" format would be right?

>> This way this driver can be used for platforms which don't need
>> specialized AC'97 CODEC drivers while at the same avoiding
>> code duplication from implementing equivalent functionality in
>> a controller driver.
> 
> I'm sorry but this just doesn't explain what this patch is intended to
> accomplish.  If we can talk to the AC'97 CODEC at all we can already
> identify whatever constraints it has by looking at the ID registers so
> it's not clear when or why a platform would need to use this.  It feels
> like there is some underlying problem that you're trying to address.

This patch itself is supposed to allow using this CODEC driver with
CODECs that support other rates that these that were already hard-coded
in the driver (8000, 11025, 22050, 44100, 48000).

In general sense what I want to accomplish is to add sound support for
UDOO board to the mainline kernel.
It uses i.MX6 SSI core as controller with VT1613 AC'97 codec.

While it would be possible to simply add ac97 bus enumeration to
either fsl_ssi driver or fsl-asoc-card sound card driver it looks to me
that even in this case a platform device for codec would need to be
registered anyway.

This is because as far I see ASoC CODEC DAIs in links are identified
either by OF node or name of their platform device.
I've already tried to accomplish all of this via adding OF node of
AC'97 codec but such patch was rejected.

If the ac97 bus enumeration is added to fsl_ssi driver then there is
also a problem of communicating a name of ac97 platform device
to sound card driver so it can then reference it in DAI links
that it builds.
If such name would be hardcoded then there would be a possibility to
use only one such CODEC in the system
(this SoC theoretically allows up to three).

And, naturally, this would result in a small code duplication with
regard to this generic driver.

Best regards,
Maciej Szmigiero

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Mark Brown]

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

On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:
> W dniu 22.03.2015 19:27, Mark Brown pisze:

> > Please use subject lines matching the style for the subsystem.  This is
> > helpful for identifying relevant patches and not getting your messages
> > deleted unread...

> I assume "[PATCH] ASoC: driver: subject" format would be right?

Yes.

> > I'm sorry but this just doesn't explain what this patch is intended to
> > accomplish.  If we can talk to the AC'97 CODEC at all we can already
> > identify whatever constraints it has by looking at the ID registers so
> > it's not clear when or why a platform would need to use this.  It feels
> > like there is some underlying problem that you're trying to address.

> This patch itself is supposed to allow using this CODEC driver with
> CODECs that support other rates that these that were already hard-coded
> in the driver (8000, 11025, 22050, 44100, 48000).

You're talking about Linux internal implementation details here but the
change you are proposing a change to the device tree?  Like I think I
said in reply to a previous iteration of this patch we can clearly
enumerate the required information from the hardware already.

> While it would be possible to simply add ac97 bus enumeration to
> either fsl_ssi driver or fsl-asoc-card sound card driver it looks to me
> that even in this case a platform device for codec would need to be
> registered anyway.

That's the way the ASoC AC'97 support currently works, yes.

> If the ac97 bus enumeration is added to fsl_ssi driver then there is
> also a problem of communicating a name of ac97 platform device
> to sound card driver so it can then reference it in DAI links
> that it builds.

...

> And, naturally, this would result in a small code duplication with
> regard to this generic driver.

This sounds like something that can easily be put in some generic code
and either used as a helper library by the drivers or just done directly
from the framework when it knows it's dealing with an AC'97 bus.

Alternatively if you're just trying to open up the constraints why not
just open them up by default and make sure that the AC'97 generic code
is constraining things appropriately if it's not doing so already?

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

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

W dniu 30.03.2015 06:19, Mark Brown pisze:
> On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:
>> W dniu 22.03.2015 19:27, Mark Brown pisze:
> 
>>> Please use subject lines matching the style for the subsystem.  This is
>>> helpful for identifying relevant patches and not getting your messages
>>> deleted unread...
> 
>> I assume "[PATCH] ASoC: driver: subject" format would be right?
> 
> Yes.

Thanks for information.

>>> I'm sorry but this just doesn't explain what this patch is intended to
>>> accomplish.  If we can talk to the AC'97 CODEC at all we can already
>>> identify whatever constraints it has by looking at the ID registers so
>>> it's not clear when or why a platform would need to use this.  It feels
>>> like there is some underlying problem that you're trying to address.
> 
>> This patch itself is supposed to allow using this CODEC driver with
>> CODECs that support other rates that these that were already hard-coded
>> in the driver (8000, 11025, 22050, 44100, 48000).
> 
> You're talking about Linux internal implementation details here but the
> change you are proposing a change to the device tree?  Like I think I
> said in reply to a previous iteration of this patch we can clearly
> enumerate the required information from the hardware already.
> 
>> While it would be possible to simply add ac97 bus enumeration to
>> either fsl_ssi driver or fsl-asoc-card sound card driver it looks to me
>> that even in this case a platform device for codec would need to be
>> registered anyway.
> 
> That's the way the ASoC AC'97 support currently works, yes.
> 
>> If the ac97 bus enumeration is added to fsl_ssi driver then there is
>> also a problem of communicating a name of ac97 platform device
>> to sound card driver so it can then reference it in DAI links
>> that it builds.
> 
> ...
> 
>> And, naturally, this would result in a small code duplication with
>> regard to this generic driver.
> 
> This sounds like something that can easily be put in some generic code
> and either used as a helper library by the drivers or just done directly
> from the framework when it knows it's dealing with an AC'97 bus.
> 
> Alternatively if you're just trying to open up the constraints why not
> just open them up by default and make sure that the AC'97 generic code
> is constraining things appropriately if it's not doing so already?

By 'open them up by default' do you mean removing them altogether 
from ASOC AC'97 generic CODEC to let the AC'97 bus driver constrain
rates on its own or do you mean retaining ability to constraint in
ASOC AC'97 driver, just have it switched off by default?
(that is, what the current patch did, just change the default setting).

If the rate constraints are removed (or disabled) from the ASOC AC'97
generic CODEC driver then it can be used as-is in the (at least) fsl_ssi
driver by registering AC'97 platform device there using index taken from
DT property (cell-index) already defined as required in
Documentation/devicetree/bindings/sound/fsl,ssi.txt.

This way the AC'97 CODECs can be uniquely identified in case there is more
than one in the system.

Sadly, despite this property being marked as required in documentation no
current DT file seems to define it.

Now the question is what is more binding: the documentation which
defines this property as required or DT files which don't define it at all?

Best regards,
Maciej Szmigiero

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

W dniu 30.03.2015 06:19, Mark Brown pisze:
> On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:
>> W dniu 22.03.2015 19:27, Mark Brown pisze:
> 
>>> Please use subject lines matching the style for the subsystem.  This is
>>> helpful for identifying relevant patches and not getting your messages
>>> deleted unread...
> 
>> I assume "[PATCH] ASoC: driver: subject" format would be right?
> 
> Yes.

Thanks for information.

>>> I'm sorry but this just doesn't explain what this patch is intended to
>>> accomplish.  If we can talk to the AC'97 CODEC at all we can already
>>> identify whatever constraints it has by looking at the ID registers so
>>> it's not clear when or why a platform would need to use this.  It feels
>>> like there is some underlying problem that you're trying to address.
> 
>> This patch itself is supposed to allow using this CODEC driver with
>> CODECs that support other rates that these that were already hard-coded
>> in the driver (8000, 11025, 22050, 44100, 48000).
> 
> You're talking about Linux internal implementation details here but the
> change you are proposing a change to the device tree?  Like I think I
> said in reply to a previous iteration of this patch we can clearly
> enumerate the required information from the hardware already.
> 
>> While it would be possible to simply add ac97 bus enumeration to
>> either fsl_ssi driver or fsl-asoc-card sound card driver it looks to me
>> that even in this case a platform device for codec would need to be
>> registered anyway.
> 
> That's the way the ASoC AC'97 support currently works, yes.
> 
>> If the ac97 bus enumeration is added to fsl_ssi driver then there is
>> also a problem of communicating a name of ac97 platform device
>> to sound card driver so it can then reference it in DAI links
>> that it builds.
> 
> ...
> 
>> And, naturally, this would result in a small code duplication with
>> regard to this generic driver.
> 
> This sounds like something that can easily be put in some generic code
> and either used as a helper library by the drivers or just done directly
> from the framework when it knows it's dealing with an AC'97 bus.
> 
> Alternatively if you're just trying to open up the constraints why not
> just open them up by default and make sure that the AC'97 generic code
> is constraining things appropriately if it's not doing so already?

By 'open them up by default' do you mean removing them altogether 
from ASOC AC'97 generic CODEC to let the AC'97 bus driver constrain
rates on its own or do you mean retaining ability to constraint in
ASOC AC'97 driver, just have it switched off by default?
(that is, what the current patch did, just change the default setting).

If the rate constraints are removed (or disabled) from the ASOC AC'97
generic CODEC driver then it can be used as-is in the (at least) fsl_ssi
driver by registering AC'97 platform device there using index taken from
DT property (cell-index) already defined as required in
Documentation/devicetree/bindings/sound/fsl,ssi.txt.

This way the AC'97 CODECs can be uniquely identified in case there is more
than one in the system.

Sadly, despite this property being marked as required in documentation no
current DT file seems to define it.

Now the question is what is more binding: the documentation which
defines this property as required or DT files which don't define it at all?

Best regards,
Maciej Szmigiero

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Mark Brown]

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

On Sun, Apr 05, 2015 at 12:16:40AM +0200, Maciej S. Szmigiero wrote:
> W dniu 30.03.2015 06:19, Mark Brown pisze:
> > On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:

> > Alternatively if you're just trying to open up the constraints why not
> > just open them up by default and make sure that the AC'97 generic code
> > is constraining things appropriately if it's not doing so already?

> By 'open them up by default' do you mean removing them altogether 
> from ASOC AC'97 generic CODEC to let the AC'97 bus driver constrain
> rates on its own or do you mean retaining ability to constraint in
> ASOC AC'97 driver, just have it switched off by default?

I mean make the default setting be the maximum possible set of rates and
then allow the CODEC code to constrain based on what it finds on the
link.

Please also note that ASoC is spelt ASoC.

> (that is, what the current patch did, just change the default setting).

No, it added a device tree property for controlling things.

> If the rate constraints are removed (or disabled) from the ASOC AC'97
> generic CODEC driver then it can be used as-is in the (at least) fsl_ssi
> driver by registering AC'97 platform device there using index taken from
> DT property (cell-index) already defined as required in
> Documentation/devicetree/bindings/sound/fsl,ssi.txt.

> Now the question is what is more binding: the documentation which
> defines this property as required or DT files which don't define it at all?

The driver really ought to continue to support existing DTs, it can
complain about them and assume the default value if the property really
is required for some reason though.

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

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

W dniu 06.04.2015 18:41, Mark Brown pisze:
> On Sun, Apr 05, 2015 at 12:16:40AM +0200, Maciej S. Szmigiero wrote:
>> W dniu 30.03.2015 06:19, Mark Brown pisze:
>>> On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:
> 
>>> Alternatively if you're just trying to open up the constraints why not
>>> just open them up by default and make sure that the AC'97 generic code
>>> is constraining things appropriately if it's not doing so already?
> 
>> By 'open them up by default' do you mean removing them altogether 
>> from ASOC AC'97 generic CODEC to let the AC'97 bus driver constrain
>> rates on its own or do you mean retaining ability to constraint in
>> ASOC AC'97 driver, just have it switched off by default?
> 
> I mean make the default setting be the maximum possible set of rates and
> then allow the CODEC code to constrain based on what it finds on the
> link.
> 
> Please also note that ASoC is spelt ASoC.
> 
>> (that is, what the current patch did, just change the default setting).
> 
> No, it added a device tree property for controlling things.

I meant this patch (about which the Subject line is), which controlled rate
constraints based on passed platform data, not the first one which
added DT support to the generic ASoC AC'97 CODEC driver.

>> If the rate constraints are removed (or disabled) from the ASOC AC'97
>> generic CODEC driver then it can be used as-is in the (at least) fsl_ssi
>> driver by registering AC'97 platform device there using index taken from
>> DT property (cell-index) already defined as required in
>> Documentation/devicetree/bindings/sound/fsl,ssi.txt.
> 
>> Now the question is what is more binding: the documentation which
>> defines this property as required or DT files which don't define it at all?
> 
> The driver really ought to continue to support existing DTs, it can
> complain about them and assume the default value if the property really
> is required for some reason though.

Thanks for explanation.

In this case this property would be only de facto required in DT of board that
I am adding sound to (and in future boards that are also using this
controller - CODEC arrangement), so there won't be any compatibility problems
with existing board DT files.

Best regards,
Maciej Szmigiero

Re: [PATCH][ASoC]Add ability to remove rate constraints from generic ASoC AC'97 CODEC driver

[Maciej S. Szmigiero]

W dniu 06.04.2015 18:41, Mark Brown pisze:
> On Sun, Apr 05, 2015 at 12:16:40AM +0200, Maciej S. Szmigiero wrote:
>> W dniu 30.03.2015 06:19, Mark Brown pisze:
>>> On Sun, Mar 29, 2015 at 06:00:33PM +0200, Maciej S. Szmigiero wrote:
> 
>>> Alternatively if you're just trying to open up the constraints why not
>>> just open them up by default and make sure that the AC'97 generic code
>>> is constraining things appropriately if it's not doing so already?
> 
>> By 'open them up by default' do you mean removing them altogether 
>> from ASOC AC'97 generic CODEC to let the AC'97 bus driver constrain
>> rates on its own or do you mean retaining ability to constraint in
>> ASOC AC'97 driver, just have it switched off by default?
> 
> I mean make the default setting be the maximum possible set of rates and
> then allow the CODEC code to constrain based on what it finds on the
> link.
> 
> Please also note that ASoC is spelt ASoC.
> 
>> (that is, what the current patch did, just change the default setting).
> 
> No, it added a device tree property for controlling things.

I meant this patch (about which the Subject line is), which controlled rate
constraints based on passed platform data, not the first one which
added DT support to the generic ASoC AC'97 CODEC driver.

>> If the rate constraints are removed (or disabled) from the ASOC AC'97
>> generic CODEC driver then it can be used as-is in the (at least) fsl_ssi
>> driver by registering AC'97 platform device there using index taken from
>> DT property (cell-index) already defined as required in
>> Documentation/devicetree/bindings/sound/fsl,ssi.txt.
> 
>> Now the question is what is more binding: the documentation which
>> defines this property as required or DT files which don't define it at all?
> 
> The driver really ought to continue to support existing DTs, it can
> complain about them and assume the default value if the property really
> is required for some reason though.

Thanks for explanation.

In this case this property would be only de facto required in DT of board that
I am adding sound to (and in future boards that are also using this
controller - CODEC arrangement), so there won't be any compatibility problems
with existing board DT files.

Best regards,
Maciej Szmigiero