Re: [PATCH] interconnect: Add bulk API helpers

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Thu 28 May 09:25 PDT 2020, Georgi Djakov wrote:
> diff --git a/drivers/interconnect/bulk.c b/drivers/interconnect/bulk.c
> new file mode 100644
> index 000000000000..9bd418594665
> --- /dev/null
> +++ b/drivers/interconnect/bulk.c
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/interconnect-provider.h>
> +#include <linux/device.h>
> +#include <linux/export.h>
> +
> +/**
> + * of_icc_bulk_get - get interconnect paths

Parenthesis on the function name here, and on the other functions below.

> + * @dev: the device requesting the path
> + * @num_paths: the number of icc_bulk_data
> + * @paths: the table with the paths we want to get
> + *
> + * Returns 0 on success or -EERROR otherwise.

s/-EERROR/negative errno/

> + */
> +int __must_check of_icc_bulk_get(struct device *dev, int num_paths,
> +				 struct icc_bulk_data *paths)
> +{
> +	int ret, i;
> +
> +	for (i = 0; i < num_paths; i++) {
> +		paths[i].path = of_icc_get(dev, paths[i].name);
> +		if (IS_ERR(paths[i].path)) {
> +			ret = PTR_ERR(paths[i].path);
> +			dev_err(dev, "of_icc_get() failed on path %s (%d)\n",
> +				paths[i].name, ret);

I think you only should print this error if ret != -EPROBE_DEFER.

> +			paths[i].path = NULL;
> +			goto err;
> +		}
> +	}
> +
> +	return 0;
> +
> +err:
> +	icc_bulk_put(i, paths);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(of_icc_bulk_get);
> +
> +/**
> + * icc_bulk_put - put a list of interconnect paths
> + * @num_paths: the number of icc_bulk_data
> + * @paths: the icc_bulk_data table with the paths being put
> + */
> +void icc_bulk_put(int num_paths, struct icc_bulk_data *paths)
> +{
> +	while (--num_paths >= 0) {
> +		icc_put(paths[num_paths].path);
> +		paths[num_paths].path = NULL;
> +	}
> +}
> +EXPORT_SYMBOL_GPL(icc_bulk_put);
> +
> +/**
> + * icc_bulk_set - set bandwidth to a set of paths
> + * @num_paths: the number of icc_bulk_data
> + * @paths: the icc_bulk_data table containing the paths and bandwidth
> + *
> + * Returns 0 on success or -EERROR otherwise.
> + */
> +int icc_bulk_set_bw(int num_paths, const struct icc_bulk_data *paths)
> +{
> +	int ret = 0;
> +	int i;
> +
> +	for (i = 0; i < num_paths; i++) {
> +		ret = icc_set_bw(paths[i].path, paths[i].avg_bw,
> +				 paths[i].peak_bw);

You can unwrap this line.

Regards,
Bjorn