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.