Re: [PATCH 04/17] PM / OPP: Introduce dev_pm_opp_get_max_volt_latency()

[Stephen Boyd <sboyd@codeaurora.org>]

On 12/22, Viresh Kumar wrote:
> @@ -230,6 +230,55 @@ unsigned long dev_pm_opp_get_max_clock_latency(struct device *dev)
>  EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_clock_latency);
>  
>  /**
> + * dev_pm_opp_get_max_volt_latency() - Get max voltage latency in nanoseconds
> + * @dev:	device for which we do this operation

Weird tab again.

> + *
> + * Return: This function returns the max voltage latency in nanoseconds.
> + *
> + * Locking: This function takes rcu_read_lock().
> + */
> +unsigned long dev_pm_opp_get_max_volt_latency(struct device *dev)
> +{
> +	struct device_opp *dev_opp;
> +	struct dev_pm_opp *opp;
> +	struct regulator *reg;
> +	unsigned long latency_ns = 0;
> +	unsigned long min_uV = ~0, max_uV = 0;
> +	int ret;
> +
> +	rcu_read_lock();
> +
> +	dev_opp = _find_device_opp(dev);
> +	if (IS_ERR(dev_opp))
> +		goto unlock;
> +
> +	reg = dev_opp->regulator;
> +	/* Regulator may not be available for device */
> +	if (IS_ERR(reg))
> +		goto unlock;
> +
> +	list_for_each_entry_rcu(opp, &dev_opp->opp_list, node) {
> +		if (!opp->available)
> +			continue;
> +
> +		if (opp->u_volt_min < min_uV)
> +			min_uV = opp->u_volt_min;
> +		if (opp->u_volt_max > max_uV)
> +			max_uV = opp->u_volt_max;
> +	}
> +
> +	ret = regulator_set_voltage_time(reg, min_uV, max_uV);

Doesn't rcu_read_lock() disable preemption sometimes? I don't
think we can call regulator ops with preemption disabled because
regulators use mutex locking. It looks like
regulator_list_voltage() may take a mutex anyway.

> +	if (ret > 0)
> +		latency_ns += ret * 1000;

Why add to 0? Perhaps this should just be an assignment?

> +
> +unlock:
> +	rcu_read_unlock();
> +
> +	return latency_ns;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_get_max_volt_latency);
> 

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project