linux-kernel - Re: [PATCH v7 07/10] i2c: of-prober: Add simple helpers for regulator support

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: <ZuQYAu9tBN0wrNfo@smile.fi.intel.com>
Date: Fri, 13 Sep 2024 13:46:26 +0300
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: Rob Herring <robh@...nel.org>, Saravana Kannan <saravanak@...gle.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
	Wolfram Sang <wsa@...nel.org>, Benson Leung <bleung@...omium.org>,
	Tzung-Bi Shih <tzungbi@...nel.org>, Mark Brown <broonie@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	chrome-platform@...ts.linux.dev, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Douglas Anderson <dianders@...omium.org>,
	Johan Hovold <johan@...nel.org>, Jiri Kosina <jikos@...nel.org>,
	linux-i2c@...r.kernel.org
Subject: Re: [PATCH v7 07/10] i2c: of-prober: Add simple helpers for
 regulator support

On Wed, Sep 11, 2024 at 03:27:45PM +0800, Chen-Yu Tsai wrote:
> Add helpers to do regulator management for the I2C OF component prober.
> Components that the prober intends to probe likely require their
> regulator supplies be enabled, and GPIOs be toggled to enable them or
> bring them out of reset before they will respond to probe attempts.
> GPIOs will be handled in the next patch.
> 
> The assumption is that the same class of components to be probed are
> always connected in the same fashion with the same regulator supply
> and GPIO. The names may vary due to binding differences, but the
> physical layout does not change.
> 
> This set of helpers supports at most one regulator supply. The user
> must specify the node from which the supply is retrieved. The supply
> name and the amount of time to wait after the supply is enabled are
> also given by the user.

...

> +static int i2c_of_probe_simple_get_supply(struct device *dev, struct device_node *node,
> +					  struct i2c_of_probe_simple_ctx *ctx)
> +{
> +	const char *supply_name;
> +	struct regulator *supply;
> +
> +	/*
> +	 * It's entirely possible for the component's device node to not have regulator
> +	 * supplies. While it does not make sense from a hardware perspective, the
> +	 * supplies could be always on or otherwise not modeled in the device tree, but
> +	 * the device would still work.
> +	 */

I would reformat as

	/*
	 * It's entirely possible for the component's device node to not have the
	 * regulator supplies. While it does not make sense from a hardware perspective,
	 * the supplies could be always on or otherwise not modeled in the device tree,
	 * but the device would still work.
	 */

> +	supply_name = ctx->opts->supply_name;
> +	if (!supply_name)
> +		return 0;
> +
> +	supply = of_regulator_get_optional(dev, node, supply_name);
> +	if (IS_ERR(supply)) {
> +		return dev_err_probe(dev, PTR_ERR(supply),
> +				     "Failed to get regulator supply \"%s\" from %pOF\n",
> +				     supply_name, node);
> +	}
> +
> +	ctx->supply = supply;
> +
> +	return 0;
> +}

...

> +int i2c_of_probe_simple_get_res(struct device *dev, struct device_node *bus_node, void *data)
> +{
> +	struct i2c_of_probe_simple_ctx *ctx = data;
> +	struct device_node *node;
> +	const char *compat;
> +	int ret;
> +
> +	dev_dbg(dev, "Requesting resources for components under I2C bus %pOF\n", bus_node);
> +
> +	if (!ctx || !ctx->opts)
> +		return -EINVAL;
> +
> +	compat = ctx->opts->res_node_compatible;
> +	if (!compat)
> +		return -EINVAL;

> +	node = of_get_compatible_child(bus_node, compat);

	__free(of_node_put) ?

> +	if (!node)
> +		return dev_err_probe(dev, -ENODEV, "No device compatible with \"%s\" found\n",
> +				     compat);
> +
> +	ret = i2c_of_probe_simple_get_supply(dev, node, ctx);
> +	if (ret)
> +		goto out_put_node;
> +
> +	return 0;
> +
> +out_put_node:
> +	of_node_put(node);
> +	return ret;
> +}

...

> + * @post_power_on_delay_ms: Delay in ms after regulators are powered on. Passed to msleep().

No need to duplicate the units as it's obvious from the variable name and msleep().

 * @post_power_on_delay_ms: Delay after regulators are powered on. Passed to msleep().

-- 
With Best Regards,
Andy Shevchenko