[PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Update the Freescale MPC8610 HPCD fabric driver to create a new platform device
for each SSI in the system, instead of registering multiple PCMs under one card.
This is necessary for alsamixer to control the individual codecs.

Signed-off-by: Timur Tabi <timur@freescale.com>
---
 sound/soc/fsl/mpc8610_hpcd.c |  236 +++++++++++++++++++++---------------------
 1 files changed, 117 insertions(+), 119 deletions(-)

diff --git a/sound/soc/fsl/mpc8610_hpcd.c b/sound/soc/fsl/mpc8610_hpcd.c
index 8813291..2cf7d95 100644
--- a/sound/soc/fsl/mpc8610_hpcd.c
+++ b/sound/soc/fsl/mpc8610_hpcd.c
@@ -40,13 +40,10 @@
 struct mpc8610_hpcd_data {
 	struct snd_soc_card soc_card;
 	struct ccsr_guts __iomem *guts;
-	unsigned int num_configs;
-	struct snd_soc_pcm_config configs[MAX_SSI];
-	struct mpc8610_hpcd_names {
-		char pcm_name[32];
-		char ssi_name[32];
-		char codec_name[32];
-	} names[MAX_SSI];
+	struct snd_soc_pcm_config pcm_config;
+	char pcm_name[32];
+	char ssi_name[32];
+	char codec_name[32];
 };
 
 /**
@@ -91,29 +88,25 @@ static int mpc8610_hpcd_audio_init(struct snd_soc_card *soc_card)
 {
 	struct snd_soc_codec *codec;
 	struct mpc8610_hpcd_data *soc_card_data = soc_card->private_data;
-	unsigned int i;
 	int ret = 0;
 
-	for (i = 0; i < soc_card_data->num_configs; i++) {
-		codec = snd_soc_card_get_codec(soc_card,
-			soc_card_data->names[i].codec_name,
-			soc_card_data->configs[i].codec_num);
+	codec = snd_soc_card_get_codec(soc_card, soc_card_data->codec_name,
+		soc_card_data->pcm_config.codec_num);
 
-		if (!codec) {
-			dev_err(soc_card->dev, "could not find codec\n");
-			return -ENODEV;
-		}
+	if (!codec) {
+		dev_err(soc_card->dev, "could not find codec\n");
+		return -ENODEV;
+	}
 
-		/* The codec driver should have called
-		 * snd_soc_card_config_codec() by now.
-		 */
-		ret = snd_soc_card_init_codec(codec, soc_card);
-		if (ret < 0) {
-			dev_err(soc_card->dev,
-				"could not initialize codec %s-%u\n",
-				codec->name, codec->num);
-			continue;
-		}
+	/* The codec driver should have called
+	 * snd_soc_card_config_codec() by now.
+	 */
+	ret = snd_soc_card_init_codec(codec, soc_card);
+	if (ret < 0) {
+		dev_err(soc_card->dev,
+			"could not initialize codec %s-%u\n",
+			codec->name, codec->num);
+		return -ENODEV;
 	}
 
 	return 0;
@@ -316,10 +309,16 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	struct snd_soc_card *soc_card = NULL;
 	struct mpc8610_hpcd_data *soc_card_data;
 	struct device_node *guts_np = NULL;
-	struct device_node *ssi_np;
-	unsigned int ssi = 0;
+	struct device_node *ssi_np =
+		(struct device_node *)platform_get_resource(pdev, 0, 0)->start;
 	int ret = -ENODEV;
 
+	struct device_node *codec_np;
+	const char *compat;
+	const char *p;
+	unsigned int bus;
+	unsigned int address;
+
 	soc_card_data = kzalloc(sizeof(struct mpc8610_hpcd_data), GFP_KERNEL);
 	if (!soc_card_data) {
 		dev_err(&pdev->dev, "could not allocate card structure\n");
@@ -353,100 +352,68 @@ static int mpc8610_hpcd_probe(struct platform_device *pdev)
 	soc_card->private_data = soc_card_data;
 	soc_card->dev = &pdev->dev;
 
-	/* Scan the device tree for SSI nodes.  Each one we find that has a
-	 * codec is registered.  Remember, we only support MAX_SSI of these.
-	 */
-	for_each_compatible_node(ssi_np, NULL, "fsl,mpc8610-ssi") {
-		struct snd_soc_pcm_config *pcm_config;
-		struct mpc8610_hpcd_names *name;
-		struct device_node *codec_np;
-		const char *compat;
-		const char *p;
-		unsigned int bus;
-		unsigned int address;
-		int i;
-
-		name = &soc_card_data->names[ssi];
-		pcm_config = &soc_card_data->configs[ssi];
-
-		pcm_config->name = name->pcm_name;
-		pcm_config->codec = name->codec_name;
-		pcm_config->codec_dai = name->codec_name;
-		pcm_config->platform = "fsl-elo";
-		pcm_config->cpu_dai = name->ssi_name;
-		pcm_config->ops = &mpc8610_hpcd_ops;
-		pcm_config->playback = 1;
-		pcm_config->capture = 1;
-
-		sprintf(name->pcm_name, "MPC8610HPCD.%u", ssi);
-
-		/* Get the SSI name */
-		i = of_get_integer(ssi_np, "cell-index");
-		if (i < 0) {
-			dev_err(soc_card->dev, "no cell-index for %s\n",
-				ssi_np->full_name);
-			continue;
-		}
-		sprintf(name->ssi_name, "ssi%u", i);
-
-		/* Get the codec name and ID */
-		codec_np = find_codec(ssi_np);
-		if (!codec_np) {
-			dev_err(soc_card->dev, "missing codec node for %s\n",
-				ssi_np->full_name);
-			continue;
-		}
-
-		/* We assume that the first (or only) string in the compatible
-		 * field is the one that counts.
-		 */
-		compat = of_get_property(codec_np, "compatible", NULL);
-		if (!compat) {
-			dev_err(soc_card->dev,
-				"missing compatible property for %s\n",
-				ssi_np->full_name);
-			continue;
-		}
-
-		/* We only care about the part after the comma */
-		p = strchr(compat, ',');
-		strcpy(name->codec_name, p ? p + 1 : compat);
-
-		/* Now determine the I2C bus and address of the codec */
-		bus = of_get_integer(of_get_parent(codec_np), "cell-index");
-		if (bus < 0) {
-			dev_err(soc_card->dev,
-				"cannot determine I2C bus number for %s\n",
-				codec_np->full_name);
-			continue;
-		}
+	soc_card_data->pcm_config.name = soc_card_data->pcm_name;
+	soc_card_data->pcm_config.codec = soc_card_data->codec_name;
+	soc_card_data->pcm_config.codec_dai = soc_card_data->codec_name;
+	soc_card_data->pcm_config.platform = "fsl-elo";
+	soc_card_data->pcm_config.cpu_dai = soc_card_data->ssi_name;
+	soc_card_data->pcm_config.ops = &mpc8610_hpcd_ops;
+	soc_card_data->pcm_config.playback = 1;
+	soc_card_data->pcm_config.capture = 1;
+
+	sprintf(soc_card_data->pcm_name, "MPC8610HPCD.%u", pdev->id);
+
+	/* Get the SSI name */
+	sprintf(soc_card_data->ssi_name, "ssi%u", pdev->id);
+
+	/* Get the codec name and ID */
+	codec_np = find_codec(ssi_np);
+	if (!codec_np) {
+		dev_err(soc_card->dev, "missing codec node for %s\n",
+			ssi_np->full_name);
+		goto error;
+	}
 
-		address = of_get_integer(codec_np, "reg");
-		if (address < 0) {
-			dev_err(soc_card->dev,
-				"cannot determine I2C address for %s\n",
-				codec_np->full_name);
-			continue;
-		}
+	/* We assume that the first (or only) string in the compatible
+	 * field is the one that counts.
+	 */
+	compat = of_get_property(codec_np, "compatible", NULL);
+	if (!compat) {
+		dev_err(soc_card->dev,
+			"missing compatible property for %s\n",
+			ssi_np->full_name);
+		goto error;
+	}
 
-		pcm_config->codec_num = bus << 16 | address;
+	/* We only care about the part after the comma */
+	p = strchr(compat, ',');
+	strcpy(soc_card_data->codec_name, p ? p + 1 : compat);
 
-		dev_dbg(soc_card->dev, "registering cpu %s with codec %s-%x\n",
-			pcm_config->cpu_dai, pcm_config->codec,
-			pcm_config->codec_num);
+	/* Now determine the I2C bus and address of the codec */
+	bus = of_get_integer(of_get_parent(codec_np), "cell-index");
+	if (bus < 0) {
+		dev_err(soc_card->dev,
+			"cannot determine I2C bus number for %s\n",
+			codec_np->full_name);
+		goto error;
+	}
 
-		if (++ssi == MAX_SSI)
-			break;
+	address = of_get_integer(codec_np, "reg");
+	if (address < 0) {
+		dev_err(soc_card->dev,
+			"cannot determine I2C address for %s\n",
+			codec_np->full_name);
+		goto error;
 	}
 
-	ret = snd_soc_card_create_pcms(soc_card, soc_card_data->configs, ssi);
+	soc_card_data->pcm_config.codec_num = bus << 16 | address;
+
+	ret = snd_soc_card_create_pcms(soc_card, &soc_card_data->pcm_config, 1);
 	if (ret) {
 		dev_err(soc_card->dev, "could not create PCMs\n");
 		return ret;
 	}
 
-	soc_card_data->num_configs = ssi;
-
 	platform_set_drvdata(pdev, soc_card);
 
 	ret = snd_soc_card_register(soc_card);
@@ -493,7 +460,8 @@ static struct platform_driver mpc8610_hpcd_driver = {
 	.remove 	= __devexit_p(mpc8610_hpcd_remove),
 };
 
-static struct platform_device *pdev;
+/* Platform device pointers for each SSI  */
+static struct platform_device *pdev[MAX_SSI];
 
 /**
  * mpc8610_hpcd_init: fabric driver initialization.
@@ -502,6 +470,9 @@ static struct platform_device *pdev;
  */
 static int __init mpc8610_hpcd_init(void)
 {
+	struct resource res;
+	struct device_node *ssi_np;
+	unsigned int i = 0;
 	int ret;
 
 	pr_info("Freescale MPC8610 HPCD ASoC fabric driver\n");
@@ -512,12 +483,35 @@ static int __init mpc8610_hpcd_init(void)
 		return ret;
 	}
 
-	pdev = platform_device_register_simple(mpc8610_hpcd_driver.driver.name,
-		0, NULL, 0);
-	if (!pdev) {
-		pr_err("mpc8610-hpcd: could not register device\n");
-		platform_driver_unregister(&mpc8610_hpcd_driver);
-		return ret;
+	memset(pdev, 0, sizeof(pdev));
+
+	memset(&res, 0, sizeof(res));
+	res.name = "ssi";
+
+	for_each_compatible_node(ssi_np, NULL, "fsl,mpc8610-ssi") {
+		int id;
+
+		id = of_get_integer(ssi_np, "cell-index");
+		if (id < 0) {
+			pr_err("mpc8610-hpcd: no cell-index for %s\n",
+				ssi_np->full_name);
+			continue;
+		}
+		if (!find_codec(ssi_np))
+			/* No codec node? Skip it */
+			continue;
+
+		res.start = (resource_size_t) ssi_np;
+
+		pdev[i] = platform_device_register_simple(
+			mpc8610_hpcd_driver.driver.name, id, &res, 1);
+		if (!pdev[i]) {
+			pr_err("mpc8610-hpcd: could not register %s\n",
+				ssi_np->full_name);
+			continue;
+		}
+
+		i++;
 	}
 
 	return 0;
@@ -530,7 +524,11 @@ static int __init mpc8610_hpcd_init(void)
  */
 static void __exit mpc8610_hpcd_exit(void)
 {
-	platform_device_unregister(pdev);
+	unsigned int i;
+
+	for (i = 0; i < MAX_SSI; i++)
+		platform_device_unregister(pdev[i]);
+
 	platform_driver_unregister(&mpc8610_hpcd_driver);
 }
 
-- 
1.5.5

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

You're loading the fabric driver as a platform driver controlled via
Kconfig. Has any progress been made on a way to trigger loading it via
the device tree?

Why are you calling all of the mpc code fsl? Freescale makes other
processors that start with imx, mcp, mac, mmc, m68... I find calling
the mpc code fsl to be very confusing. Or is Freescale planning on
putting these ssi, dma, etc cells into all of their other processors?

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:
> You're loading the fabric driver as a platform driver controlled via
> Kconfig. Has any progress been made on a way to trigger loading it via
> the device tree?

What should I trigger it on?  For this idea to work, I need to have a 
node in the device tree that no other driver wants.  What node should 
that be?

> Why are you calling all of the mpc code fsl? Freescale makes other
> processors that start with imx, mcp, mac, mmc, m68...

The SSI device exists in both i.MX and MPC parts, so it makes sense to 
call that fsl_.  The DMA controller could exist on non-MPC parts, but 
currently it doesn't.  I called it fsl_dma.c just to be consistent.

If it turns out that there's a real name clash, we can always rename them.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

I've been out of the loop for a few months working on something else.
I just looked at this driver. My understanding of the fabric driver is
different than the way you implemented it. I thought the fabric driver
would only contain the essence of code specific to the board. Your
fabric driver has a lot more in it than just this minimal essence.

In my model a lot of the code in your fabric driver would be pushed
into the ssi driver. So if you used ssi and a codec in the standard
manner, the board wouldn't need a fabric driver at all. That would
probably be the case for most AC97/HDA systems.

My fabric driver contains code for hooking processor GPIOs up to the
codec, initializing an external clock generator, etc. All of the code
for parsing the device tree and setting up DMA is in the ssi driver.

Moving this code would also fix your problem with creating a fabric
device for each SSI. There shouldn't need to be a loop in the fabric
driver iterating over the SSIs.


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

 * Even more frustrating is the fact that the CS4270 driver is an I2C
 * driver, so it's probed via the I2C bus.  We need to update the powerpc
 * platform to initialize client->dev->archdata.of_node to point to the
 * device node.  Then the CS4270 driver can get its own clock rate.

That is easy to fix in drivers/of/of_i2c.c, add...
info.platform_data = node;

The node will be in
client->dev.platform_data

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/12/08, Jon Smirl <jonsmirl@gmail.com> wrote:
>  * Even more frustrating is the fact that the CS4270 driver is an I2C
>   * driver, so it's probed via the I2C bus.  We need to update the powerpc
>   * platform to initialize client->dev->archdata.of_node to point to the
>   * device node.  Then the CS4270 driver can get its own clock rate.
>
>  That is easy to fix in drivers/of/of_i2c.c, add...
>  info.platform_data = node;
>
>  The node will be in
>  client->dev.platform_data

Thinking about this for a minute, it is not what you want. Adding code
like this will make the codec driver platform specific. Codecs are
platform drivers, not of_platform drivers specific since they are
supposed to be arch independent.

Instead the codec should make a module parameter for the clock rate.
On platforms without a device tree you would set the clock rate on the
load module command line or some other way. With device trees the ssi
driver can stuff the clock rate into the module parameter after the
module is loaded.

Now I just have to remember how to access a module parameter from code
inside the kernel. I think there is a way to access the names of the
modules parameters. Code in the ssi driver (or a library) could match
up entries in the devices tree to parameter names. That will let one
piece of code in ssi load different modules with varying parameters.

Is there a better way to get parameters to cross platform codec drivers?

>  --
>  Jon Smirl
>  jonsmirl@gmail.com
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:
> On 6/12/08, Jon Smirl <jonsmirl@gmail.com> wrote:
>>  * Even more frustrating is the fact that the CS4270 driver is an I2C
>>   * driver, so it's probed via the I2C bus.  We need to update the powerpc
>>   * platform to initialize client->dev->archdata.of_node to point to the
>>   * device node.  Then the CS4270 driver can get its own clock rate.
>>
>>  That is easy to fix in drivers/of/of_i2c.c, add...
>>  info.platform_data = node;
>>
>>  The node will be in
>>  client->dev.platform_data
> 
> Thinking about this for a minute, it is not what you want. Adding code
> like this will make the codec driver platform specific. Codecs are
> platform drivers, not of_platform drivers specific since they are
> supposed to be arch independent.

Or, we could say that client->dev.platform_data must point to 
platform-agnostic information for the codec, regardless of the 
architecture.  That is, in order for this codec driver to work on ARM, 
the ARM's platform code would need to do the same thing.

That's why I didn't go this route right away.  I wanted to think about 
it some more.

> Instead the codec should make a module parameter for the clock rate.

Ugh.  I'm not really crazy about parameters.

> On platforms without a device tree you would set the clock rate on the
> load module command line or some other way. With device trees the ssi
> driver can stuff the clock rate into the module parameter after the
> module is loaded.

Well, ASoC already has a mechanism for passing the clock rate from the 
fabric driver to the codec driver.  I currently use it, as you know, but 
my comment in the code is really just "thinking out loud" about another 
way to do it.  I'm still on the fence about this.

> Now I just have to remember how to access a module parameter from code
> inside the kernel. I think there is a way to access the names of the
> modules parameters.

Yeah, it's not a big deal, but I don't think there's a standard way for 
platform code to pass parameters to a module.

> Code in the ssi driver (or a library) could match
> up entries in the devices tree to parameter names. That will let one
> piece of code in ssi load different modules with varying parameters.

This is normally done the way fsl_soc.c does it:  Create a platform 
device, and add platform resources objects to it.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Fri, Jun 13, 2008 at 07:17:42AM -0500, Timur Tabi wrote:
> Jon Smirl wrote:
>> On 6/12/08, Jon Smirl <jonsmirl@gmail.com> wrote:

>> Instead the codec should make a module parameter for the clock rate.

> Ugh.  I'm not really crazy about parameters.

Yes, that doesn't seem too clever.

Besides, the codec drivers need to treat all the clocking configuration
as dynamic - for example, it's a fairly common pattern for systems that
derive the audio clocks from a PLL or similar source to reconfigure the
master clock rate depending on the sample rates currently in use,
disabling it when not required.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jun 13, 2008 at 07:17:42AM -0500, Timur Tabi wrote:
>  > Jon Smirl wrote:
>  >> On 6/12/08, Jon Smirl <jonsmirl@gmail.com> wrote:
>
>
> >> Instead the codec should make a module parameter for the clock rate.
>
>  > Ugh.  I'm not really crazy about parameters.
>
>
> Yes, that doesn't seem too clever.
>
>  Besides, the codec drivers need to treat all the clocking configuration
>  as dynamic - for example, it's a fairly common pattern for systems that
>  derive the audio clocks from a PLL or similar source to reconfigure the
>  master clock rate depending on the sample rates currently in use,
>  disabling it when not required.

My system works like you describe. We have an external clock
generator. It needs to be set up in the fabric code.

Isn't the parameter needed for codecs like AC97 where they supply the
clock back to the CPU? Timur's device tree is the one with a codec
clock rate. Are there other parameters the system might need to set
into a codec?


>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:

> My system works like you describe. We have an external clock
> generator. It needs to be set up in the fabric code.

But can the clock frequency change during runtime?  The current ASoC interface
can handle that.

> Isn't the parameter needed for codecs like AC97 where they supply the
> clock back to the CPU? Timur's device tree is the one with a codec
> clock rate. Are there other parameters the system might need to set
> into a codec?

The direction of the clock.  That's why I have "i2s-slave" in the SSI nodes.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
>  > My system works like you describe. We have an external clock
>  > generator. It needs to be set up in the fabric code.
>
>
> But can the clock frequency change during runtime?  The current ASoC interface
>  can handle that.

Our i2s hardware is capable of changing the clock frequency at
runtime. I haven't tried doing it in software yet. The main reason for
changing is to adjust to sources that are multiples of 44.1 vs 48K.
Intel HDA has a much better scheme for handling that via a fixed clock
and skipping slots. I suspect I'll have to do some work on alsa to
make this work.

Get Freescale to put an HDA interface into the SOC chips so that I can
used HDA codecs. The only reason we are using an external clock chip
is because the MPC5200 generates a 192K audio clock that has a 0.7%
error in it. If the MPC5200 had a fractional-N divider instead of an
integer one on the audio clock we wouldn't need the external chip.
There's no way to fix the MPC5200 clock error without breaking the USB
clock.

>  > Isn't the parameter needed for codecs like AC97 where they supply the
>  > clock back to the CPU? Timur's device tree is the one with a codec
>  > clock rate. Are there other parameters the system might need to set
>  > into a codec?
>
> The direction of the clock.  That's why I have "i2s-slave" in the SSI nodes.

There should be a way for the ssi driver to extract the parameters
from the device tree and then feed them into the codec in an arch
independent manner. The codecs need to stay arch independent. I
haven't tried doing this yet in the code since I haven't needed setup
parameters.

>
>
>  --
>  Timur Tabi
>  Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Fri, Jun 13, 2008 at 11:26:57AM -0400, Jon Smirl wrote:

[Dynamic clocking.]
> Intel HDA has a much better scheme for handling that via a fixed clock
> and skipping slots. I suspect I'll have to do some work on alsa to
> make this work.

As well adjusting things to support the widest range of sample rates you
also see this used in order to reduce power consumption.

> On 6/13/08, Timur Tabi <timur@freescale.com> wrote:

> >  > clock back to the CPU? Timur's device tree is the one with a codec
> >  > clock rate. Are there other parameters the system might need to set
> >  > into a codec?

> > The direction of the clock.  That's why I have "i2s-slave" in the SSI nodes.

Not just one clock, either - some devices support independent control of
the directions of frame and bit clocks, for example.  Other things that
can be configured include clock sharing, timeslot configuration for
buses with more than two devices, parameters for the PLLs and dividers 
in the internal clocking tree of the codec, clock outputs not associated
with an audio interface and jack detection.

For added fun, consider codecs with multiple I2S interfaces.

> There should be a way for the ssi driver to extract the parameters
> from the device tree and then feed them into the codec in an arch
> independent manner. The codecs need to stay arch independent. I
> haven't tried doing this yet in the code since I haven't needed setup
> parameters.

Could you explain in more detail why you want to do this in the SSI
driver in particular?  Trying to have a generic machine driver that
takes configuration from the device tree seems like a worthwhile goal
but I'm not clear what the gain from integrating this into the
controller driver is.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jun 13, 2008 at 11:26:57AM -0400, Jon Smirl wrote:
>
>  [Dynamic clocking.]
>
> > Intel HDA has a much better scheme for handling that via a fixed clock
>  > and skipping slots. I suspect I'll have to do some work on alsa to
>  > make this work.
>
>
> As well adjusting things to support the widest range of sample rates you
>  also see this used in order to reduce power consumption.
>
>
>  > On 6/13/08, Timur Tabi <timur@freescale.com> wrote:
>
>
> > >  > clock back to the CPU? Timur's device tree is the one with a codec
>  > >  > clock rate. Are there other parameters the system might need to set
>  > >  > into a codec?
>
>  > > The direction of the clock.  That's why I have "i2s-slave" in the SSI nodes.
>
>
> Not just one clock, either - some devices support independent control of
>  the directions of frame and bit clocks, for example.  Other things that
>  can be configured include clock sharing, timeslot configuration for
>  buses with more than two devices, parameters for the PLLs and dividers
>  in the internal clocking tree of the codec, clock outputs not associated
>  with an audio interface and jack detection.
>
>  For added fun, consider codecs with multiple I2S interfaces.
>
>
>  > There should be a way for the ssi driver to extract the parameters
>  > from the device tree and then feed them into the codec in an arch
>  > independent manner. The codecs need to stay arch independent. I
>  > haven't tried doing this yet in the code since I haven't needed setup
>  > parameters.
>
>
> Could you explain in more detail why you want to do this in the SSI
>  driver in particular?  Trying to have a generic machine driver that
>  takes configuration from the device tree seems like a worthwhile goal
>  but I'm not clear what the gain from integrating this into the
>  controller driver is.

In the PowerPC world the ssi drivers are loaded based on the device
tree. When the ssi driver loads it gets passed in it's device tree
node and can see what codecs are on its bus.
Codecs get loaded two ways, if they are i2c based the i2c subsystem
loaded them. That's because of the PowerPC rule that the controlling
bus should load the driver. If it is an AC97/HDA codec, the ssi code
needs to trigger the loading since the AC97/HDA bus is the controlling
bus.

Note that on the mpc5200 there are separate drivers for psc chip in
the different modes. Instead of having a unified ssi driver, there are
uart, ac97, i2s, irda, etc drivers.  Each of these drivers configures
the psc in to the correct mode when they load. (I think Timur has a
unified driver, I don't work with his hardware).

After the codec is loaded it may need parameters from the device tree.
My though was to have two mechanisms. First a very simple one where
the SSI driver just copies the parameters from the device tree into
the codec. That removes the need for platform specific code from the
codec driver.  The simple copy should be enough for AC97/HDA.

In the second case the SSI would have a call out into the fabric code.
If the simple copy isn't enough the fabric code can do whatever
complicated things it needs to do. This is the case in my hardware,
the fabric driver needs to set up the external clock chip.

The basic concept is that there is one SSI driver per device and a
single global fabric driver. So anything that needs to be done on a
per ssi case needs to go into the ssi driver. Making the soc
structures and attaching the codec is a per ssi task, so the code
should go into the ssi driver. There's on a single, global fabric
driver, that's why Timur had to add a loop initializing the ssi
drivers.


>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:
> Each of these drivers configures
> the psc in to the correct mode when they load. (I think Timur has a
> unified driver, I don't work with his hardware).

I only support the I2S mode of the SSI hardware.  If I were to add AC97 support,
then I would probably make it a unified driver, but I'm not sure because I don't
know enough about AC97 to see how that would work.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Fri, Jun 13, 2008 at 12:59:44PM -0400, Jon Smirl wrote:

> In the PowerPC world the ssi drivers are loaded based on the device
> tree. When the ssi driver loads it gets passed in it's device tree
> node and can see what codecs are on its bus.
> Codecs get loaded two ways, if they are i2c based the i2c subsystem
> loaded them. That's because of the PowerPC rule that the controlling
> bus should load the driver. If it is an AC97/HDA codec, the ssi code
> needs to trigger the loading since the AC97/HDA bus is the controlling
> bus.

This is no different to any other system.

> After the codec is loaded it may need parameters from the device tree.
> My though was to have two mechanisms. First a very simple one where
> the SSI driver just copies the parameters from the device tree into
> the codec. That removes the need for platform specific code from the
> codec driver.  The simple copy should be enough for AC97/HDA.

> In the second case the SSI would have a call out into the fabric code.
> If the simple copy isn't enough the fabric code can do whatever
> complicated things it needs to do. This is the case in my hardware,
> the fabric driver needs to set up the external clock chip.

I'm not seeing much substantial difference between the two cases here:
in both of them you look at the system and load a machine driver, it's
just that in the first case the machine driver happens to be a generic
one that gets more of what it needs from the device tree.

> The basic concept is that there is one SSI driver per device and a
> single global fabric driver. So anything that needs to be done on a
> per ssi case needs to go into the ssi driver. Making the soc
> structures and attaching the codec is a per ssi task, so the code

Remember that if there's more than one SSI<->codec link they may not be
independant of each other.  For example, a six channel DAC could be
connected to three stereo I2S ports on the SoC with shared clock
signals.  You can also get codec<->codec links (for example, separate
ADC and DAC parts or Bluetooth devices) which bypass the SoC entirely.

It may be that you just intend to wrap all these cases up in your "or
load a machine driver" case, I'm not clear.

> should go into the ssi driver. There's on a single, global fabric
> driver, that's why Timur had to add a loop initializing the ssi
> drivers.

There's one machine driver per system in V1, but one of the intentions
going forward is to remove that limitation.  IIRC it may already be
possible to have more than one SoC card in V2 but I don't know if anyone
actually did that yet.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Fri, Jun 13, 2008 at 12:59:44PM -0400, Jon Smirl wrote:
>
>  > In the PowerPC world the ssi drivers are loaded based on the device
>  > tree. When the ssi driver loads it gets passed in it's device tree
>  > node and can see what codecs are on its bus.
>  > Codecs get loaded two ways, if they are i2c based the i2c subsystem
>  > loaded them. That's because of the PowerPC rule that the controlling
>  > bus should load the driver. If it is an AC97/HDA codec, the ssi code
>  > needs to trigger the loading since the AC97/HDA bus is the controlling
>  > bus.
>
>
> This is no different to any other system.
>
>
>  > After the codec is loaded it may need parameters from the device tree.
>  > My though was to have two mechanisms. First a very simple one where
>  > the SSI driver just copies the parameters from the device tree into
>  > the codec. That removes the need for platform specific code from the
>  > codec driver.  The simple copy should be enough for AC97/HDA.
>
>  > In the second case the SSI would have a call out into the fabric code.
>  > If the simple copy isn't enough the fabric code can do whatever
>  > complicated things it needs to do. This is the case in my hardware,
>  > the fabric driver needs to set up the external clock chip.
>
>
> I'm not seeing much substantial difference between the two cases here:
>  in both of them you look at the system and load a machine driver, it's
>  just that in the first case the machine driver happens to be a generic
>  one that gets more of what it needs from the device tree.

The idea was to allow the fabric driver to be optional for very simple
hard. For example the Efika has simple hardware. It is a STAC9766
hooked to a PSC. No GPIOs or special clocks are involved.  AC97 has
registers for allowing the BIOS to indicate jack presence. Efika
shouldn't need a fabric driver.

Everything except for the most basic system will need a fabric driver.
I think being able to run with an optional fabric driver is a good
test of having the design right.
>
>
>  > The basic concept is that there is one SSI driver per device and a
>  > single global fabric driver. So anything that needs to be done on a
>  > per ssi case needs to go into the ssi driver. Making the soc
>  > structures and attaching the codec is a per ssi task, so the code
>
>
> Remember that if there's more than one SSI<->codec link they may not be
>  independant of each other.  For example, a six channel DAC could be
>  connected to three stereo I2S ports on the SoC with shared clock
>  signals.  You can also get codec<->codec links (for example, separate
>  ADC and DAC parts or Bluetooth devices) which bypass the SoC entirely.
>
>  It may be that you just intend to wrap all these cases up in your "or
>  load a machine driver" case, I'm not clear.

Yes, anything but the most basic configuration ends up in the loading
a fabric driver. I suspect all i2s setups will need one. AC97 and HDA
may not.

BTW, the fabric terminology came form the Mac drivers in the aoa directory.

>
>
>  > should go into the ssi driver. There's on a single, global fabric
>  > driver, that's why Timur had to add a loop initializing the ssi
>  > drivers.
>
>
> There's one machine driver per system in V1, but one of the intentions
>  going forward is to remove that limitation.  IIRC it may already be
>  possible to have more than one SoC card in V2 but I don't know if anyone
>  actually did that yet.

I would find it confusing to have more than one machine/fabric driver
in a system.

-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Liam Girdwood]

On Fri, 2008-06-13 at 15:25 -0400, Jon Smirl wrote:
> On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> > On Fri, Jun 13, 2008 at 12:59:44PM -0400, Jon Smirl wrote:
> >
> >  > In the PowerPC world the ssi drivers are loaded based on the device
> >  > tree. When the ssi driver loads it gets passed in it's device tree
> >  > node and can see what codecs are on its bus.
> >  > Codecs get loaded two ways, if they are i2c based the i2c subsystem
> >  > loaded them. That's because of the PowerPC rule that the controlling
> >  > bus should load the driver. If it is an AC97/HDA codec, the ssi code
> >  > needs to trigger the loading since the AC97/HDA bus is the controlling
> >  > bus.
> >
> >
> > This is no different to any other system.
> >
> >
> >  > After the codec is loaded it may need parameters from the device tree.
> >  > My though was to have two mechanisms. First a very simple one where
> >  > the SSI driver just copies the parameters from the device tree into
> >  > the codec. That removes the need for platform specific code from the
> >  > codec driver.  The simple copy should be enough for AC97/HDA.
> >
> >  > In the second case the SSI would have a call out into the fabric code.
> >  > If the simple copy isn't enough the fabric code can do whatever
> >  > complicated things it needs to do. This is the case in my hardware,
> >  > the fabric driver needs to set up the external clock chip.
> >
> >
> > I'm not seeing much substantial difference between the two cases here:
> >  in both of them you look at the system and load a machine driver, it's
> >  just that in the first case the machine driver happens to be a generic
> >  one that gets more of what it needs from the device tree.
> 
> The idea was to allow the fabric driver to be optional for very simple
> hard. For example the Efika has simple hardware. It is a STAC9766
> hooked to a PSC. No GPIOs or special clocks are involved.  AC97 has
> registers for allowing the BIOS to indicate jack presence. Efika
> shouldn't need a fabric driver.
> 
> Everything except for the most basic system will need a fabric driver.
> I think being able to run with an optional fabric driver is a good
> test of having the design right.

The fabric is still required for most SoC based AC97 configurations.
Although for AC97 (basic stereo playback & capture) it's only really
used to map the DMA channel, irq & AC97 port to the codec (so it's
possible this could be optional or minimised in the future) and to
configure unused DAPM pins (to conserve power).

The main reason for AC97 fabric is when we want to do anything special
with AC97 (which we usually do in embedded systems - it's usually an
embedded codec's selling point!). Another benefit is for workarounds
(the AC97 spec has many grey area's that have been implemented
differently by different vendors).

AC97 implements better on Laptop/PC hardware because the spec is well
defined in the basic areas (i.e stereo playback and capture, 5.1
playback). 

> >
> >
> >  > The basic concept is that there is one SSI driver per device and a
> >  > single global fabric driver. So anything that needs to be done on a
> >  > per ssi case needs to go into the ssi driver. Making the soc
> >  > structures and attaching the codec is a per ssi task, so the code
> >
> >
> > Remember that if there's more than one SSI<->codec link they may not be
> >  independant of each other.  For example, a six channel DAC could be
> >  connected to three stereo I2S ports on the SoC with shared clock
> >  signals.  You can also get codec<->codec links (for example, separate
> >  ADC and DAC parts or Bluetooth devices) which bypass the SoC entirely.
> >
> >  It may be that you just intend to wrap all these cases up in your "or
> >  load a machine driver" case, I'm not clear.
> 
> Yes, anything but the most basic configuration ends up in the loading
> a fabric driver. I suspect all i2s setups will need one. AC97 and HDA
> may not.

I agree, all I2S & PCM setups will need fabric. HDA and Mipi won't due
to well defined specs whilst AC97 is the grey area. AC97 needs fabric on
some systems but not others.

Liam

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Fri, Jun 13, 2008 at 09:04:53PM +0100, Liam Girdwood wrote:

> > Yes, anything but the most basic configuration ends up in the loading
> > a fabric driver. I suspect all i2s setups will need one. AC97 and HDA
> > may not.

Ah!  I had thought that you wanted to see much wider use of generic
driver than this.

> I agree, all I2S & PCM setups will need fabric. HDA and Mipi won't due

Me too - there's just not the standardisation for software to work with
for I2S and PCM.

> to well defined specs whilst AC97 is the grey area. AC97 needs fabric on
> some systems but not others.

Yeah, part of what I was driving at was that it would be good if we
could handle standard conforming AC97 systems by having a fabric/machine
driver which provides the required glue to connect up whatever codecs
it was able to probe on the bus to the controller.

Having such a driver would mean that with ASoC support for their AC97
controller platforms would only have to have a mechanism to decide to
use the generic driver to also get support for standard AC97 subsystems.
This would leave these systems using an ASoC machine driver but not one
that is custom to any given system.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:
> I've been out of the loop for a few months working on something else.
> I just looked at this driver. My understanding of the fabric driver is
> different than the way you implemented it. 

Well, my fabric driver does work, so it can't be that different. :-)

> I thought the fabric driver
> would only contain the essence of code specific to the board. 

No, the fabric driver also tells ASoC how the other three drivers are 
connected.  That is, it tells ASoC "please connect SSI1 to the codec at 
address 4F, and here's the DMA information that the SSI driver needs".

> In my model a lot of the code in your fabric driver would be pushed
> into the ssi driver. So if you used ssi and a codec in the standard
> manner, the board wouldn't need a fabric driver at all. That would
> probably be the case for most AC97/HDA systems.

The SSI driver is only concerned with talking to an SSI.  I don't see 
how you could get away with no fabric driver.  That just isn't the model.

> My fabric driver contains code for hooking processor GPIOs up to the
> codec, initializing an external clock generator, etc. All of the code
> for parsing the device tree and setting up DMA is in the ssi driver.

That's how my driver *used* to be.  With ASoC V2, I chose to move some 
of that code from the SSI driver into the fabric driver.  It just makes 
more sense to me.

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Thu, Jun 12, 2008 at 10:57:39PM -0400, Jon Smirl wrote:

> In my model a lot of the code in your fabric driver would be pushed
> into the ssi driver. So if you used ssi and a codec in the standard
> manner, the board wouldn't need a fabric driver at all. That would
> probably be the case for most AC97/HDA systems.

I'd certainly like to see some standard support for setting up at least
AC97 DAI links automatically based on the probed codec.  That bit could
probably be done by the core.  HDA should be amenable to this model too
but I haven't looked at it in anything more than passing detail yet.

That's only part of the story, though.  What's much more tricky is
making the decision that you've got all the components for the sound
subsystem - for example, there are AC97 codecs like the WM9712 and
WM9713 which also have I2S interfaces.  You also get systems which need
to jump through hoops to set up the clocking since they're doing
non-standard things.  Simple systems would probably need an explict flag
to say that they could be handled as such, which isn't a million miles
off having something to load a generic machine driver.

Things get a lot more tricky for I2S due to the complexity of the
available clocking configurations - there are so many possible ways of
connecting devices, especially once you start to take into account codec
internal clocking and systems which support more than two devices 

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Jun 12, 2008 at 10:57:39PM -0400, Jon Smirl wrote:
>
>  > In my model a lot of the code in your fabric driver would be pushed
>  > into the ssi driver. So if you used ssi and a codec in the standard
>  > manner, the board wouldn't need a fabric driver at all. That would
>  > probably be the case for most AC97/HDA systems.
>
>
> I'd certainly like to see some standard support for setting up at least
>  AC97 DAI links automatically based on the probed codec.  That bit could
>  probably be done by the core.  HDA should be amenable to this model too
>  but I haven't looked at it in anything more than passing detail yet.
>
>  That's only part of the story, though.  What's much more tricky is
>  making the decision that you've got all the components for the sound
>  subsystem - for example, there are AC97 codecs like the WM9712 and
>  WM9713 which also have I2S interfaces.  You also get systems which need
>  to jump through hoops to set up the clocking since they're doing
>  non-standard things.  Simple systems would probably need an explict flag
>  to say that they could be handled as such, which isn't a million miles
>  off having something to load a generic machine driver.

We can't eliminate a fabric driver is all cases, I'd just like to
minimize its need. This code has the card and codec creation code in
the fabric driver, I would push down into the ssi driver. Code in the
ssi would locate the codec and load it. The model needs to be able to
function if there is no need for a fabric driver.

That also flips things around, now the ssi driver needs to locate the
fabric driver, not the other way around. In this model the fabric
driver doesn't need to make a device, it just becomes a library of
code called by the ssi driver. Code in the ssi driver could make a
list of codec parameters, pass it to the fabric driver for
modification, and then set it into the codec.

We're missing a cross platform way to set parameters into a codec. A
quick fix is to pass the device tree handle in and use arch specific
code in the codec, but that will need to be removed in the longer
term.

>
>  Things get a lot more tricky for I2S due to the complexity of the
>  available clocking configurations - there are so many possible ways of
>  connecting devices, especially once you start to take into account codec
>  internal clocking and systems which support more than two devices
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:
> On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>> On Thu, Jun 12, 2008 at 10:57:39PM -0400, Jon Smirl wrote:
>>
>>  > In my model a lot of the code in your fabric driver would be pushed
>>  > into the ssi driver. So if you used ssi and a codec in the standard
>>  > manner, the board wouldn't need a fabric driver at all. That would
>>  > probably be the case for most AC97/HDA systems.
>>
>>
>> I'd certainly like to see some standard support for setting up at least
>>  AC97 DAI links automatically based on the probed codec.  That bit could
>>  probably be done by the core.  HDA should be amenable to this model too
>>  but I haven't looked at it in anything more than passing detail yet.
>>
>>  That's only part of the story, though.  What's much more tricky is
>>  making the decision that you've got all the components for the sound
>>  subsystem - for example, there are AC97 codecs like the WM9712 and
>>  WM9713 which also have I2S interfaces.  You also get systems which need
>>  to jump through hoops to set up the clocking since they're doing
>>  non-standard things.  Simple systems would probably need an explict flag
>>  to say that they could be handled as such, which isn't a million miles
>>  off having something to load a generic machine driver.
> 
> We can't eliminate a fabric driver is all cases, I'd just like to
> minimize its need. This code has the card and codec creation code in
> the fabric driver, I would push down into the ssi driver. Code in the
> ssi would locate the codec and load it. The model needs to be able to
> function if there is no need for a fabric driver.

Like I said earlier, I don't like this idea.  I don't want all this gunk in the
SSI driver, and I don't see anything wrong with the fabric driver being the
master that sets up all the inter-device connections.

> That also flips things around, now the ssi driver needs to locate the
> fabric driver, not the other way around. In this model the fabric
> driver doesn't need to make a device, it just becomes a library of
> code called by the ssi driver.

The DMA driver is already a library that is called by the SSI.  It registers
itself as a regular driver, but under ASoC V2, it really acts like a library.

> Code in the ssi driver could make a
> list of codec parameters, pass it to the fabric driver for
> modification, and then set it into the codec.

That sounds too complicated.

> We're missing a cross platform way to set parameters into a codec. 

No we're not.  ASoC already provides an API for sending parameters to a codec,
and if we need more than that, we can create platform resources and pass those
to the codec.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>  > On 6/13/08, Mark Brown <broonie@opensource.wolfsonmicro.com> wrote:
>  >> On Thu, Jun 12, 2008 at 10:57:39PM -0400, Jon Smirl wrote:
>  >>
>  >>  > In my model a lot of the code in your fabric driver would be pushed
>  >>  > into the ssi driver. So if you used ssi and a codec in the standard
>  >>  > manner, the board wouldn't need a fabric driver at all. That would
>  >>  > probably be the case for most AC97/HDA systems.
>  >>
>  >>
>  >> I'd certainly like to see some standard support for setting up at least
>  >>  AC97 DAI links automatically based on the probed codec.  That bit could
>  >>  probably be done by the core.  HDA should be amenable to this model too
>  >>  but I haven't looked at it in anything more than passing detail yet.
>  >>
>  >>  That's only part of the story, though.  What's much more tricky is
>  >>  making the decision that you've got all the components for the sound
>  >>  subsystem - for example, there are AC97 codecs like the WM9712 and
>  >>  WM9713 which also have I2S interfaces.  You also get systems which need
>  >>  to jump through hoops to set up the clocking since they're doing
>  >>  non-standard things.  Simple systems would probably need an explict flag
>  >>  to say that they could be handled as such, which isn't a million miles
>  >>  off having something to load a generic machine driver.
>  >
>  > We can't eliminate a fabric driver is all cases, I'd just like to
>  > minimize its need. This code has the card and codec creation code in
>  > the fabric driver, I would push down into the ssi driver. Code in the
>  > ssi would locate the codec and load it. The model needs to be able to
>  > function if there is no need for a fabric driver.
>
>
> Like I said earlier, I don't like this idea.  I don't want all this gunk in the
>  SSI driver, and I don't see anything wrong with the fabric driver being the
>  master that sets up all the inter-device connections.

That forces every board to copy/paste the fabric driver and then makes
tweaks to it even if it doesn't need board specific code.


>  > That also flips things around, now the ssi driver needs to locate the
>  > fabric driver, not the other way around. In this model the fabric
>  > driver doesn't need to make a device, it just becomes a library of
>  > code called by the ssi driver.
>
>
> The DMA driver is already a library that is called by the SSI.  It registers
>  itself as a regular driver, but under ASoC V2, it really acts like a library.
>
>
>  > Code in the ssi driver could make a
>  > list of codec parameters, pass it to the fabric driver for
>  > modification, and then set it into the codec.
>
>
> That sounds too complicated.
>
>
>  > We're missing a cross platform way to set parameters into a codec.
>
>
> No we're not.  ASoC already provides an API for sending parameters to a codec,
>  and if we need more than that, we can create platform resources and pass those
>  to the codec.
>
>
>  --
>  Timur Tabi
>  Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:

> That forces every board to copy/paste the fabric driver and then makes
> tweaks to it even if it doesn't need board specific code.

Not at all.   There's no reason we can't create a generic fabric driver that
does no board-level programming.  I haven't done it because the MPC8610 does
need that kind of programming, and it's based on the information I get from the
SSI driver.  But you could just as easily take my fabric driver and make it
generic.  I don't know who would use it, though.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Jon Smirl]

On 6/13/08, Timur Tabi <timur@freescale.com> wrote:
> Jon Smirl wrote:
>
>  > That forces every board to copy/paste the fabric driver and then makes
>  > tweaks to it even if it doesn't need board specific code.
>
>
> Not at all.   There's no reason we can't create a generic fabric driver that
>  does no board-level programming.  I haven't done it because the MPC8610 does
>  need that kind of programming, and it's based on the information I get from the
>  SSI driver.  But you could just as easily take my fabric driver and make it
>  generic.  I don't know who would use it, though.

Doesn't the fact that you needed to modify the fabric driver to create
two PCMs indicate that there is something wrong with the design?
There is one fabric and two ssi devices.


>
>
>  --
>
> Timur Tabi
>  Linux kernel developer at Freescale
>


-- 
Jon Smirl
jonsmirl@gmail.com

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Timur Tabi]

Jon Smirl wrote:

> Doesn't the fact that you needed to modify the fabric driver to create
> two PCMs indicate that there is something wrong with the design?
> There is one fabric and two ssi devices.

Well, you have a point there.  There is some redundancy between my fabric and
SSI drivers when it comes to figuring out what devices there are.

-- 
Timur Tabi
Linux kernel developer at Freescale

Re: [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs

[Mark Brown]

On Thu, Jun 12, 2008 at 07:01:39PM -0500, Timur Tabi wrote:
> Update the Freescale MPC8610 HPCD fabric driver to create a new platform device
> for each SSI in the system, instead of registering multiple PCMs under one card.
> This is necessary for alsamixer to control the individual codecs.

Applied, thanks.