On Sun, Sep 10, 2017 at 08:32:59AM +0530, Naveen M wrote:
> rt5663 needs mclk/sclk early to synchronize its internal clocks.
> Clocks are enabled in hw_params and disabled in platform_clk_control
> as the codec requires mclk/bclk need to turned ON before set_sysclk
> call for a successful clock synchronization.
This would be dependent on the series Subhransu posted right? If so please
do mention that
> +static int kabylake_enable_ssp_clks(struct snd_soc_card *card)
> +{
> +
we don't need the empty line here
> + struct kbl_codec_private *priv = snd_soc_card_get_drvdata(card);
> + int ret;
> +
> + /* Enable MCLK */
> + ret = clk_set_rate(priv->mclk, 24000000);
> + if (ret < 0) {
> + dev_err(card->dev, "Can't set rate for mclk, err: %d\n", ret);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(priv->mclk);
> + if (ret < 0) {
> + dev_err(card->dev, "Can't enable mclk, err: %d\n", ret);
> + return ret;
> + }
> +
> + /* Enable SCLK */
> + ret = clk_set_rate(priv->sclk, 3072000);
why do we need both mclk and sclk for this codec? One should be enough for
codec to sync
> + if (ret < 0) {
> + dev_err(card->dev, "Can't set rate for sclk, err: %d\n", ret);
> + clk_disable_unprepare(priv->mclk);
> + return ret;
> + }
> +
> + ret = clk_prepare_enable(priv->sclk);
> + if (ret < 0) {
> + dev_err(card->dev, "Can't enable sclk, err: %d\n", ret);
> + clk_disable_unprepare(priv->mclk);
> + }
> +
> + return 0;
this looks wrong to me, shouldn't this be return ret;
> @@ -647,6 +710,7 @@ static int kabylake_audio_probe(struct platform_device *pdev)
> {
> struct kbl_codec_private *ctx;
> struct skl_machine_pdata *pdata;
> + int ret = 0;
>
> ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
> if (!ctx)
> @@ -662,6 +726,34 @@ static int kabylake_audio_probe(struct platform_device *pdev)
> dmic_constraints = pdata->dmic_num == 2 ?
> &constraints_dmic_2ch : &constraints_dmic_channels;
>
> + ctx->mclk = devm_clk_get(&pdev->dev, "ssp1_mclk");
> + if (IS_ERR(ctx->mclk)) {
> + ret = PTR_ERR(ctx->mclk);
> + if (ret == -ENOENT) {
> + dev_info(&pdev->dev,
> + "Failed to get ssp1_mclk, defer probe\n");
can you check this, I think driver core emits warnings as well, so do we
need it here?
> + return -EPROBE_DEFER;
> + }
> +
> + dev_err(&pdev->dev, "Failed to get ssp1_mclk with err:%d\n",
> + ret);
> + return ret;
> + }
> +
> + ctx->sclk = devm_clk_get(&pdev->dev, "ssp1_sclk");
> + if (IS_ERR(ctx->sclk)) {
> + ret = PTR_ERR(ctx->sclk);
> + if (ret == -ENOENT) {
> + dev_info(&pdev->dev,
> + "Failed to get ssp1_sclk, defer probe\n");
shouldn't the mclk ref be given back?
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
