On Mon, 2010-12-06 at 23:09 +0000, Mark Brown wrote:
On Mon, Dec 06, 2010 at 10:55:32PM +0000, Liam Girdwood wrote:
On Mon, 2010-12-06 at 22:43 +0000, Mark Brown wrote:
Why not use the existing speaker widget? It's at pretty much the same point in the sequence and is intended for use with external GPIO controlled speaker drivers. It'd be good to discuss this in the changelog.
In this case the driver block is on the CODEC IC and after the PGA in the audio output path, hence this version is better suited than the external GPIO version.
Sure, but it's fulfiling the same role in the system - it's just that these days a lot more CODECs are pulling speaker drivers directly into the CODEC die. Mostly these have worked well handled as PGAs so it's not been an issue.
In this case as we need to enable the PGA before the driver and disable the driver before the PGA for pop reduction. Hence the current ordering needs an addition/refactoring to deal with the newer generation of CODECs here.
I'd certainly expect to see it handled the same way from a DAPM sequencing point of view as it's fulfilling the same role in the system (so in the same slot rather than separately as the patch was doing). Do we just need to refactor the existing external widgets to be able to exist in either register or GPIO based versions?
+#define SND_SOC_DAPM_DRV(wname, wreg, wshift, winvert,\
wcontrols, wncontrols) \+{ .id = snd_soc_dapm_drv, .name = wname, .reg = wreg, .shift = wshift, \
- .invert = winvert, .kcontrols = wcontrols, .num_kcontrols = wncontrols}
The _DRV name seems rather opaque - I'd suggest _SPK as a name but obviously that's in use. If we do want this I guess _SPK_DRV or sommething.
I was thinking this too, but then I thought we may want to drive other loads than just speakers here. e.g. haptic, vibra
My main thing here is _DRV - it's not just that it's undescriptive, it's also that calling something a driver in a context like this is confusing. _OUT_DRV is another idea?
Ok, lets go for OUT_DRV.
Liam