Re: [PATCH 000/127] ASoC: codec cleanup - codec probe/remove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 09, 2016 at 09:52:48AM +0200, Lars-Peter Clausen wrote:
> On 08/09/2016 06:52 AM, Kuninori Morimoto wrote:

> > Current "codec" has its .probe/.remove, and "component" has it too.
> > codec side is just relayed it to component side.
> > This is the time to cleanup.

> While for the widgets, routes and controls it did not really matter, I do
> not think this is the right approach for probe and remove.

> This is kind of sub-classing a normal design pattern found all throughout
> the kernel. You have a generic callback function and a specialized callback
> function that takes the type of the object that is being worked on rather
> than the generic object type.

I tend to agree here - look at struct device_driver for example, it has
a bunch of callbacks that are in practice rarely used because each
subsystem defines subsystem specific versions that are going to be much
more appropriate for most drivers.  This sort of refactoring tends to
make more sense when it's done with pure data (as for your previous
series).

> 
> Especially done as a blind mechanical conversion like you did this is not a
> cleanup. Most probe functions do not actually need to know that they work on
> a CODEC.
> 
> E.g. you now have very often this kind of code:
> 
>     struct snd_soc_codec *codec = snd_soc_component_to_codec(component);
>     struct tas5086_private *priv = snd_soc_codec_get_drvdata(codec);
> 
> This could easily be replaced with
> 
>     struct tas5086_private *priv = snd_soc_component_get_drvdata(component);
> 
> But again, this indirection is not the problem we currently have with ASoC
> and it's layering. This kind of cleanup should be done as a very final step
> when all other CODEC specific elements have been removed from the driver.

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux