Quoting Rajendra Nayak (2018-12-03 21:21:15)
> @@ -221,6 +224,47 @@ static int rpmpd_power_off(struct generic_pm_domain *domain)
> return ret;
> }
>
> +static int rpmpd_set_performance(struct generic_pm_domain *domain,
> + unsigned int state)
> +{
> + int ret = 0;
> + struct rpmpd *pd = domain_to_rpmpd(domain);
> +
> + mutex_lock(&rpmpd_lock);
> +
> + if (state > MAX_RPMPD_STATE)
> + goto out;
Does this need to be under the mutex lock? Doesn't look like it.
> +
> + pd->corner = state;
> +
> + if (!pd->enabled && (pd->key != KEY_FLOOR_CORNER))
Please drop useless parenthesis.
> + goto out;
> +
> + ret = rpmpd_aggregate_corner(pd);
> +
> +out:
> + mutex_unlock(&rpmpd_lock);
> +
> + return ret;
> +}
> +
> +static unsigned int rpmpd_get_performance(struct generic_pm_domain *genpd,
> + struct dev_pm_opp *opp)
> +{
> + struct device_node *np;
> + unsigned int corner = 0;
> +
> + np = dev_pm_opp_get_of_node(opp);
> + if (of_property_read_u32(np, "qcom,level", &corner)) {
> + pr_err("%s: missing 'qcom,level' property\n", __func__);
We leak np reference here, assuming dev_pm_opp_get_of_node() returns an
of_node_get() pointer to the caller.
> + return 0;
> + }
> +
> + of_node_put(np);
This same code exists twice. Perhaps a helper needs to exist for
qcom_rpm_get_performance() to pull the number out of the DT.
> +
> + return corner;
> +}
