On Mon, Apr 27, 2015 at 12:40:44PM +0100, Sudeep Holla wrote: > On some ARM based systems, a separate Cortex-M based System Control > Processor(SCP) provides the overall power, clock, reset and system > control. System Control and Power Interface(SCPI) Message Protocol > is defined for the communication between the Application Cores(AP) > and the SCP. > > This patch adds support for the clocks provided by SCP using SCPI > protocol. > > Signed-off-by: Sudeep Holla <[email protected]> > Cc: Mike Turquette <[email protected]> > Cc: Stephen Boyd <[email protected]> > Cc: Liviu Dudau <[email protected]> > Cc: Lorenzo Pieralisi <[email protected]> > Cc: Jon Medhurst (Tixy) <[email protected]> > Cc: [email protected] > --- > drivers/clk/Kconfig | 10 ++ > drivers/clk/Makefile | 1 + > drivers/clk/clk-scpi.c | 344 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 355 insertions(+) > create mode 100644 drivers/clk/clk-scpi.c > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 9897f353bf1a..906bf7dd72a2 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -59,6 +59,16 @@ config COMMON_CLK_RK808 > clocked at 32KHz each. Clkout1 is always on, Clkout2 can off > by control register. > > +config COMMON_CLK_SCPI > + tristate "Clock driver controlled via SCPI interface" > + depends on ARM_SCPI_PROTOCOL > + ---help--- > + This driver provides support for clocks that are controlled > + by firmware that implements the SCPI interface. > + > + This driver uses SCPI Message Protocol to interact with the > + firware providing all the clock controls.
/s/firware/firmware > + > config COMMON_CLK_SI5351 > tristate "Clock driver for SiLabs 5351A/B/C" > depends on I2C > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index 3d00c25382c5..442ab6ebd5b1 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -36,6 +36,7 @@ obj-$(CONFIG_COMMON_CLK_PALMAS) += clk-palmas.o > obj-$(CONFIG_CLK_QORIQ) += clk-qoriq.o > obj-$(CONFIG_COMMON_CLK_RK808) += clk-rk808.o > obj-$(CONFIG_COMMON_CLK_S2MPS11) += clk-s2mps11.o > +obj-$(CONFIG_COMMON_CLK_SCPI) += clk-scpi.o > obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o > obj-$(CONFIG_COMMON_CLK_SI570) += clk-si570.o > obj-$(CONFIG_CLK_TWL6040) += clk-twl6040.o > diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c > new file mode 100644 > index 000000000000..70a2d57da32d > --- /dev/null > +++ b/drivers/clk/clk-scpi.c > @@ -0,0 +1,344 @@ > +/* > + * System Control and Power Interface (SCPI) Protocol based clock driver > + * > + * Copyright (C) 2015 ARM Ltd. > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms and conditions of the GNU General Public License, > + * version 2, as published by the Free Software Foundation. > + * > + * This program is distributed in the hope it will be useful, but WITHOUT > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or > + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for > + * more details. > + * > + * You should have received a copy of the GNU General Public License along > with > + * this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/clkdev.h> > +#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; > + > +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; > +} > + > +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 struct clk_ops scpi_clk_ops = { > + .recalc_rate = scpi_clk_recalc_rate, > + .round_rate = scpi_clk_round_rate, > + .set_rate = scpi_clk_set_rate, > +}; > + > +/* find closest match to given frequency in OPP table */ > +static int __scpi_dvfs_round_rate(struct scpi_clk *clk, unsigned long rate) > +{ > + int idx; > + u32 fmin = 0, fmax = ~0, ftmp; > + struct scpi_opp *opp = clk->info->opps; > + > + for (idx = 0; idx < clk->info->count; idx++, opp++) { > + ftmp = opp->freq; > + if (ftmp >= (u32)rate) { > + if (ftmp <= fmax) > + fmax = ftmp; > + } else { > + if (ftmp >= fmin) > + fmin = ftmp; > + } > + } Is the opp list sorted ? Can't you break instead of going through the whole opp set ? > + if (fmax != ~0) > + return fmax; > + return fmin; return fmax != ~0 ? fmax : fmin; > +} > + > +static unsigned long scpi_dvfs_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct scpi_clk *clk = to_scpi_clk(hw); > + int idx = scpi_ops->dvfs_get_idx(clk->id); > + struct scpi_opp *opp; > + > + if (idx < 0) > + return 0; > + > + opp = clk->info->opps + idx; > + return opp->freq; > +} > + > +static long scpi_dvfs_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ > + struct scpi_clk *clk = to_scpi_clk(hw); > + > + return __scpi_dvfs_round_rate(clk, rate); > +} > + > +static int __scpi_find_dvfs_index(struct scpi_clk *clk, unsigned long rate) > +{ > + int idx, max_opp = clk->info->count; > + struct scpi_opp *opp = clk->info->opps; > + > + for (idx = 0; idx < max_opp; idx++, opp++) > + if (opp->freq == rate) > + break; return idx; > + return (idx == max_opp) ? -EINVAL : idx; return -EINVAL; > +} > + > +static int scpi_dvfs_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct scpi_clk *clk = to_scpi_clk(hw); > + int ret = __scpi_find_dvfs_index(clk, rate); > + > + if (ret < 0) > + return ret; > + return scpi_ops->dvfs_set_idx(clk->id, (u8)ret); > +} > + > +static struct clk_ops scpi_dvfs_ops = { > + .recalc_rate = scpi_dvfs_recalc_rate, > + .round_rate = scpi_dvfs_round_rate, > + .set_rate = scpi_dvfs_set_rate, > +}; > + > +static struct clk * > +scpi_dvfs_ops_init(struct device *dev, struct device_node *np, > + struct scpi_clk *sclk) > +{ > + struct clk_init_data init; > + struct scpi_dvfs_info *info; > + > + init.name = sclk->name; > + init.flags = CLK_IS_ROOT; > + init.num_parents = 0; > + init.ops = &scpi_dvfs_ops; > + sclk->hw.init = &init; > + > + info = scpi_ops->dvfs_get_info(sclk->id); > + if (IS_ERR(info)) > + return (struct clk *)info; Can't you just return NULL ? > + > + sclk->info = info; > + > + return devm_clk_register(dev, &sclk->hw); > +} > + > +static struct clk * > +scpi_clk_ops_init(struct device *dev, struct device_node *np, > + struct scpi_clk *sclk) > +{ > + struct clk_init_data init; > + int ret; > + > + init.name = sclk->name; > + init.flags = CLK_IS_ROOT; > + init.num_parents = 0; > + init.ops = &scpi_clk_ops; > + sclk->hw.init = &init; > + > + ret = scpi_ops->clk_get_range(sclk->id, &sclk->rate_min, > + &sclk->rate_max); > + if (!sclk->rate_max) > + ret = -EINVAL; > + if (ret) > + return ERR_PTR(ret); Ditto, this ERR_PTR song and dance is not nice at all. > + > + return devm_clk_register(dev, &sclk->hw); > +} These two functions are identical apart from the ops, can't we have just one function and differentiate behavior according to the matching compatible string ? > +static const struct of_device_id scpi_clk_match[] = { > + { .compatible = "arm,scpi-dvfs", .data = scpi_dvfs_ops_init, }, > + { .compatible = "arm,scpi-clk", .data = scpi_clk_ops_init, }, > + {} > +}; > + > +static int scpi_clk_probe(struct platform_device *pdev) > +{ > + struct clk **clks; > + int idx, count; > + struct clk_onecell_data *clk_data; > + struct device *dev = &pdev->dev; > + struct device_node *np = dev->of_node; > + struct clk *(*clk_ops_init)(struct device *, struct device_node *, > + struct scpi_clk *); > + > + if (!of_device_is_available(np)) > + return -ENODEV; > + > + clk_ops_init = of_match_node(scpi_clk_match, np)->data; > + if (!clk_ops_init) > + return -ENODEV; > + > + 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) { > + dev_err(dev, "failed to allocate clock provider data\n"); > + return -ENOMEM; > + } > + > + clks = devm_kmalloc(dev, count * sizeof(*clks), GFP_KERNEL); > + if (!clks) { > + dev_err(dev, "failed to allocate clock providers\n"); > + return -ENOMEM; > + } > + > + for (idx = 0; idx < count; idx++) { > + struct scpi_clk *sclk; > + u32 val; > + > + sclk = devm_kzalloc(dev, sizeof(*sclk), GFP_KERNEL); > + if (!sclk) { > + dev_err(dev, "failed to allocate scpi clocks\n"); > + 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] = clk_ops_init(dev, np, sclk); > + if (IS_ERR(clks[idx])) { See above. > + dev_err(dev, "failed to register clock '%s'\n", > + sclk->name); > + } > + > + 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); > + > + platform_set_drvdata(pdev, clk_data); Where is this used ? > + > + return 0; > +} > + > +static int scpi_clk_remove(struct platform_device *pdev) > +{ > + of_clk_del_provider(pdev->dev.of_node); > + platform_set_drvdata(pdev, NULL); > + return 0; > +} > + > +static struct platform_driver scpi_clk_driver = { > + .driver = { > + .name = "scpi_clk", > + .of_match_table = scpi_clk_match, > + }, > + .probe = scpi_clk_probe, > + .remove = scpi_clk_remove, > +}; > + > +static int scpi_clocks_probe(struct platform_device *pdev) > +{ > + scpi_ops = get_scpi_ops(); > + if (!scpi_ops) > + return -ENXIO; > + > + return of_platform_populate(pdev->dev.of_node, scpi_clk_match, > + NULL, &pdev->dev); Do you really need to create a platform_device per sub-node ? > +} > + > +static int scpi_clocks_remove(struct platform_device *pdev) > +{ > + of_platform_depopulate(&pdev->dev); > + scpi_ops = NULL; > + return 0; > +} > + > +static const struct of_device_id scpi_clocks_ids[] = { > + { .compatible = "arm,scpi-clocks", }, > + {} > +}; > + > +static struct platform_driver scpi_clocks_driver = { > + .driver = { > + .name = "scpi_clocks", > + .of_match_table = scpi_clocks_ids, > + }, > + .probe = scpi_clocks_probe, > + .remove = scpi_clocks_remove, > +}; Why do you need two drivers for the clocks ? Isn't the parent device tree node and respective platform device enough to initialize all the subnodes clocks ? > +static int __init scpi_clocks_init(void) > +{ > + int ret; > + > + ret = platform_driver_register(&scpi_clk_driver); > + if (ret) > + return ret; > + > + return platform_driver_register(&scpi_clocks_driver); See above. Thanks, Lorenzo > +} > +module_init(scpi_clocks_init); > + > +static void __exit scpi_clocks_exit(void) > +{ > + platform_driver_unregister(&scpi_clk_driver); > + platform_driver_unregister(&scpi_clocks_driver); > +} > +module_exit(scpi_clocks_exit); > + > +MODULE_AUTHOR("Sudeep Holla <[email protected]>"); > +MODULE_DESCRIPTION("ARM SCPI clock driver"); > +MODULE_LICENSE("GPL"); > -- > 1.9.1 > -- 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/
