On 10/20, Georgi Djakov wrote:
> diff --git a/drivers/clk/qcom/clk-smd-rpm.c b/drivers/clk/qcom/clk-smd-rpm.c
> new file mode 100644
> index 000000000000..aa634bdf0aae
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-smd-rpm.c
> @@ -0,0 +1,260 @@
> +
> +static int clk_smd_rpm_set_rate_active(struct clk_smd_rpm *r,
> + unsigned long value)
> +{
> + struct clk_smd_rpm_req req = {
> + .key = r->rpm_key,
> + .nbytes = sizeof(u32),
> + .value = DIV_ROUND_UP(value, 1000), /* RPM expects kHz */
> + };
> +
> + return qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_ACTIVE_STATE,
> + r->rpm_res_type, r->rpm_clk_id, &req,
> + sizeof(req));
> +}
> +
> +static int clk_smd_rpm_set_rate_sleep(struct clk_smd_rpm *r,
> + unsigned long value)
> +{
> + struct clk_smd_rpm_req req = {
> + .key = r->rpm_key,
> + .nbytes = sizeof(u32),
> + .value = DIV_ROUND_UP(value, 1000), /* RPM expects kHz */
Don't we need to do all the cpu_to_le32() stuff on these
structures?
> + };
> +
> + return qcom_rpm_smd_write(r->rpm, QCOM_SMD_RPM_SLEEP_STATE,
> + r->rpm_res_type, r->rpm_clk_id, &req,
> + sizeof(req));
> +}
> +
[..]
> +
> +static int clk_smd_rpm_prepare(struct clk_hw *hw)
> +{
> + struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> + struct clk_smd_rpm *peer = r->peer;
> + unsigned long this_rate = 0, this_sleep_rate = 0;
> + unsigned long peer_rate = 0, peer_sleep_rate = 0;
> + unsigned long active_rate, sleep_rate;
> + int ret = 0;
> +
> + mutex_lock(&rpm_clk_lock);
> +
> + /* Don't send requests to the RPM if the rate has not been set. */
> + if (!r->rate)
> + goto out;
> +
> + to_active_sleep(r, r->rate, &this_rate, &this_sleep_rate);
> +
> + /* Take peer clock's rate into account only if it's enabled. */
> + if (peer->enabled)
> + to_active_sleep(peer, peer->rate,
> + &peer_rate, &peer_sleep_rate);
> +
> + active_rate = max(this_rate, peer_rate);
> +
> + if (r->branch)
> + active_rate = !!active_rate;
> +
> + ret = clk_smd_rpm_set_rate_active(r, active_rate);
> + if (ret)
> + goto out;
> +
> + sleep_rate = max(this_sleep_rate, peer_sleep_rate);
> + if (r->branch)
> + sleep_rate = !!sleep_rate;
> +
> + ret = clk_smd_rpm_set_rate_sleep(r, sleep_rate);
> + if (ret)
> + /* Undo the active set vote and restore it */
> + ret = clk_smd_rpm_set_rate_active(r, peer_rate);
> +
> +out:
> + if (!ret)
> + r->enabled = true;
> +
> + mutex_unlock(&rpm_clk_lock);
> +
> + return ret;
> +}
> +
> +static void clk_smd_rpm_unprepare(struct clk_hw *hw)
> +{
> + struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> +
> + mutex_lock(&rpm_clk_lock);
> +
> + if (r->rate) {
The style is different than the prepare path here. Why? Please
use if (!r->rate) instead and move the local variables below to
the top of the function.
> + struct clk_smd_rpm *peer = r->peer;
> + unsigned long peer_rate = 0, peer_sleep_rate = 0;
> + unsigned long active_rate, sleep_rate;
> + int ret;
> +
> + /* Take peer clock's rate into account only if it's enabled. */
> + if (peer->enabled)
> + to_active_sleep(peer, peer->rate, &peer_rate,
> + &peer_sleep_rate);
> +
> + active_rate = r->branch ? !!peer_rate : peer_rate;
> + ret = clk_smd_rpm_set_rate_active(r, active_rate);
> + if (ret)
> + goto out;
> +
> + sleep_rate = r->branch ? !!peer_sleep_rate : peer_sleep_rate;
> + ret = clk_smd_rpm_set_rate_sleep(r, sleep_rate);
> + if (ret)
> + goto out;
> + }
> + r->enabled = false;
> +
> +out:
> + mutex_unlock(&rpm_clk_lock);
> +}
> +
> +static int clk_smd_rpm_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_smd_rpm *r = to_clk_smd_rpm(hw);
> + int ret = 0;
> +
> + mutex_lock(&rpm_clk_lock);
> +
> + if (r->enabled) {
Same comment here. De-indent the code below.
> + struct clk_smd_rpm *peer = r->peer;
> + unsigned long active_rate, sleep_rate;
> + unsigned long this_rate = 0, this_sleep_rate = 0;
> + unsigned long peer_rate = 0, peer_sleep_rate = 0;
> +
> + to_active_sleep(r, rate, &this_rate, &this_sleep_rate);
> +
> + /* Take peer clock's rate into account only if it's enabled. */
> + if (peer->enabled)
> + to_active_sleep(peer, peer->rate,
> + &peer_rate, &peer_sleep_rate);
> +
> + active_rate = max(this_rate, peer_rate);
> + ret = clk_smd_rpm_set_rate_active(r, active_rate);
> + if (ret)
> + goto out;
> +
> + sleep_rate = max(this_sleep_rate, peer_sleep_rate);
> + ret = clk_smd_rpm_set_rate_sleep(r, sleep_rate);
> + if (ret)
> + goto out;
> + }
> + r->rate = rate;
> +out:
> + mutex_unlock(&rpm_clk_lock);
> +
> + return ret;
> +}
> +
> +
> diff --git a/drivers/clk/qcom/clk-smd-rpm.h b/drivers/clk/qcom/clk-smd-rpm.h
> new file mode 100644
> index 000000000000..d5dafad6b0fc
> --- /dev/null
> +++ b/drivers/clk/qcom/clk-smd-rpm.h
> @@ -0,0 +1,142 @@
> +
> +#define __DEFINE_CLK_SMD_RPM(_name, active, type, r_id, stat_id, dep, key) \
> + static struct clk_smd_rpm active; \
Can you please align the '\' at the same column on the right
side?
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
