[PATCH v2 4/8] imx8mp: power-domain: Expose high performance PLL clock

[Marek Vasut]

On 2/26/24 9:04 AM, Sumit Garg wrote:
> Expose the high performance PLL as a regular Linux clock, so the
> PCIe PHY can use it when there is no external refclock provided.
> 
> Inspired from counterpart Linux kernel v6.8-rc3 driver:
> drivers/pmdomain/imx/imx8mp-blk-ctrl.c

Commit ID, please, see previous comments in this series.

[...]

> +static int hsio_pll_clk_enable(struct clk *clk)
> +{
> +	void *base = (void *)dev_get_driver_data(clk->dev);
> +	u32 val;
> +	int ret;
> +
> +	/* Setup HSIO PLL */
> +	clrsetbits_le32(base + GPR_REG2,
> +			P_PLL_MASK | M_PLL_MASK | S_PLL_MASK,
> +			FIELD_PREP(P_PLL_MASK, 12) |
> +			FIELD_PREP(M_PLL_MASK, 800) |
> +			FIELD_PREP(S_PLL_MASK, 4));

These magic numbers 12, 800, 4 could use explanation (why these 
numbers?) and a dedicated macro .

> +	/* de-assert PLL reset */
> +	setbits_le32(base + GPR_REG3, PLL_RST);
> +
> +	/* enable PLL */
> +	setbits_le32(base + GPR_REG3, PLL_CKE);
> +
> +	/* Check if PLL is locked */
> +	ret = readl_poll_sleep_timeout(base + GPR_REG1, val,
> +				       val & PLL_LOCK, 10, 100000);
> +	if (ret)
> +		dev_err(clk->dev, "failed to lock HSIO PLL\n");
> +
> +	return ret;
> +}
> +
> +static int hsio_pll_clk_disable(struct clk *clk)
> +{
> +	void *base = (void *)dev_get_driver_data(clk->dev);
> +
> +	clrbits_le32(base + GPR_REG3, PLL_CKE);
> +	clrbits_le32(base + GPR_REG3, PLL_RST);

Can you clear both at once, or do they have to be cleared in sequence ?

[...]

The series is starting to look much better compared to V1 btw , these ^ 
are only nitpicks .