On Fri, Jun 8, 2012 at 6:10 AM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:

On Mon, Jun 04, 2012 at 02:37:29PM +0800, Zhangfei Gao wrote:

...

The SSPA is a configurable multi-channel audio serial (TDM) interface. It's configurable at runtime to support up to 128 channels and the number of bits per sample: 8, 12, 16, 20, 24 and 32 bits. It also support stereo format: I2S, left-justified or right-justified.

This looks mostly good (though you want to check your indentation a bit) but...

...

+static int __init mmp_sspa_modinit(void) +{

• audio_clk = clk_get(NULL, "mmp-audio");
• if (IS_ERR(audio_clk))
• return PTR_ERR(audio_clk);
• sysclk = clk_get(NULL, "mmp-sysclk");
• if (IS_ERR(sysclk))
• return PTR_ERR(sysclk);
• clk_enable(audio_clk);

You should be doing this stuff from the driver, not from the module init (and ideally you'd be managing the audio clock dynamically).  Even if it's the same clock every time in current silicon it's still a good idea to do this from a quality of implementation point of view.

Thanks, will remove the module init, and move clk staff to probe. However, dynamically control audio_clk make audio subsystem works abnormally.