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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <75884d07-f052-435d-9f1a-44e9e0bb755f@wanadoo.fr>
Date: Fri, 4 Oct 2024 18:34:57 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Théo Lebrun <theo.lebrun@...tlin.com>,
 Michael Turquette <mturquette@...libre.com>, Stephen Boyd
 <sboyd@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>
Cc: devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
 linux-kernel@...r.kernel.org,
 Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>,
 Grégory Clement <gregory.clement@...tlin.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 Tawfik Bayouk <tawfik.bayouk@...ileye.com>
Subject: Re: [PATCH v4 4/4] clk: eyeq: add driver

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@...tlin.com>
> ---

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);
> +	}
> +}