[alsa-devel] Need help with WM8960
Mark,
I'm trying to port some old Linux code to upstream for a third-party board that uses a Freescale P1022 and a Wolfson WM8960. I've got audio working, but it's using a hack from the third-party code, and I don't know how to do implement the change properly, mostly because I don't understand DAPM.
The first problem is that I get this:
wm8960 0-001a: No platform data supplied
What kind of platform data is the driver expecting, and how do I get it that data? I don't seem to need any platform data to make the code work.
Second, I uploaded the hack to wm8960.c at http://pastebin.com/RwVy6cQg
You can see that he disables the DAPM controls for DACL, DACR, ADCL, and ADCR, and forces those bits set in the wm8960_probe function. There's also a comment that documents some limitations. What is the proper way to implement this change?
Third, in order to actually hear audio, I had to unmute the "Left Output Mixer PCM" and "Right Output Mixer PCM". Is this something that the machine driver should do automatically? And if so, how?
On Wed, Sep 12, 2012 at 06:09:00PM -0500, Timur Tabi wrote:
The first problem is that I get this:
wm8960 0-001a: No platform data supplied
What kind of platform data is the driver expecting, and how do I get it that data? I don't seem to need any platform data to make the code work.
So, it should be fairly clear from the code that its looking for a struct wm8960_data. You can actually leave this totally blank but it's important that the capless setting is correct one way or another - you need to look at the schematic and check to see if there are capacitors on the output not.
You can see that he disables the DAPM controls for DACL, DACR, ADCL, and ADCR, and forces those bits set in the wm8960_probe function. There's also a comment that documents some limitations. What is the proper way to implement this change?
I'm not entirely sure why the user is doing this - unfortuantely they just bodged the driver without reporting the issue. The first issue does not make much sense, the issues described are system level issues that are unrelated to the ADC for the most part (for example, things related to the micbias generally depend on the micbias enable). Without more information it's hard to comment. The second sounds like they missed the LRCM bit, I'll just add the ability to control that from platform data.
Third, in order to actually hear audio, I had to unmute the "Left Output Mixer PCM" and "Right Output Mixer PCM". Is this something that the machine driver should do automatically? And if so, how?
No, it's something that the application should do.
Mark Brown wrote:
Third, in order to actually hear audio, I had to unmute the "Left Output Mixer PCM" and "Right Output Mixer PCM". Is this something that the machine driver should do automatically? And if so, how?
No, it's something that the application should do.
How would the application know to unmute those two particular controls? The only way I figured it out is by launching alsamixer and playing with every control until I heard sound.
On Thu, Sep 13, 2012 at 03:26:03AM +0000, Tabi Timur-B04825 wrote:
Mark Brown wrote:
No, it's something that the application should do.
How would the application know to unmute those two particular controls? The only way I figured it out is by launching alsamixer and playing with every control until I heard sound.
The user needs to configure this as part of their system integration. There's no real way to know what configuration is sane for a given machine and the user is already doing system integration anyway; putting things into userspace makes things much easier to maintain. Even for something like this we'd need to know if the output is connected to headphone or line outputs as the volumes configured would generally be rather different (and of course that can vary at runtime...).
Mark Brown wrote:
So, it should be fairly clear from the code that its looking for a struct wm8960_data. You can actually leave this totally blank but it's important that the capless setting is correct one way or another - you need to look at the schematic and check to see if there are capacitors on the output not.
There are 120uF capacitors connected to HP_L and HP_R. Unfortunately, I can't quite figure out whether I need platform data or not.
Also, how do I get the platform data to the codec driver? Do I need to create some arch code that reads the device tree properties and parses them into a wm8960_data object, and then (somehow) injects that into the wm8960 driver?
Timur Tabi wrote:
Also, how do I get the platform data to the codec driver? Do I need to create some arch code that reads the device tree properties and parses them into a wm8960_data object, and then (somehow) injects that into the wm8960 driver?
So, I'm guessing that there is no magical way to inject platform data into an OF-initialized driver. What do you think about this patch:
@@ -1029,6 +1030,7 @@ static const struct regmap_config wm8960_regmap = { static __devinit int wm8960_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id) { + struct device_node *np = i2c->dev.of_node; struct wm8960_data *pdata = dev_get_platdata(&i2c->dev); struct wm8960_priv *wm8960; int ret; @@ -1038,6 +1040,31 @@ static __devinit int wm8960_i2c_probe(struct i2c_client *i2c, if (wm8960 == NULL) return -ENOMEM;
+ /* + * Are we running on an OF platform? If so, then parse the device + * tree node for properties and create the platform data object + * ourselves. + */ + if (np && !pdata) { + const __be32 *iprop; + int len; + + pdata = devm_kzalloc(&i2c->dev, sizeof(struct wm8960_data), + GFP_KERNEL); + if (!pdata) + return -ENOMEM; + + if (of_find_property(np, "wlf,capless", NULL)) + pdata->capless = true; + + iprop = of_get_property(np, "wlf,discharge-resistance", &len); + if (iprop && len == sizeof(uint32_t)) + pdata->dres = be32_to_cpup(iprop); + + if (of_find_property(np, "wlf,shared-lrclk", NULL)) + pdata->shared_lrclk = true; + + i2c->dev.platform_data = pdata; + } + wm8960->regmap = regmap_init_i2c(i2c, &wm8960_regmap); if (IS_ERR(wm8960->regmap)) return PTR_ERR(wm8960->regmap);
Timur Tabi wrote:
iprop = of_get_property(np, "wlf,discharge-resistance", &len);if (iprop && len == sizeof(uint32_t))pdata->dres = be32_to_cpup(iprop);
I just noticed that pdata->dres isn't actually used in the driver. In fact, I'm also confused by the DISOP bit. The datasheet says that the default value of DISOP is 0. However, the only time we touch register R29 is here:
case SND_SOC_BIAS_STANDBY: switch (codec->dapm.bias_level) { case SND_SOC_BIAS_PREPARE: /* Disable HP discharge */ snd_soc_update_bits(codec, WM8960_APOP2, WM8960_DISOP | WM8960_DRES_MASK, 0);
All we do here is set DISOP and DRES_MASK to 0, which they should already be.
Am I missing something?
On Thu, Sep 13, 2012 at 11:29:10AM -0500, Timur Tabi wrote:
I just noticed that pdata->dres isn't actually used in the driver. In
This was used in the original version of the driver when entering standby from off but we stopped using it when we switched from having a single startup sequence to having separate cap and capless sequences so it's redundant now and could be removed.
participants (3)
-
Mark Brown -
Tabi Timur-B04825 -
Timur Tabi