Re: [PATCH v2] dmaengine: qcom: bam_dma: allow omitting num-{channels,ees}

[Bhupesh Sharma]

On Fri, 19 May 2023 at 16:30, Stephan Gerhold <stephan@xxxxxxxxxxx> wrote:
>
> The bam_dma driver needs to know the number of channels and execution
> environments (EEs) at probe time. If we are in full control of the BAM
> controller this information can be obtained from the BAM identification
> registers (BAM_REVISION/BAM_NUM_PIPES).
>
> When the BAM is "controlled remotely" it is more complicated. The BAM
> might not be on at probe time, so reading the registers could fail.
> This is why the information must be added to the device tree in this
> case, using "num-channels" and "qcom,num-ees".
>
> However, there are also some BAM instances that are initialized by
> something else but we still have a clock that allows to turn it on when
> needed. This can be set up in the DT with "qcom,controlled-remotely"
> and "clocks" and is already supported by the bam_dma driver. Examples
> for this are the typical BLSP BAM instances on older SoCs, QPIC BAM
> (for NAND) and the crypto BAM on some SoCs.
>
> In this case, there is no need to read "num-channels" and
> "qcom,num-ees" from the DT. The BAN can be turned on using the clock
> so we can just read it from the BAM registers like in the normal case.
>
> Check for the BAM clock earlier and skip reading "num-channels" and
> "qcom,num-ees" if it is present to allow simplifying the DT description
> a bit.
>
> Signed-off-by: Stephan Gerhold <stephan@xxxxxxxxxxx>
> ---
> Changes in v2:
> - Rewrite commit message for more clarity (discussion with Bhupesh)
> - Link to v1: https://lore.kernel.org/r/20230518-bamclk-dt-v1-1-82f738c897d9@xxxxxxxxxxx
> ---
>  drivers/dma/qcom/bam_dma.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 1e47d27e1f81..4c3eb972039d 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1272,7 +1272,15 @@ static int bam_dma_probe(struct platform_device *pdev)
>         bdev->powered_remotely = of_property_read_bool(pdev->dev.of_node,
>                                                 "qcom,powered-remotely");
>
> -       if (bdev->controlled_remotely || bdev->powered_remotely) {
> +       if (bdev->controlled_remotely || bdev->powered_remotely)
> +               bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
> +       else
> +               bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> +
> +       if (IS_ERR(bdev->bamclk))
> +               return PTR_ERR(bdev->bamclk);
> +
> +       if (!bdev->bamclk) {
>                 ret = of_property_read_u32(pdev->dev.of_node, "num-channels",
>                                            &bdev->num_channels);
>                 if (ret)
> @@ -1284,14 +1292,6 @@ static int bam_dma_probe(struct platform_device *pdev)
>                         dev_err(bdev->dev, "num-ees unspecified in dt\n");
>         }
>
> -       if (bdev->controlled_remotely || bdev->powered_remotely)
> -               bdev->bamclk = devm_clk_get_optional(bdev->dev, "bam_clk");
> -       else
> -               bdev->bamclk = devm_clk_get(bdev->dev, "bam_clk");
> -
> -       if (IS_ERR(bdev->bamclk))
> -               return PTR_ERR(bdev->bamclk);
> -
>         ret = clk_prepare_enable(bdev->bamclk);
>         if (ret) {
>                 dev_err(bdev->dev, "failed to prepare/enable clock\n");

Reviewed-by: Bhupesh Sharma <bhupesh.sharma@xxxxxxxxxx>

Thanks.