lkml.org 
[lkml]   [2019]   [Oct]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/7] ASoC: amd: Enabling I2S instance in DMA and DAI
On Sat, Oct 19, 2019 at 02:35:41AM +0530, Ravulapati Vishnu vardhan rao wrote:

> + case I2S_BT_INSTANCE:
> + val = rv_readl(rtd->acp3x_base + mmACP_BTTDM_ITER);
> + val = val | (rtd->xfer_resolution << 3);
> + rv_writel(val, rtd->acp3x_base + mmACP_BTTDM_ITER);
> + break;

For some reason the break; isn't indented with the rest of the block.
I'm fairly sure I've mentioned this before...

> + case I2S_SP_INSTANCE:
> + default:
> + val = rv_readl(rtd->acp3x_base + mmACP_I2STDM_ITER);
> + val = val | (rtd->xfer_resolution << 3);
> + rv_writel(val, rtd->acp3x_base + mmACP_I2STDM_ITER);
> + }

Missing break; here - again it's normal kernel coding style to include
it.

> + struct snd_soc_pcm_runtime *prtd = substream->private_data;
> + struct snd_soc_card *card = prtd->card;
> + struct acp3x_platform_info *pinfo = snd_soc_card_get_drvdata(card);
> +
> + if (pinfo) {
> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> + rtd->i2s_instance = pinfo->play_i2s_instance;
> + else
> + rtd->i2s_instance = pinfo->cap_i2s_instance;
> + }

Looks like you need an error handling case here if pinfo is missing,
i2s_instance needs to be set. There are default cases but it's not
clear that that's a good idea, the intent of the code is clearly that
there's always platform data.
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2019-10-24 13:41    [W:0.175 / U:0.968 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site