On Tue, Feb 25, 2025 at 11:42:43AM +0000, Caleb Connolly wrote:
[ . . . ]
> > +static int ipq9574_enable(struct clk *clk)
> > +{
> > + struct msm_clk_priv *priv = dev_get_priv(clk->dev);
> > +
> > + debug("%s: clk %s\n", __func__, ipq9574_clks[clk->id].name);
> > +
> > + switch (clk->id) {
> > + case GCC_BLSP1_UART3_APPS_CLK:
> > + qcom_gate_clk_en(priv, GCC_BLSP1_UART3_APPS_CLK);
> > + break;
> > + case GCC_BLSP1_AHB_CLK:
> > + qcom_gate_clk_en(priv, GCC_BLSP1_AHB_CLK);
> > + break;
> > + case GCC_SDCC1_AHB_CLK:
> > + qcom_gate_clk_en(priv, GCC_SDCC1_AHB_CLK);
> > + break;
> > + case GCC_SDCC1_APPS_CLK:
> > + qcom_gate_clk_en(priv, GCC_SDCC1_APPS_CLK);
> > + break;
> > + case GCC_SDCC1_ICE_CORE_CLK:
> > + qcom_gate_clk_en(priv, GCC_SDCC1_ICE_CORE_CLK);
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
>
> Last nitpick here, the reason I proposed using the gate_clk API is because
> it lets you drop this switch/case. Since you don't do any prep before
> enabling these clocks you can just call
>
> qcom_gate_clk_en(priv, clk->id);
>
> With that fixed
>
> Reviewed-by: Caleb Connolly <caleb.conno...@linaro.org>
>
> Thanks for sticking with me on this.
Thanks for the feedback. Have fixed this and posted v6. Could you kindly
review the 4th patch [1] of the series too.
-Varada
1 -
https://lore.kernel.org/u-boot/20250226064505.1178054-5-quic_var...@quicinc.com/
