Looks good to me, yet two small comments inline. Please add this to this patch in the next version: Acked-by: Nicolin Chen <nicoleotsuka@xxxxxxxxx> On Wed, Jul 03, 2019 at 02:42:04PM +0800, shengjiu.wang@xxxxxxx wrote: > +static int fsl_esai_register_restore(struct fsl_esai *esai_priv) > +{ > + int ret; > + /* FIFO reset for safety */ > + regmap_update_bits(esai_priv->regmap, REG_ESAI_TFCR, Checkpatch script would probably warn this. Usually we add a blank line after variable declarations. > @@ -866,22 +935,9 @@ static int fsl_esai_probe(struct platform_device *pdev) > > dev_set_drvdata(&pdev->dev, esai_priv); > > - /* Reset ESAI unit */ > - ret = regmap_write(esai_priv->regmap, REG_ESAI_ECR, ESAI_ECR_ERST); > - if (ret) { > - dev_err(&pdev->dev, "failed to reset ESAI: %d\n", ret); > + ret = fsl_esai_init(esai_priv); Could we rename this function to fsl_easi_hw_init() or something clear like fsl_esai_register_init? fsl_easi_init() feels like a driver init() function to me. Thank you Nicolin _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel