Re: [PATCH 3/8] ASoC: samsung: i2s: Prevent external abort on exynos5433 I2S1 access

[Krzysztof Kozlowski]

On Mon, Feb 5, 2018 at 4:43 PM, Sylwester Nawrocki
<s.nawrocki@xxxxxxxxxxx> wrote:
> It seems both PCLK_I2S1 and SCLK_I2S1 clocks need to be enabled before
> I2S1 control registers can be accessed on exynos5433, if SCLK clock

Full stop before "if"?

> is disabled an exception is triggered. To fix this parent clock of
> the RCLK_SRC clock is assigned to pri_dai->op_clk so required gate
> clock is handled by the runtime PM ops.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> ---
>  sound/soc/samsung/i2s.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
> index b7d25a63da8b..1c05170e4999 100644
> --- a/sound/soc/samsung/i2s.c
> +++ b/sound/soc/samsung/i2s.c
> @@ -1328,6 +1328,7 @@ static int samsung_i2s_probe(struct platform_device *pdev)
>                 dev_err(&pdev->dev, "failed to enable clock: %d\n", ret);
>                 return ret;
>         }
> +

This should not be part of this patch. Beside that looks okay.

Best regards,
Krzysztof

>         pri_dai->dma_playback.addr = regs_base + I2STXD;
>         pri_dai->dma_capture.addr = regs_base + I2SRXD;
>         pri_dai->dma_playback.chan_name = "tx";
> @@ -1401,9 +1402,14 @@ static int samsung_i2s_probe(struct platform_device *pdev)
>         pm_runtime_enable(&pdev->dev);
>
>         ret = i2s_register_clock_provider(pdev);
> -       if (!ret)
> -               return 0;
> +       if (ret < 0)
> +               goto err_disable_pm;
> +
> +       pri_dai->op_clk = clk_get_parent(pri_dai->clk_table[CLK_I2S_RCLK_SRC]);
> +
> +       return 0;
>
> +err_disable_pm:
>         pm_runtime_disable(&pdev->dev);
>  err_disable_clk:
>         clk_disable_unprepare(pri_dai->clk);
> --
> 2.14.2
>