Re: [PATCH v3 3/3] clk: ultrarisc: Add DP1000 clock driver

[Brian Masney]

Hi Jia,

I have one minor question below.

On Tue, Jul 14, 2026 at 09:35:09AM +0800, Jia Wang via B4 Relay wrote:
> From: Jia Wang <wangjia@xxxxxxxxxxxxx>
> 
> Add a clock driver for the UltraRISC DP1000 SoC.
> 
> The clock tree is driven by a SYSPLL and provides fixed-factor clocks for
> the subsystem and PCIe, divider-based root clocks for GMAC and the UART,
> I2C, and SPI blocks, and per-instance gate clocks for UART0-3, I2C0-3,
> and SPI0-1.
> 
> Signed-off-by: Jia Wang <wangjia@xxxxxxxxxxxxx>
> ---
>  MAINTAINERS                           |   1 +
>  drivers/clk/Kconfig                   |   1 +
>  drivers/clk/Makefile                  |   1 +
>  drivers/clk/ultrarisc/Kconfig         |  18 ++
>  drivers/clk/ultrarisc/Makefile        |   4 +
>  drivers/clk/ultrarisc/clk-dp1000.c    | 154 +++++++++++++
>  drivers/clk/ultrarisc/clk-ultrarisc.c | 407 ++++++++++++++++++++++++++++++++++
>  drivers/clk/ultrarisc/clk-ultrarisc.h |  71 ++++++
>  8 files changed, 657 insertions(+)

[snip]

> 
> +static int ultrarisc_clk_register_dividers(struct platform_device *pdev,
> +					   struct clk_hw_onecell_data *clk_data,
> +					   const struct ultrarisc_clk_soc_data *soc_data,
> +					   void __iomem *base,
> +					   spinlock_t *lock)
> +{
> +	struct device *dev = &pdev->dev;
> +	u32 i;
> +
> +	for (i = 0; i < soc_data->num_dividers; i++) {
> +		const struct ultrarisc_divider_desc *desc;
> +		struct clk_hw *parent_hw;
> +		struct clk_hw *hw;
> +
> +		desc = &soc_data->dividers[i];
> +		if (desc->id >= clk_data->num || desc->parent_id >= clk_data->num)
> +			return -EINVAL;
> +
> +		parent_hw = clk_data->hws[desc->parent_id];
> +		if (!parent_hw)
> +			return -EINVAL;
> +
> +		hw = ultrarisc_clk_register_divider(dev, desc, parent_hw, base,
> +						    lock);
> +		if (IS_ERR(hw))
> +			return PTR_ERR(hw);
> +
> +		if (desc->max_rate) {
> +			unsigned long rate;
> +
> +			clk_hw_set_rate_range(hw, 0, desc->max_rate);
> +
> +			rate = clk_hw_get_rate(hw);
> +			if (rate > desc->max_rate)
> +				dev_warn(dev, "%s rate %lu exceeds max %lu\n",
> +					 desc->name, rate, desc->max_rate);

At this point in the probe, the clock tree may not be fully propagated
yet. Is this check needed?

Everything else in the driver looks fine to me.

Brian