About the title, please follow the old pattern:
ASoC: fsl_esai: Refine....
You can get the history from 'git log --oneline sound/soc/fsl/fsl_esai.c'
On Wed, Aug 06, 2014 at 06:23:03PM +0800, Shengjiu Wang wrote:
Add parameter for slots, and caculate the number of TX/RX pins and bclk with slots. Then driver will be compatible with slots > 2 in TDM mode.
Hmm...although I totally understand what the patch's exactly improving, I suggest to describe more so that other people can get a quick idea even without looking at the code, like what the original driver lacks or what's the problem of the original design, how this patch fulfills this feature and why some change have to be done, those change in the header file for example.
Signed-off-by: Shengjiu Wang shengjiu.wang@freescale.com
sound/soc/fsl/fsl_esai.c | 10 +++++++--- sound/soc/fsl/fsl_esai.h | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c index 72d154e..89aa531 100644 --- a/sound/soc/fsl/fsl_esai.c +++ b/sound/soc/fsl/fsl_esai.c @@ -56,6 +56,7 @@ struct fsl_esai { struct clk *fsysclk; u32 fifo_depth; u32 slot_width;
- u32 slots; u32 hck_rate[2]; u32 sck_rate[2]; bool hck_dir[2];
Please append a description for the new member to the existing comments above the structure define.
@@ -363,6 +364,7 @@ static int fsl_esai_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask, ESAI_xSMB_xS_MASK, ESAI_xSMB_xS(rx_mask));
esai_priv->slot_width = slot_width;
- esai_priv->slots = slots;
I prefer not to add indentation here, even thought this doesn't hurt. Just...If there's going to be a subsequent patch adding a new member assignment whose name's longer than slot_width, the style'll be broken as well. For example: esai_priv->slot_width = slot_width; esai_priv->slots = slots; esai_priv->slot_samplebit = slots;
The indentation for the 'slots' will look abnormal.
return 0; } @@ -510,10 +512,11 @@ static int fsl_esai_hw_params(struct snd_pcm_substream *substream, bool tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK; u32 width = snd_pcm_format_width(params_format(params)); u32 channels = params_channels(params);
- u32 pin = DIV_ROUND_UP(channels, esai_priv->slots);
This may cause a 'Division by zero' here. The 'slots' is only assigned in set_dai_tdm_slot(). As default, a machine driver who does not call set_dai_tdm_slot() will get a zero slot. We must give 'slots' a default value (2 for example) somewhere during initialization.
And another personal suggestion -- using name 'pins' might be better.
Thank you Shengjiu, Nicolin