| Message ID | 1433760002-24120-4-git-send-email-sudeep.holla@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/08, Sudeep Holla wrote: > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 9897f353bf1a..0fe8daefc105 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 || COMPILE_TEST > + ---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 > + firmware providing all the clock controls. The tabbing is weird here. Both paragraphs should have the same alignment. > diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c > new file mode 100644 > index 000000000000..707b3430c55f > --- /dev/null > +++ b/drivers/clk/clk-scpi.c > + > +#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> Please include <linux/platform_device.h> as well. > + > +struct scpi_clk { > + u32 id; > + const char *name; Do you need this? Or can you just use __clk_get_name() in places where the name is used? > + struct clk_hw hw; > + struct scpi_dvfs_info *info; > + struct scpi_ops *scpi_ops; > +}; > + > +#define to_scpi_clk(clk) container_of(clk, struct scpi_clk, hw) > + > +static unsigned long scpi_clk_recalc_rate(struct clk_hw *hw, > + unsigned long parent_rate) > +{ > + struct scpi_clk *clk = to_scpi_clk(hw); > + > + return clk->scpi_ops->clk_get_val(clk->id); > +} > + > +static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long *parent_rate) > +{ Maybe a comment here like: /* * We can't figure out what rate it will be, so just return the rate * back to the caller. scpi_clk_recalc_rate() will be called * after the rate is set and we'll know what rate the clock is * running at then. */ > + 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 clk->scpi_ops->clk_set_val(clk->id, rate); > +} > + > +static void scpi_clk_disable(struct clk_hw *hw) > +{ > + scpi_clk_set_rate(hw, 0, 0); Does this mean you have to set a rate to enable the clock? Are you relying on drivers to call clk_set_rate() to implicitly enable the clock? If so, it would be better to cache the rate of the clock in set_rate if the clock isn't enabled in software and then send the cached rate during enable. > +} > + [..] > +/* 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; > + const? > + for (idx = 0; idx < clk->info->count; idx++, opp++) { > + ftmp = opp->freq; > + if (ftmp >= (u32)rate) { > + if (ftmp <= fmax) > + fmax = ftmp; > + break; > + } else if (ftmp >= fmin) { > + fmin = ftmp; > + } > + } > + 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 = clk->scpi_ops->dvfs_get_idx(clk->id); > + struct scpi_opp *opp; const? > + > + 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; const? > + > + for (idx = 0; idx < max_opp; idx++, opp++) > + if (opp->freq == rate) > + return idx; > + return -EINVAL; > +} > + [..] > +static struct clk * > +scpi_clk_ops_init(struct device *dev, const struct of_device_id *match, > + struct scpi_clk *sclk) > +{ > + struct clk_init_data init; > + struct clk *clk; > + unsigned long min = 0, max = 0; > + > + init.name = sclk->name; > + init.flags = CLK_IS_ROOT; > + init.num_parents = 0; > + init.ops = match->data; > + sclk->hw.init = &init; > + sclk->scpi_ops = get_scpi_ops(); > + > + if (init.ops == &scpi_dvfs_ops) { > + sclk->info = sclk->scpi_ops->dvfs_get_info(sclk->id); > + if (IS_ERR(sclk->info)) > + return NULL; > + } else if (init.ops == &scpi_clk_ops) { > + if (sclk->scpi_ops->clk_get_range(sclk->id, &min, &max) || !max) > + return NULL; > + } else { > + return NULL; > + } > + > + clk = devm_clk_register(dev, &sclk->hw); > + if (!IS_ERR(clk) && max) > + clk_set_rate_range(clk, min, max); Hm.. we're planning to make clk_register() return a struct clk_hw, so this will block that. We need some sort of clk_hw API that allows us to setup min/max limits on the clock from the provider side. Care to add that?
Hi Stephen, Thanks for the review. On 02/07/15 18:23, Stephen Boyd wrote: > On 06/08, Sudeep Holla wrote: >> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >> index 9897f353bf1a..0fe8daefc105 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 || COMPILE_TEST >> + ---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 >> + firmware providing all the clock controls. > > The tabbing is weird here. Both paragraphs should have the same > alignment. > Indeed, sorry for that, fixed now. >> diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c >> new file mode 100644 >> index 000000000000..707b3430c55f >> --- /dev/null >> +++ b/drivers/clk/clk-scpi.c >> + >> +#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> > > Please include <linux/platform_device.h> as well. > Added now, didn't bother as of_platform.h includes it. >> + >> +struct scpi_clk { >> + u32 id; >> + const char *name; > > Do you need this? Or can you just use __clk_get_name() in places > where the name is used? > Not used, so removed it. [...] >> +static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate, >> + unsigned long *parent_rate) >> +{ > > Maybe a comment here like: > > /* > * We can't figure out what rate it will be, so just return the rate > * back to the caller. scpi_clk_recalc_rate() will be called > * after the rate is set and we'll know what rate the clock is > * running at then. > */ > Done >> + 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 clk->scpi_ops->clk_set_val(clk->id, rate); >> +} >> + >> +static void scpi_clk_disable(struct clk_hw *hw) >> +{ >> + scpi_clk_set_rate(hw, 0, 0); > > Does this mean you have to set a rate to enable the clock? Are > you relying on drivers to call clk_set_rate() to implicitly > enable the clock? If so, it would be better to cache the rate of > the clock in set_rate if the clock isn't enabled in software and > then send the cached rate during enable. > Agreed, I have asked the firmware/SCPI specification guys about more details on what to expect from firmware. Once they get back, will update the code considering your feedback. [...] >> +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; > > const? > All the 3 above fixed now. [...] >> + >> + clk = devm_clk_register(dev, &sclk->hw); >> + if (!IS_ERR(clk) && max) >> + clk_set_rate_range(clk, min, max); > > Hm.. we're planning to make clk_register() return a struct > clk_hw, so this will block that. We need some sort of clk_hw API > that allows us to setup min/max limits on the clock from the > provider side. Care to add that? > Can you provide pointer to the patches or the tree containing those changes ? Are they targeted for v4.3 ? Regards, Sudeep
On 03/07/15 15:52, Sudeep Holla wrote: [...] >>> +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 clk->scpi_ops->clk_set_val(clk->id, rate); >>> +} >>> + >>> +static void scpi_clk_disable(struct clk_hw *hw) >>> +{ >>> + scpi_clk_set_rate(hw, 0, 0); >> >> Does this mean you have to set a rate to enable the clock? Are >> you relying on drivers to call clk_set_rate() to implicitly >> enable the clock? If so, it would be better to cache the rate of >> the clock in set_rate if the clock isn't enabled in software and >> then send the cached rate during enable. >> > > Agreed, I have asked the firmware/SCPI specification guys about > more details on what to expect from firmware. Once they get back, > will update the code considering your feedback. > OK, I got feedback and looks like it was never planned to implement that and even specification needs to be fixed. So I will drop the disable callback support. For now, we don't support {en,dis}able features. We need to amend the specification to fix that. Regards, Sudeep
On 07/03/2015 07:52 AM, Sudeep Holla wrote: > Hi Stephen, > > > Thanks for the review. > > On 02/07/15 18:23, Stephen Boyd wrote: >> On 06/08, Sudeep Holla wrote: >>> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig >>> index 9897f353bf1a..0fe8daefc105 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 || COMPILE_TEST >>> + ---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 >>> + firmware providing all the clock controls. >> >> The tabbing is weird here. Both paragraphs should have the same >> alignment. >> > > Indeed, sorry for that, fixed now. > >>> diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c >>> new file mode 100644 >>> index 000000000000..707b3430c55f >>> --- /dev/null >>> +++ b/drivers/clk/clk-scpi.c >>> + >>> +#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> >> >> Please include <linux/platform_device.h> as well. >> > > Added now, didn't bother as of_platform.h includes it. > >>> + >>> +struct scpi_clk { >>> + u32 id; >>> + const char *name; >> >> Do you need this? Or can you just use __clk_get_name() in places >> where the name is used? >> > > Not used, so removed it. > > > [...] > >>> +static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate, >>> + unsigned long *parent_rate) >>> +{ >> >> Maybe a comment here like: >> >> /* >> * We can't figure out what rate it will be, so just return the rate >> * back to the caller. scpi_clk_recalc_rate() will be called >> * after the rate is set and we'll know what rate the clock is >> * running at then. >> */ >> > > Done > >>> + 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 clk->scpi_ops->clk_set_val(clk->id, rate); >>> +} >>> + >>> +static void scpi_clk_disable(struct clk_hw *hw) >>> +{ >>> + scpi_clk_set_rate(hw, 0, 0); >> >> Does this mean you have to set a rate to enable the clock? Are >> you relying on drivers to call clk_set_rate() to implicitly >> enable the clock? If so, it would be better to cache the rate of >> the clock in set_rate if the clock isn't enabled in software and >> then send the cached rate during enable. >> > > Agreed, I have asked the firmware/SCPI specification guys about > more details on what to expect from firmware. Once they get back, > will update the code considering your feedback. > > [...] > >>> +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; >> >> const? >> > > All the 3 above fixed now. > > [...] > >>> + >>> + clk = devm_clk_register(dev, &sclk->hw); >>> + if (!IS_ERR(clk) && max) >>> + clk_set_rate_range(clk, min, max); >> >> Hm.. we're planning to make clk_register() return a struct >> clk_hw, so this will block that. We need some sort of clk_hw API >> that allows us to setup min/max limits on the clock from the >> provider side. Care to add that? >> > > Can you provide pointer to the patches or the tree containing those > changes ? Are they targeted for v4.3 ? > If I have time I may try to start doing the clk_register() conversion, but it will take a while so I doubt it will be in v4.3. I'm asking if you can add a clk_hw based API that does something like clk_set_rate_range() without requiring a struct clk pointer. i.e. clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the min/max rate of the clock. This way, the driver is only using clk provider APIs and not clk consumer APIs.
On 06/07/15 20:52, Stephen Boyd wrote: > On 07/03/2015 07:52 AM, Sudeep Holla wrote: >> Hi Stephen, >> >> >> Thanks for the review. >> >> On 02/07/15 18:23, Stephen Boyd wrote: >>> On 06/08, Sudeep Holla wrote: [...] >>>> + >>>> + clk = devm_clk_register(dev, &sclk->hw); >>>> + if (!IS_ERR(clk) && max) >>>> + clk_set_rate_range(clk, min, max); >>> >>> Hm.. we're planning to make clk_register() return a struct >>> clk_hw, so this will block that. We need some sort of clk_hw API >>> that allows us to setup min/max limits on the clock from the >>> provider side. Care to add that? >>> >> >> Can you provide pointer to the patches or the tree containing those >> changes ? Are they targeted for v4.3 ? >> > > If I have time I may try to start doing the clk_register() conversion, > but it will take a while so I doubt it will be in v4.3. I'm asking if > you can add a clk_hw based API that does something like > clk_set_rate_range() without requiring a struct clk pointer. i.e. > clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the > min/max rate of the clock. This way, the driver is only using clk > provider APIs and not clk consumer APIs. > I understand the intention of separating clk provider helpers/APIs and clk consumer APIs. Since {min,max}_rate are part of struct clk itself, I was thinking that you would have moved it to struct clk_core as part of the rework you mentioned and hence asked about the patches. IIUC, if {min,max}_rate remain part of struct clk, then how are we restricting that operation to just the clk providers ? clk consumer can still directly modify or use clk_set_rate_range. Do we continue to provide that feature for both provider and consumer ? If so I assume {min,max}_rate range requested by consumer should be within the limits set by provider and do we maintain both the limits ? Sorry if I am missing something fundamental since I don't have much knowledge of clk layer internals. Regards, Sudeep
On 07/07, Sudeep Holla wrote: > > > On 06/07/15 20:52, Stephen Boyd wrote: > >> > > > >If I have time I may try to start doing the clk_register() conversion, > >but it will take a while so I doubt it will be in v4.3. I'm asking if > >you can add a clk_hw based API that does something like > >clk_set_rate_range() without requiring a struct clk pointer. i.e. > >clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the > >min/max rate of the clock. This way, the driver is only using clk > >provider APIs and not clk consumer APIs. > > > > I understand the intention of separating clk provider helpers/APIs > and clk consumer APIs. Since {min,max}_rate are part of struct clk > itself, I was thinking that you would have moved it to struct clk_core > as part of the rework you mentioned and hence asked about the patches. > > IIUC, if {min,max}_rate remain part of struct clk, then how are we > restricting that operation to just the clk providers ? clk consumer > can still directly modify or use clk_set_rate_range. > > Do we continue to provide that feature for both provider and consumer ? > If so I assume {min,max}_rate range requested by consumer should be > within the limits set by provider and do we maintain both the limits ? > > Sorry if I am missing something fundamental since I don't have much > knowledge of clk layer internals. > Yes struct clk would have min/max, and struct clk_core would have min/max. Then some sort of provider API (or possibly even clk_init_data) would take the min/max fields and copy them over to struct clk_core. Then during set_rate operations we would aggregate the constraints from struct clk like we already do and add in the constrains in struct clk_core. One downside to adding new fields to clk_init_data is that there are drivers out there that aren't initializing that structure to 0, and they're putting it on the stack, so stack junk can come through. Furthermore, min/max would mean that every driver needs to specify some large number for max or we have to special case min == max == 0 and ignore it. Somehow it needs to be opt-in. If we want to go down the clk_init_data route then perhaps we need some sort of rate_constraint struct pointer in there that drivers can optionally setup. struct clk_rate_constraint { unsigned long min; unsigned long max; }; struct clk_init_data { ... struct clk_rate_constraint *rate_constraint; }; I haven't thought it through completely, but I can probably write up some patch tomorrow after I sleep on it.
Hi Stephen, On 08/07/15 02:46, Stephen Boyd wrote: > On 07/07, Sudeep Holla wrote: >> >> >> On 06/07/15 20:52, Stephen Boyd wrote: >>>> >>> >>> If I have time I may try to start doing the clk_register() conversion, >>> but it will take a while so I doubt it will be in v4.3. I'm asking if >>> you can add a clk_hw based API that does something like >>> clk_set_rate_range() without requiring a struct clk pointer. i.e. >>> clk_hw_set_rate_range(struct clk_hw *hw, min, max) that constraints the >>> min/max rate of the clock. This way, the driver is only using clk >>> provider APIs and not clk consumer APIs. >>> >> >> I understand the intention of separating clk provider helpers/APIs >> and clk consumer APIs. Since {min,max}_rate are part of struct clk >> itself, I was thinking that you would have moved it to struct clk_core >> as part of the rework you mentioned and hence asked about the patches. >> >> IIUC, if {min,max}_rate remain part of struct clk, then how are we >> restricting that operation to just the clk providers ? clk consumer >> can still directly modify or use clk_set_rate_range. >> >> Do we continue to provide that feature for both provider and consumer ? >> If so I assume {min,max}_rate range requested by consumer should be >> within the limits set by provider and do we maintain both the limits ? >> >> Sorry if I am missing something fundamental since I don't have much >> knowledge of clk layer internals. >> > > Yes struct clk would have min/max, and struct clk_core would have > min/max. Then some sort of provider API (or possibly even > clk_init_data) would take the min/max fields and copy them over > to struct clk_core. Then during set_rate operations we would > aggregate the constraints from struct clk like we already do and > add in the constrains in struct clk_core. > > One downside to adding new fields to clk_init_data is that there > are drivers out there that aren't initializing that structure to > 0, and they're putting it on the stack, so stack junk can come > through. Furthermore, min/max would mean that every driver needs > to specify some large number for max or we have to special case > min == max == 0 and ignore it. Somehow it needs to be opt-in. If > we want to go down the clk_init_data route then perhaps we need > some sort of rate_constraint struct pointer in there that drivers > can optionally setup. > > struct clk_rate_constraint { > unsigned long min; > unsigned long max; > }; > > struct clk_init_data { > ... > struct clk_rate_constraint *rate_constraint; > }; > > I haven't thought it through completely, but I can probably write > up some patch tomorrow after I sleep on it. > I am hoping to get this series for v4.3. In order to avoid using consumer API, I can revert back to the min,max check I had in the round_rate earlier if that's fine with you ? Let me know so that I can post the next version based on that. All the other comments are already addressed. Also since this series depends on SCPI, I was thinking to get it merged via ARM-SoC, but that might conflict with the round_rate prototype change. Do do plan to share a stable base with arm-soc guys or you expect all the changes to be contained in clk tree ? Regards, Sudeep
diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig index 9897f353bf1a..0fe8daefc105 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 || COMPILE_TEST + ---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 + firmware providing all the clock controls. + 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..707b3430c55f --- /dev/null +++ b/drivers/clk/clk-scpi.c @@ -0,0 +1,286 @@ +/* + * 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/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; + struct scpi_ops *scpi_ops; +}; + +#define to_scpi_clk(clk) container_of(clk, struct scpi_clk, hw) + +static unsigned long scpi_clk_recalc_rate(struct clk_hw *hw, + unsigned long parent_rate) +{ + struct scpi_clk *clk = to_scpi_clk(hw); + + return clk->scpi_ops->clk_get_val(clk->id); +} + +static long scpi_clk_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *parent_rate) +{ + 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 clk->scpi_ops->clk_set_val(clk->id, rate); +} + +static void scpi_clk_disable(struct clk_hw *hw) +{ + scpi_clk_set_rate(hw, 0, 0); +} + +static const struct clk_ops scpi_clk_ops = { + .recalc_rate = scpi_clk_recalc_rate, + .round_rate = scpi_clk_round_rate, + .set_rate = scpi_clk_set_rate, + .disable = scpi_clk_disable, +}; + +/* 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; + break; + } else if (ftmp >= fmin) { + fmin = ftmp; + } + } + 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 = clk->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) + return 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 clk->scpi_ops->dvfs_set_idx(clk->id, (u8)ret); +} + +static const 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 const struct of_device_id scpi_clk_match[] = { + { .compatible = "arm,scpi-dvfs-clocks", .data = &scpi_dvfs_ops, }, + { .compatible = "arm,scpi-variable-clocks", .data = &scpi_clk_ops, }, + {} +}; + +static struct clk * +scpi_clk_ops_init(struct device *dev, const struct of_device_id *match, + struct scpi_clk *sclk) +{ + struct clk_init_data init; + struct clk *clk; + unsigned long min = 0, max = 0; + + init.name = sclk->name; + init.flags = CLK_IS_ROOT; + init.num_parents = 0; + init.ops = match->data; + sclk->hw.init = &init; + sclk->scpi_ops = get_scpi_ops(); + + if (init.ops == &scpi_dvfs_ops) { + sclk->info = sclk->scpi_ops->dvfs_get_info(sclk->id); + if (IS_ERR(sclk->info)) + return NULL; + } else if (init.ops == &scpi_clk_ops) { + if (sclk->scpi_ops->clk_get_range(sclk->id, &min, &max) || !max) + return NULL; + } else { + return NULL; + } + + clk = devm_clk_register(dev, &sclk->hw); + if (!IS_ERR(clk) && max) + clk_set_rate_range(clk, min, max); + return clk; +} + +static int scpi_clk_add(struct device *dev, struct device_node *np, + const struct of_device_id *match) +{ + 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, match, 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; + + return of_clk_add_provider(np, of_clk_src_onecell_get, clk_data); +} + +static int scpi_clocks_remove(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *child, *np = dev->of_node; + + for_each_available_child_of_node(np, child) + of_clk_del_provider(np); + return 0; +} + +static int scpi_clocks_probe(struct platform_device *pdev) +{ + int ret; + struct device *dev = &pdev->dev; + struct device_node *child, *np = dev->of_node; + const struct of_device_id *match; + + if (!get_scpi_ops()) + return -ENXIO; + + for_each_available_child_of_node(np, child) { + match = of_match_node(scpi_clk_match, child); + if (!match) + continue; + ret = scpi_clk_add(dev, child, match); + if (ret) { + scpi_clocks_remove(pdev); + return ret; + } + } + 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, +}; +module_platform_driver(scpi_clocks_driver); + +MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>"); +MODULE_DESCRIPTION("ARM SCPI clock driver"); +MODULE_LICENSE("GPL v2");
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 <sudeep.holla@arm.com> Cc: Mike Turquette <mturquette@linaro.org> Cc: Stephen Boyd <sboyd@codeaurora.org> Cc: Liviu Dudau <Liviu.Dudau@arm.com> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Jon Medhurst (Tixy) <tixy@linaro.org> Cc: linux-clk@vger.kernel.org --- drivers/clk/Kconfig | 10 ++ drivers/clk/Makefile | 1 + drivers/clk/clk-scpi.c | 286 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 297 insertions(+) create mode 100644 drivers/clk/clk-scpi.c