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: <8334a319-96e4-4249-9659-132c8698c895@wanadoo.fr>
Date: Fri, 4 Oct 2024 23:20:31 +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 à 18:55, Théo Lebrun a écrit :
> Hello Christophe,
> 
> On Fri Oct 4, 2024 at 6:34 PM CEST, Christophe JAILLET wrote:
>> Le 04/10/2024 à 17:45, Théo Lebrun a écrit :
>>> +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()?
> 
> This driver does not support being removed. It provides essential PLLs
> and the system has not chance of working without them.
> 
> Almost all instances will be instantiated at of_clk_init() stage by the
> way (ie before platform bus infrastructure init). Devres isn't a
> solution in those cases.

eqc_probe_init_plls() and eqc_probe_init_divs() are called from 
eqc_probe(), which has several devm_ function calls.

Would it make sense to remove these devm_ ?


devm_platform_ioremap_resource(),
devm_kzalloc(),
devm_of_clk_add_hw_provider(),
eqc_auxdev_create() which calls devm_add_action_or_reset().

I sent this patch because of these calls.

Either I miss something, either maybe things can be simplified.

CJ


> 
> We are missing suppress_bind_attrs though.
> I can add that at next revision.
> 
> Thanks,
> 
> --
> Théo Lebrun, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> 
> 
>