On Thu, 20 Apr 2017 21:46:46 +0200, Mark Brown wrote:
On Wed, Apr 19, 2017 at 05:48:15PM +0100, Jose Abreu wrote:
What do you think Mark? If you want to keep the PCM as a module then we will need to abstract this more, by reducing the dependencies.
I think forcing this to be built in to the kernel (which is what the commit message says the change is going to do) is an obviously bad idea. Anything we add to the base kernel image needs to have a good reason to be there and it is hard to think what that reason might be for any audio driver, we need to be able to put this code into a module.
Well, I guess the original patch description caused a big confusion. As far as I see, the intention of the patch is not about the module or built-in kernel. Instead it's rather to fold designware_pcm stuff into the single designware_i2s driver.
The former is merely an extension of the latter driver, and the latter invokes the former directly. Thus there is little merit to keep them separate. I think the current code is even buggy, which allows to leave CONFIG_SND_DESIGNWARE_I2S=y and CONFIG_SND_DESIGNWARE_PCM=m.
So, I think Lubomir's change is right. But the patch subject and description should be rephrased.
One thing I don't like is the rename of the file. But in this particular case, it's unavoidable unless we rename the module name.
BTW, we should drop the superfluous EXPORT_SYMBOL*(), too.
thanks,
Takashi