Re: [PATCH 2/7] ASoC: DAPM: Pass snd_dapm_soc_update as parameter

[Lars-Peter Clausen <lars@metafoo.de>]

On 04/28/2011 09:40 PM, Mark Brown wrote:
> On Thu, Apr 28, 2011 at 06:46:08PM +0200, Lars-Peter Clausen wrote:
>> Pass the snd_dapm_soc_update struct as parameter to dapm_widget_update instead
>> of passing it indirectly as a field of the snd_soc_dapm_context struct.
>> This should make it more obvious were the struct is coming from and make the
>> code easier to comprehend.
> 
> If we're going to do something like this it would be better to make the
> update a more complex object so we weren't passing the stream update
> separately.  The update should be "what we're doing right now" which
> includes the stream operation. [...]

Hm... the current update struct only contains information on register updates,
since we don't not always want to update a register when we sent a stream event
and vice versa it does make sense to keep them separate.
If we end up adding more context information later we might want to reconsider,
but currently keeping them separate is fine in my opinion.

> 
> TBH I'm not sure how much of a help I find this, it's nice to not have
> to pass the operation we're doing about all the time especially if we
> end up using more context information in the middle of the operation
> (which I suspect we may end up doing).  You still have to work back
> through the call chain to find where the data came from.

Well, but you know that it is a parameter to the function call. With the
current scheme you don't know this immediately. Right now dapm->update is some
random global variable and it is not quite clear that it is only valid for the
time dapm_power_widgets is called.

- Lars