On 05/27, Sudeep Holla wrote:
> +
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/of.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/scpi_protocol.h>
> +
> +struct scpi_clk {
> + u32 id;
> + const char *name;
> + struct clk_hw hw;
> + struct scpi_dvfs_info *info;
> + unsigned long rate_min;
> + unsigned long rate_max;
> +};
> +
> +#define to_scpi_clk(clk) container_of(clk, struct scpi_clk, hw)
> +
> +static struct scpi_ops *scpi_ops;
Why do we need this singleton? Can we put this pointer into scpi_clk?
> +
> +static unsigned long scpi_clk_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct scpi_clk *clk = to_scpi_clk(hw);
> +
> + return scpi_ops->clk_get_val(clk->id);
> +}
> +
> +static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct scpi_clk *clk = to_scpi_clk(hw);
> +
> + if (WARN_ON(clk->rate_min && rate < clk->rate_min))
> + rate = clk->rate_min;
> + if (WARN_ON(clk->rate_max && rate > clk->rate_max))
> + rate = clk->rate_max;
> +
> + return rate;
> +}
Hm.. this seems really generic. It might be better to support a
way to tell the framework to limit the min/max rate that's
accepted for a clk. That could be done later though.
> +
> +static int scpi_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct scpi_clk *clk = to_scpi_clk(hw);
> +
> + return scpi_ops->clk_set_val(clk->id, rate);
> +}
> +
[..]
> +
> +static int scpi_clk_add(struct device *dev, struct device_node *np)
> +{
> + struct clk **clks;
> + int idx, count;
> + struct clk_onecell_data *clk_data;
> +
> + count = of_property_count_strings(np, "clock-output-names");
> + if (count < 0) {
> + dev_err(dev, "%s: invalid clock output count\n", np->name);
> + return -EINVAL;
> + }
> +
> + clk_data = devm_kmalloc(dev, sizeof(*clk_data), GFP_KERNEL);
> + if (!clk_data)
> + return -ENOMEM;
> +
> + clks = devm_kcalloc(dev, count, sizeof(*clks), GFP_KERNEL);
> + if (!clks)
> + return -ENOMEM;
> +
> + for (idx = 0; idx < count; idx++) {
> + struct scpi_clk *sclk;
> + u32 val;
> +
> + sclk = devm_kzalloc(dev, sizeof(*sclk), GFP_KERNEL);
> + if (!sclk)
> + return -ENOMEM;
> +
> + if (of_property_read_string_index(np, "clock-output-names",
> + idx, &sclk->name)) {
> + dev_err(dev, "invalid clock name @ %s\n", np->name);
> + return -EINVAL;
> + }
> +
> + if (of_property_read_u32_index(np, "clock-indices",
> + idx, &val)) {
> + dev_err(dev, "invalid clock index @ %s\n", np->name);
> + return -EINVAL;
> + }
> +
> + sclk->id = val;
> +
> + clks[idx] = scpi_clk_ops_init(dev, np, sclk);
> + if (IS_ERR_OR_NULL(clks[idx]))
> + dev_err(dev, "failed to register clock '%s'\n",
> + sclk->name);
> + else
> + dev_dbg(dev, "Registered clock '%s'\n", sclk->name);
> + }
> +
> + clk_data->clks = clks;
> + clk_data->clk_num = idx;
> + of_clk_add_provider(np, of_clk_src_onecell_get, clk_data);
And if of_clk_add_provider() fails?
--
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-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
