Re: [PATCH v4 4/4] clk: eyeq: add driver

[Christophe JAILLET]

Le 04/10/2024 à 17:45, Théo Lebrun a écrit :
> Add Mobileye EyeQ5, EyeQ6L and EyeQ6H clock controller driver. It is
> both a platform driver and a hook onto of_clk_init() used for clocks
> required early (GIC timer, UARTs).
>
> For some compatible, it is both at the same time. eqc_early_init()
> initialises early PLLs and stores clock array in a static linked list.
> It marks other clocks as deferred. eqc_probe() retrieves the clock
> array and adds all remaining clocks.
>
> It exposes read-only PLLs derived from the main crystal on board.
> It also exposes another type of clocks: divider clocks.
> They always have even divisors and have one PLL as parent.
>
> This driver also bears the responsability for optional reset and pinctrl
> auxiliary devices. The match data attached to the devicetree node
> compatible indicate if such devices should be created. They all get
> passed a pointer to the start of the OLB region.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---

Hi,

> +static void eqc_probe_init_plls(struct device *dev, struct eqc_priv *priv)
> +{
> +	const struct eqc_match_data *data = priv->data;
> +	unsigned long mult, div, acc;
> +	const struct eqc_pll *pll;
> +	struct clk_hw *hw;
> +	unsigned int i;
> +	u32 r0, r1;
> +	u64 val;
> +	int ret;
> +
> +	for (i = 0; i < data->pll_count; i++) {
> +		pll = &data->plls[i];
> +
> +		val = readq(priv->base + pll->reg64);
> +		r0 = val;
> +		r1 = val >> 32;
> +
> +		ret = eqc_pll_parse_registers(r0, r1, &mult, &div, &acc);
> +		if (ret) {
> +			dev_warn(dev, "failed parsing state of %s\n", pll->name);
> +			priv->cells->hws[pll->index] = ERR_PTR(ret);
> +			continue;
> +		}
> +
> +		hw = clk_hw_register_fixed_factor_with_accuracy_fwname(dev,
> +				dev->of_node, pll->name, "ref", 0, mult, div, acc);

Should this be freed somewhere or is it auto-magically freed by a 
put_something()?
Maybe devm_action_or_reset()?

> +		priv->cells->hws[pll->index] = hw;
> +		if (IS_ERR(hw))
> +			dev_warn(dev, "failed registering %s: %pe\n", pll->name, hw);
> +	}
> +}
> +
> +static void eqc_probe_init_divs(struct platform_device *pdev, struct device *dev,
> +				struct eqc_priv *priv)
> +{
> +	const struct eqc_match_data *data = priv->data;
> +	const struct eqc_div *div;
> +	struct clk_hw *parent;
> +	void __iomem *reg;
> +	struct clk_hw *hw;
> +	unsigned int i;
> +
> +	for (i = 0; i < data->div_count; i++) {
> +		div = &data->divs[i];
> +		reg = priv->base + div->reg;
> +		parent = priv->cells->hws[div->parent];
> +
> +		hw = clk_hw_register_divider_table_parent_hw(dev, div->name,
> +				parent, 0, reg, div->shift, div->width,
> +				CLK_DIVIDER_EVEN_INTEGERS, NULL, NULL);

Same.

CJ

> +		priv->cells->hws[div->index] = hw;
> +		if (IS_ERR(hw))
> +			dev_warn(dev, "failed registering %s: %pe\n",
> +				 div->name, hw);
> +	}
> +}