Messages in this thread | | | Subject | Re: [v8] clk: qcom: clk-rpmh: Add QCOM RPMh clock driver | From | Taniya Das <> | Date | Mon, 7 May 2018 16:18:06 +0530 |
| |
Hello Stephen,
Could you please let me know your comments on the below.
On 5/4/2018 10:21 PM, Stephen Boyd wrote: > Quoting Taniya Das (2018-05-04 03:02:38) >> diff --git a/drivers/clk/qcom/clk-rpmh.c b/drivers/clk/qcom/clk-rpmh.c >> new file mode 100644 >> index 0000000..944fe04 >> --- /dev/null >> +++ b/drivers/clk/qcom/clk-rpmh.c >> @@ -0,0 +1,334 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018, The Linux Foundation. All rights reserved. >> + */ >> + >> +}; >> + >> +struct clk_rpmh_desc { >> + struct clk_hw **clks; >> + size_t num_clks; >> +}; > > This could be replaced with the clk_hw_onecell_data struct and then the > only problem becomes the const part which seems pretty impossible to fix > at this point. One "workaround" is to memdup the structure. Ugh. >
Will be okay, if I can the following?
_probe... { struct clk_rpmh_desc *hw_desc_data; ....
hw_desc_data = kmemdup(desc, sizeof(*desc), GFP_KERNEL);
... ret = devm_of_clk_add_hw_provider(&pdev->dev, of_clk_rpmh_hw_get, hw_desc_data); ....
}
And also I fix the "getter" function.
>> + >> +static DEFINE_MUTEX(rpmh_clk_lock); >> + >> +#define __DEFINE_CLK_RPMH(_platform, _name, _name_active, _res_name, \ >> + _res_en_offset, _res_on, _div) \ >> + static struct clk_rpmh _platform##_##_name_active; \ >> + static struct clk_rpmh _platform##_##_name = { \ > [..] >> + >> +static unsigned long clk_rpmh_recalc_rate(struct clk_hw *hw, >> + unsigned long prate) >> +{ >> + struct clk_rpmh *r = to_clk_rpmh(hw); >> + unsigned long rate = prate; >> + >> + /* >> + * RPMh clocks have a fixed rate. Return static rate. >> + */ >> + do_div(rate, r->div); > > Integer division won't suffice? > >> + return rate; >> +} > [...] >> + >> +static struct clk_hw *of_clk_rpmh_hw_get(struct of_phandle_args *clkspec, >> + void *data) >> +{ >> + struct clk_hw *hw_clks = data; >> + unsigned int idx = clkspec->args[0]; >> + >> + if (idx < 0) { > > It can't be less than 0 though because it's unsigned. > >> + pr_err("%s: invalid index %u\n", __func__, idx); >> + return ERR_PTR(-EINVAL); >> + } >> + >> + return &hw_clks[idx]; >> +}
-- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation.
--
| |