Re: [PATCH 00/19]: add mrfld DSP support

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

On 06/13/2014 02:33 PM, Vinod Koul wrote:
> Here is the split patch series for adding DSP support for Intel's merrfield
> platform.
>
> The last patch si mxer update patch which we have been discussing over the list.
> This is only for discussion and sake of complteness. For that patch we need the
> dapm_kcontrol_get/set series to do merged OR patch to be reworked after
> compenent series

Hi,

Some general feedback on this series.

You are often accessing data structures which are more or less internal to 
the DAPM core, all without doing any of the necessary locking. Those are in 
particular the widget and path structures. This access in the driver will 
make it harder to make modifications to the core as you'd not only 
understand the core but also the special semantics that this driver puts on 
those data structures. If you think the core does not provide what you need 
try to extend the core with helper functions, with well defined semantics, 
that do what you need.

Also this series is a bit of a step backwards in terms of 
compartmentalization as it re-introduces things that we've been trying to 
get rid of over the last few months:
  * Don't access widget->platform or dapm->platform, use 
snd_soc_dapm_to_platform()
* Don't access dai->platform, use rtd->platform.
* Use regmap for IO instead of the read/write callbacks in the 
platform_driver struct.

- Lars