[alsa-devel] [PATCH v2 2/2] ASoC: omap-mcpdm: Replace legacy driver
peter.ujfalusi at ti.com
Tue Aug 23 08:49:44 CEST 2011
On Monday 22 August 2011 15:39:15 Lars-Peter Clausen wrote:
> omap_mcpdm_widgets is a global variable.
Yeah, as most of the snd_soc_dapm_widget.
> You assign to it in asoc_mcpdm_probe
Since at compile time I don't have the pointer for the mcpdm (it is allocated
earlier in the same function), I need to assign it somewhere.
> and read from it in omap_mcpdm_add_dapm_widget.
I don't see any reference to the omap_mcpdm_widgets in there.
> The fact that you hide your *mcpdm in a void pointer doesn't make it less
Well, from that point of view most of the kernel is hackish. We tend to have
void pointers for various things, like platform_data, device_data,
driver_data, private_data, etc.
You see, the point here is that this private_data for the widget can be used
for others as well, if needed. It would make no sense to put "struct
omap_mcpdm *mcpdm", just because I have this requirement first, does it?
For sure, I could have chosen to create one global pointer for this event
static struct omap_mcpdm *mcpdm_global;
Use the mcpdm_global within omap_mcpdm_interface_event function, and assign it
at asoc_mcpdm_probe time.
Would that look better? IMHO it is not.
As Mark suggested, we should have accessor for this, if we are going to have
such a thing.
The reason for this (as I described in the commit message):
OMAP McPDM has the requirement (along with the twl6040), that we need to stop
the cpu dai _after_ the DAC/ADC has been stopped on the codec side.
Now, I do not want to hard wire the twl6040, nor the omap-mcpdm with each
The omap-mcpdm implements the event callback, and the machine driver is
responsible to create the DAPM route to make sure we follow the correct
I could as well implemented these things in the machine driver (+global
pointer for mcpdm), but that can lead duplicated code in the future (new
machine driver, etc).
More information about the Alsa-devel