Just some small comments. On Wed, Apr 01, 2020 at 04:45:37PM +0800, Shengjiu Wang wrote: > In order to align with new ESARC, we add new property fsl,asrc-format. > The fsl,asrc-format can replace the fsl,asrc-width, driver > can accept format from devicetree, don't need to convert it to > format through width. > > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx> > --- > sound/soc/fsl/fsl_asrc.c | 40 ++++++++++++++++++++++-------------- > sound/soc/fsl/fsl_asrc.h | 4 ++-- > sound/soc/fsl/fsl_asrc_dma.c | 15 +++++++++++--- > 3 files changed, 39 insertions(+), 20 deletions(-) > > diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c > index 4d3e51bfa949..eea19e2b723b 100644 > --- a/sound/soc/fsl/fsl_asrc.c > +++ b/sound/soc/fsl/fsl_asrc.c > @@ -1052,16 +1047,31 @@ static int fsl_asrc_probe(struct platform_device *pdev) > return ret; > } > > - ret = of_property_read_u32(np, "fsl,asrc-width", > - &asrc->asrc_width); > + ret = of_property_read_u32(np, "fsl,asrc-format", &asrc->asrc_format); > if (ret) { > - dev_err(&pdev->dev, "failed to get output width\n"); > - return ret; > + ret = of_property_read_u32(np, "fsl,asrc-width", &width); > + if (ret) { > + dev_err(&pdev->dev, "failed to get output width\n"); Similar to the comments against sound card driver: "failed to decide output format" > + return ret; > + } > + > + switch (width) { > + case 16: > + asrc->asrc_format = SNDRV_PCM_FORMAT_S16_LE; > + break; > + case 24: > + asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE; > + break; > + default: > + dev_warn(&pdev->dev, "unsupported width, switching to 24bit\n"); Should match what the code does after the change: + dev_warn(&pdev->dev, + "unsupported width, use default S24_LE\n"); > + asrc->asrc_format = SNDRV_PCM_FORMAT_S24_LE; > + break; > + } > } > > - if (asrc->asrc_width != 16 && asrc->asrc_width != 24) { > - dev_warn(&pdev->dev, "unsupported width, switching to 24bit\n"); > - asrc->asrc_width = 24; > + if (!(FSL_ASRC_FORMATS & (1ULL << asrc->asrc_format))) { > + dev_warn(&pdev->dev, "unsupported format, switching to S24_LE\n"); Could fit 80 characters: + dev_warn(&pdev->dev, "unsupported width, use default S24_LE\n"); > diff --git a/sound/soc/fsl/fsl_asrc_dma.c b/sound/soc/fsl/fsl_asrc_dma.c > index 5fe83aece25b..b15946e03380 100644 > --- a/sound/soc/fsl/fsl_asrc_dma.c > +++ b/sound/soc/fsl/fsl_asrc_dma.c > @@ -230,10 +230,19 @@ static int fsl_asrc_dma_hw_params(struct snd_soc_component *component, > return -EINVAL; > } > > - if (asrc->asrc_width == 16) > + bits = snd_pcm_format_physical_width(asrc->asrc_format); Can we just use 'width' to match the function name?