On Mon, May 28, 2012 at 10:59 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Fri, May 25, 2012 at 03:11:02PM +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.
Mostly looks good. A few fairly minor things...
Thanks Mark for kind review.
- if (!cpu_dai->active) {
- clk_enable(sysclk);
- clk_enable(sspa->clk);
- }
The clock API is refcounted so you shouldn't need to worry about multiple enables, you should just be able to unconditionally enable and disable. If this is needed we probably have a problem we should fix.
Will update.
- switch (pll_id) {
- case MMP_SYSCLK:
- clk_set_rate(sysclk, freq_out);
- break;
- case MMP_SSPA_CLK:
- clk_set_rate(sspa->clk, freq_out);
- break;
You're ignoring the return values here.
will update.
- priv->sspa->clk = clk_get(&pdev->dev, NULL);
- if (IS_ERR(priv->sspa->clk)) {
- ret = PTR_ERR(priv->sspa->clk);
- goto err_free_clk;
- }
devm_clk_get().
Unfortunately, not find devm_clk_get.
- res = request_mem_region(res->start, resource_size(res),
- pdev->name);
- if (res == NULL) {
- dev_err(&pdev->dev, "failed to request memory resource\n");
- ret = -EBUSY;
- goto err_get_res;
- }
- priv->sspa->mmio_base = ioremap(res->start, resource_size(res));
- if (priv->sspa->mmio_base == NULL) {
- dev_err(&pdev->dev, "failed to ioremap() registers\n");
- ret = -ENODEV;
- goto err_free_mem;
- }
devm_request_and_ioremap().
will update.
+err_free_clk:
- devm_kfree(&pdev->dev, priv->dma_params);
+err_priv_dma_params:
- devm_kfree(&pdev->dev, priv->sspa);
+err_priv_sspa:
- devm_kfree(&pdev->dev, priv);
The whole point of the devm_ stuff is that you don't need to do stuff like this.
Thanks confirmation, not sure before, also find devm_kfree is used. Will remove the devm_kfree etc in err handle as well as remove function.
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel