linux-kernel - Re: [RFC] reset: add reset_control

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] [day] [month] [year] [list]

Message-ID: <b1704d04-14bf-9950-5bae-d97e357d6832@gmail.com>
Date:   Tue, 9 Mar 2021 20:05:00 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Philipp Zabel <p.zabel@...gutronix.de>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC] reset: add reset_control_bulk API

03.03.2021 15:10, Philipp Zabel пишет:
> Follow the clock and regulator subsystems' lead and add a bulk API
> for reset controls.
> 
> Signed-off-by: Philipp Zabel <p.zabel@...gutronix.de>
> ---

Hello Philipp,

The patch is almost okay to me. I'll fix the minor issues and include it
in the next version of the Tegra audio patches. Thanks!

> +/**
> + * reset_control_bulk_deassert - asserts the reset lines in order

I'll correct this comment

> + * @num_rstcs: number of entries in rstcs array
> + * @rstcs: array of struct reset_control_bulk_data with reset controls set
> + *
> + * Deassert the reset lines for all provided reset controls, in order.
> + * If a deassertion fails, already deasserted resets are asserted again.
> + *
> + * See also: reset_control_assert()
> + */
> +int reset_control_bulk_deassert(int num_rstcs,
> +				struct reset_control_bulk_data *rstcs)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < num_rstcs; i++) {
> +		ret = reset_control_deassert(rstcs[i].rstc);
> +		if (ret)
> +			goto err;
> +	}
> +
> +	return 0;
> +
> +err:
> +	while (i--)
> +		reset_control_assert(rstcs[i].rstc);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(reset_control_bulk_deassert);

and change the bulk_deassert to use the reverse order

...
> +int __reset_control_bulk_get(struct device *dev, int num_rstcs,
> +			     struct reset_control_bulk_data *rstcs,
> +			     bool shared, bool optional, bool acquired)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < num_rstcs; i++) {
> +		rstcs[i].rstc = __reset_control_get(dev, rstcs[i].id, 0,
> +						    shared, optional, acquired);
> +		if (IS_ERR(rstcs[i].rstc)) {
> +			ret = PTR_ERR(rstcs[i].rstc);
> +			goto err;
> +		}
> +	}
> +

and add the missing "return 0" here.

> +err:
> +	mutex_lock(&reset_list_mutex);
> +	while (i--)
> +		__reset_control_put_internal(rstcs[i].rstc);
> +	mutex_unlock(&reset_list_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(__reset_control_bulk_get);
...