Hi Jonathan,
On Sun, 2020-09-20 at 19:12 +0100, Jonathan McDowell wrote:
> Add the DMA engine driver for the QCOM Application Data Mover (ADM) DMA
> controller found in the MSM8x60 and IPQ/APQ8064 platforms.
>
> The ADM supports both memory to memory transactions and memory
> to/from peripheral device transactions. The controller also provides
> flow control capabilities for transactions to/from peripheral devices.
>
> The initial release of this driver supports slave transfers to/from
> peripherals and also incorporates CRCI (client rate control interface)
> flow control.
>
> Signed-off-by: Andy Gross <agross@xxxxxxxxxxxxxx>
> Signed-off-by: Thomas Pedersen <twp@xxxxxxxxxxxxxx>
> Signed-off-by: Jonathan McDowell <noodles@xxxxxxxx>
[...]
> +static int adm_dma_probe(struct platform_device *pdev)
> {
[...]
> + adev->core_clk = devm_clk_get(adev->dev, "core");
> + if (IS_ERR(adev->core_clk))
> + return PTR_ERR(adev->core_clk);
> +
> + ret = clk_prepare_enable(adev->core_clk);
> + if (ret) {
> + dev_err(adev->dev, "failed to prepare/enable core clock\n");
> + return ret;
> + }
It is better to only start enabling the hardware after all resources
have been acquired, see below.
> + adev->iface_clk = devm_clk_get(adev->dev, "iface");
> + if (IS_ERR(adev->iface_clk)) {
> + ret = PTR_ERR(adev->iface_clk);
> + goto err_disable_core_clk;
> + }
Move this up before the core_clk enable, and you can directly return the
error.
> +
> + ret = clk_prepare_enable(adev->iface_clk);
> + if (ret) {
> + dev_err(adev->dev, "failed to prepare/enable iface clock\n");
> + goto err_disable_core_clk;
> + }
> +
> + adev->clk_reset = devm_reset_control_get(&pdev->dev, "clk");
> + if (IS_ERR(adev->clk_reset)) {
> + dev_err(adev->dev, "failed to get ADM0 reset\n");
> + ret = PTR_ERR(adev->clk_reset);
> + goto err_disable_clks;
> + }
> +
> + adev->c0_reset = devm_reset_control_get(&pdev->dev, "c0");
> + if (IS_ERR(adev->c0_reset)) {
> + dev_err(adev->dev, "failed to get ADM0 C0 reset\n");
> + ret = PTR_ERR(adev->c0_reset);
> + goto err_disable_clks;
> + }
> +
> + adev->c1_reset = devm_reset_control_get(&pdev->dev, "c1");
> + if (IS_ERR(adev->c1_reset)) {
> + dev_err(adev->dev, "failed to get ADM0 C1 reset\n");
> + ret = PTR_ERR(adev->c1_reset);
> + goto err_disable_clks;
> + }
> +
> + adev->c2_reset = devm_reset_control_get(&pdev->dev, "c2");
> + if (IS_ERR(adev->c2_reset)) {
> + dev_err(adev->dev, "failed to get ADM0 C2 reset\n");
> + ret = PTR_ERR(adev->c2_reset);
> + goto err_disable_clks;
> + }
Please use devm_reset_control_get_exclusive(). Move these up before the
core_clk enable, and you can directly return the error.
regards
Philipp
