[alsa-devel] [PATCH][ASoC v2] Update Freescale MPC8610HPCD fabric driver to support multiple codecs
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); }
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 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.
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.
* 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
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 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.
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.
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 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.
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
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.
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 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.
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.
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.
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
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.
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.
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
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 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.
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 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.
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 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.
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.
participants (4)
-
Jon Smirl -
Liam Girdwood -
Mark Brown -
Timur Tabi