[PATCH v3 02/10] clk/qcom: add initial clock driver for ipq5210

[Simon Glass]

Hi Varadarajan,

On 2026-04-16T05:39:18, Varadarajan Narayanan
<varadarajan.narayanan at oss.qualcomm.com> wrote:
> clk/qcom: add initial clock driver for ipq5210
>
> Add initial set of clocks and resets for enabling U-Boot on ipq5210
> based RDP platforms.
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan at oss.qualcomm.com>
>
> drivers/clk/qcom/Kconfig         |  8 ++++
>  drivers/clk/qcom/Makefile        |  1 +
>  drivers/clk/qcom/clock-ipq5210.c | 97 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+)

> +#include <dm/device-internal.h>

Is this needed?

> +static ulong ipq5210_set_rate(struct clk *clk, ulong rate)
> +{
> +     struct msm_clk_priv *priv = dev_get_priv(clk->dev);
> +
> +     switch (clk->id) {
> +     case GCC_QUPV3_WRAP_SE1_CLK:
> +             clk_rcg_set_rate_mnd(priv->base, priv->data->clks[clk->id].cbcr_reg,

cbcr_reg is a bit confusing since that name suggests a CBCR register -
see the ipq5424 driver which uses 'reg'. What do you think about using
reg consistently, or maybe add a comment explaining this?

> +static int ipq5210_enable(struct clk *clk)
> +{
> +     struct msm_clk_priv *priv = dev_get_priv(clk->dev);
> +
> +     if (priv->data->num_clks <= clk->id) {
> +             debug("%s: unknown clk id %lu\n", __func__, clk->id);
> +             return 0;
> +     }

Shouldn't the comparison be <  ?

Also, should return an error, not 0.

> +     qcom_gate_clk_en(priv, clk->id);
> +
> +     return 0;

Should return any error from qcom_gate_clk_en()

Regards,
Simon