| Message ID | 1384515691-26299-2-git-send-email-sachin.kamat@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, CC'ing Doug and Andrew who have also worked on ASV. [...] > diff --git a/drivers/power/asv/Kconfig b/drivers/power/asv/Kconfig > new file mode 100644 > index 000000000000..761119d9f7f8 > --- /dev/null > +++ b/drivers/power/asv/Kconfig > @@ -0,0 +1,10 @@ > +menuconfig POWER_ASV > + bool "Adaptive Supply Voltage (ASV) support" Select POWER_SUPPLY here ? > + help > + ASV is a technique used on Samsung SoCs which provides the > + recommended supply voltage for some specific parts(like CPU, MIF, etc) > + that support DVFS. For a given operating frequency, the voltage is > + recommended based on SoCs ASV group. ASV group info is provided in the > + chip id info which depends on the chip manufacturing process. > + [...] > + > + if (asv_info->ops->init_asv) > + ret = asv_info->ops->init_asv(asv_info); > + if (ret) { > + pr_err("asv_init failed for %s : %d\n", > + asv_info->name, ret); > + goto err; > + } > + > + /* In case of parsing table from DT, we may need to add flag to identify > + DT supporting members and call init_asv_table from asv_init_opp_table( > + after getting dev_node from dev,if required), instead of calling here. > + */ Please fix Multi-line comment here and through the rest of the patches as well. [...] > + * @nr_dvfs_level: Number of dvfs levels supported by member. > + * @dvfs_table: Table containing supported ASV freqs and corresponding volts. > + * @asv_grp: ASV group of member. > + * @flags: ASV flags What are the ASV flags you had in mind ? > + */ > +struct asv_info { > + const char *name; > + enum asv_type_id type; > + struct asv_ops *ops; > + unsigned int nr_dvfs_level; > + struct asv_freq_table *dvfs_table; > + unsigned int asv_grp; > + unsigned int flags; > +}; [...] > + > +#ifdef CONFIG_POWER_ASV > +/* asv_get_volt - get the ASV for target_freq for particular target_type. > + * returns 0 if target_freq is not supported Could you add a comment for asv_init_opp_table as well. > + */ > +extern unsigned int asv_get_volt(enum asv_type_id target_type, > + unsigned int target_freq); > +extern int asv_init_opp_table(struct device *dev, > + enum asv_type_id target_type); > +#else > +static inline unsigned int asv_get_volt(enum asv_type_id target_type, > + unsigned int target_freq) { return 0; } > +static int asv_init_opp_table(struct device *dev, enum asv_type_id target_type) > + { return 0; } > + > +#endif /* CONFIG_POWER_EXYNOS_AVS */ > +#endif /* __ASV_H */ Regards, Abhilash -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Yadwinder, Sachin, On Friday 15 of November 2013 17:11:28 Sachin Kamat wrote: > From: Yadwinder Singh Brar <yadi.brar@samsung.com> > > This patch introduces a common ASV (Adaptive Supply Voltage) basic framework > for samsung SoCs. It provides common APIs (to be called by users to get ASV > values or init opp_table) and an interface for SoC specific drivers to > register ASV members (instances). [snip] > diff --git a/drivers/power/asv/asv.c b/drivers/power/asv/asv.c > new file mode 100644 > index 000000000000..3f2c31a0d3a9 > --- /dev/null > +++ b/drivers/power/asv/asv.c > @@ -0,0 +1,176 @@ > +/* > + * ASV(Adaptive Supply Voltage) common core > + * > + * Copyright (c) 2013 Samsung Electronics Co., Ltd. > + * http://www.samsung.com/ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > +*/ > + > +#include <linux/io.h> > +#include <linux/pm_opp.h> > +#include <linux/slab.h> > +#include <linux/device.h> > +#include <linux/power/asv-driver.h> > + > +static LIST_HEAD(asv_list); > +static DEFINE_MUTEX(asv_mutex); > + > +struct asv_member { > + struct list_head node; > + struct asv_info *asv_info; nit: Inconsistent indentation of member names. In general I would recommend dropping the tabs between types and names and using a single space instead, since this is more future proof - you will never have to change other lines to add new ones. > +}; > + > +static void add_asv_member(struct asv_member *asv_mem) > +{ > + mutex_lock(&asv_mutex); > + list_add_tail(&asv_mem->node, &asv_list); > + mutex_unlock(&asv_mutex); > +} > + > +static struct asv_member *asv_get_mem(enum asv_type_id asv_type) I don't really like this enum based look-up. It's hard to define an enum that covers any possible existing and future platforms that would not be bloated with single platform specific entries. IMHO something string based could be more scalable. > +{ > + struct asv_member *asv_mem; > + struct asv_info *asv_info; > + > + list_for_each_entry(asv_mem, &asv_list, node) { > + asv_info = asv_mem->asv_info; > + if (asv_type == asv_info->type) > + return asv_mem; > + } Don't you need any kind of locking here? A mutex in add_asv_member() suggests that read access to the list should be protected as well. > + > + return NULL; > +} > + > +unsigned int asv_get_volt(enum asv_type_id target_type, > + unsigned int target_freq) Do you need this function at all? I believe this is all about populating device's OPP array with frequencies and voltages according to its ASV level. Users will be able to query for required voltage using standard OPP calls then, without a need for ASV specific functions like this one. > +{ > + struct asv_member *asv_mem = asv_get_mem(target_type); > + struct asv_freq_table *dvfs_table; > + struct asv_info *asv_info; > + unsigned int i; > + > + if (!asv_mem) > + return 0; > + > + asv_info = asv_mem->asv_info; > + dvfs_table = asv_info->dvfs_table; > + > + for (i = 0; i < asv_info->nr_dvfs_level; i++) { > + if (dvfs_table[i].freq == target_freq) > + return dvfs_table[i].volt; > + } > + > + return 0; > +} > + > +int asv_init_opp_table(struct device *dev, enum asv_type_id target_type) > +{ > + struct asv_member *asv_mem = asv_get_mem(target_type); > + struct asv_info *asv_info; > + struct asv_freq_table *dvfs_table; > + unsigned int i; > + > + if (!asv_mem) > + return -EINVAL; > + > + asv_info = asv_mem->asv_info; > + dvfs_table = asv_info->dvfs_table; > + > + for (i = 0; i < asv_info->nr_dvfs_level; i++) { > + if (dev_pm_opp_add(dev, dvfs_table[i].freq * 1000, > + dvfs_table[i].volt)) { > + dev_warn(dev, "Failed to add OPP %d\n", > + dvfs_table[i].freq); Hmm, shouldn't it be considered a failure instead? > + continue; > + } > + } > + > + return 0; > +} > + > +static struct asv_member *asv_init_member(struct asv_info *asv_info) > +{ > + struct asv_member *asv_mem; > + int ret = 0; > + > + if (!asv_info) { > + pr_err("No ASV info provided\n"); > + return NULL; I'd suggest adopting the ERR_PTR() convention, which allows returning more information about the error. > + } > + > + asv_mem = kzalloc(sizeof(*asv_mem), GFP_KERNEL); > + if (!asv_mem) { > + pr_err("Allocation failed for member: %s\n", asv_info->name); > + return NULL; > + } > + > + asv_mem->asv_info = kmemdup(asv_info, sizeof(*asv_info), GFP_KERNEL); > + if (!asv_mem->asv_info) { > + pr_err("Copying asv_info failed for member: %s\n", > + asv_info->name); > + kfree(asv_mem); > + return NULL; For consistency, the error here should be handled as below, by using the error path. > + } > + asv_info = asv_mem->asv_info; > + > + if (asv_info->ops->get_asv_group) { > + ret = asv_info->ops->get_asv_group(asv_info); > + if (ret) { > + pr_err("get_asv_group failed for %s : %d\n", > + asv_info->name, ret); > + goto err; > + } > + } > + > + if (asv_info->ops->init_asv) > + ret = asv_info->ops->init_asv(asv_info); > + if (ret) { > + pr_err("asv_init failed for %s : %d\n", > + asv_info->name, ret); > + goto err; > + } > + > + /* In case of parsing table from DT, we may need to add flag to identify > + DT supporting members and call init_asv_table from asv_init_opp_table( > + after getting dev_node from dev,if required), instead of calling here. > + */ coding style: Wrong multi-line comment style. /* * This is a valid multi-line comment. */ > + > + if (asv_info->ops->init_asv_table) { > + ret = asv_info->ops->init_asv_table(asv_info); > + if (ret) { > + pr_err("init_asv_table failed for %s : %d\n", > + asv_info->name, ret); > + goto err; > + } > + } Hmm, I don't see a point of these three separate callbacks above. In general, I'd suggest a different architecture. I'd see this more as: 1) Platform code registers static platform device to instantiate SoC ASV driver. 2) SoC specific ASV driver probes, reads group ID from hardware register, calls register_asv_member() with appropriate DVFS table for detected group. 3) Driver using ASV calls asv_init_opp_table() with its struct device and ASV member name, which causes the ASV code to fill device's operating point using OPP calls. Now client driver has all the information it needs and the work of ASV subsystem is done. The control flow between drivers would be much simpler and no callbacks would have to be called. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, Thanks for your thorough review and nice suggestions. [snip] >> +} >> + >> +static struct asv_member *asv_get_mem(enum asv_type_id asv_type) > > I don't really like this enum based look-up. It's hard to define an enum > that covers any possible existing and future platforms that would not be > bloated with single platform specific entries. IMHO something string based > could be more scalable. > Yes, I also agree string based look-up will be better. I was thinking to convert to it, after initial discussion over the APIs. >> +{ >> + struct asv_member *asv_mem; >> + struct asv_info *asv_info; >> + >> + list_for_each_entry(asv_mem, &asv_list, node) { >> + asv_info = asv_mem->asv_info; >> + if (asv_type == asv_info->type) >> + return asv_mem; >> + } > > Don't you need any kind of locking here? A mutex in add_asv_member() > suggests that read access to the list should be protected as well. > hmmm, yes should be their for completeness of code. >> + >> + return NULL; >> +} >> + >> +unsigned int asv_get_volt(enum asv_type_id target_type, >> + unsigned int target_freq) > > Do you need this function at all? I believe this is all about populating > device's OPP array with frequencies and voltages according to its ASV > level. Users will be able to query for required voltage using standard OPP > calls then, without a need for ASV specific functions like this one. > Yes, I had put a comment in initial version after commit message : "Hopefully asv_get_volt() can go out in future, once all users start using OPP library." , which seems to be missed in this version. I had kept it for the time being in initial version, to keep it usable(for testing) with existing cpufreq drivers, which need to reworked and may take time. [snip] >> + >> + for (i = 0; i < asv_info->nr_dvfs_level; i++) { >> + if (dev_pm_opp_add(dev, dvfs_table[i].freq * 1000, >> + dvfs_table[i].volt)) { >> + dev_warn(dev, "Failed to add OPP %d\n", >> + dvfs_table[i].freq); > > Hmm, shouldn't it be considered a failure instead? > hmm, not really always. Theoretically system with some less(failed to add) levels can work. Moreover I had prefered to keep it only warning, just to keep the behaviour of asv_init_opp_table() similar to that of its counter part of_init_opp_table(). >> + continue; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static struct asv_member *asv_init_member(struct asv_info *asv_info) >> +{ >> + struct asv_member *asv_mem; >> + int ret = 0; >> + >> + if (!asv_info) { >> + pr_err("No ASV info provided\n"); >> + return NULL; > > I'd suggest adopting the ERR_PTR() convention, which allows returning more > information about the error. > Will it be really usefull here?, as we are not checking return value of any function. Bur for some cases below, i will also like to get it used. >> + } >> + >> + asv_mem = kzalloc(sizeof(*asv_mem), GFP_KERNEL); >> + if (!asv_mem) { >> + pr_err("Allocation failed for member: %s\n", asv_info->name); >> + return NULL; >> + } [snip] > > Hmm, I don't see a point of these three separate callbacks above. > > In general, I'd suggest a different architecture. I'd see this more as: > > 1) Platform code registers static platform device to instantiate SoC ASV > driver. > 2) SoC specific ASV driver probes, reads group ID from hardware register, > calls register_asv_member() with appropriate DVFS table for detected > group. > 3) Driver using ASV calls asv_init_opp_table() with its struct device and > ASV member name, which causes the ASV code to fill device's operating > point using OPP calls. > > Now client driver has all the information it needs and the work of ASV > subsystem is done. The control flow between drivers would be much simpler > and no callbacks would have to be called. > Architecture stated above seems to be a subset(one possible way of use), of the proposed architecture. If someone really have nothing much to do, he can adopt the above stated approach using this framework also, callbacks are not mandatory. Since we usually have more things to do other than only reading fused group value and simply parsing a table index, so in drivers we have to implement functions to segregate stuff and different people do it in different way. Its an attempt to provide a way to keep structure(functions) similar for easy understanding and factoring out of common code. Moreover, I feels need of callbacks if we have to do something depending upon(specific) the user/instance of ASV member. One thing came in my mind was dev_node may be required if we may think of parsing ASV table from DT and may be more things in future. I would like to get rectified, other nit/suggestions stated by you in next version. Thanks & Regards, Yadwinder -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Abhilash, [ ... ] >> + * @nr_dvfs_level: Number of dvfs levels supported by member. >> + * @dvfs_table: Table containing supported ASV freqs and corresponding volts. >> + * @asv_grp: ASV group of member. >> + * @flags: ASV flags > What are the ASV flags you had in mind ? Right now we don't have any, some thing like delayed init of asv table depending upon dev_node/user(instance) was coming in my mind. Actually I missed to remove it for the time being. Thanks for your review and other suggestions. Regards, Yadwinder -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sunday 15 of December 2013 22:30:13 Yadwinder Singh Brar wrote: [snip] > >> + > >> + return NULL; > >> +} > >> + > >> +unsigned int asv_get_volt(enum asv_type_id target_type, > >> + unsigned int target_freq) > > > > Do you need this function at all? I believe this is all about populating > > device's OPP array with frequencies and voltages according to its ASV > > level. Users will be able to query for required voltage using standard OPP > > calls then, without a need for ASV specific functions like this one. > > > > Yes, I had put a comment in initial version after commit message : > "Hopefully asv_get_volt() can go out in future, once all users start using OPP > library." , which seems to be missed in this version. > I had kept it for the time being in initial version, to keep it > usable(for testing) with > existing cpufreq drivers, which need to reworked and may take time. Hmm, at the moment none of cpufreq drivers use ASV, so they need to be reworked anyway to use it either by the means of a private get_volt function or OPP framework. I agree that OPP may require more work, though. If we decide to keep this function in final version, a comment should be added saying that its usage is deprecated in favor of generic OPP helpers. > > [snip] > >> + > >> + for (i = 0; i < asv_info->nr_dvfs_level; i++) { > >> + if (dev_pm_opp_add(dev, dvfs_table[i].freq * 1000, > >> + dvfs_table[i].volt)) { > >> + dev_warn(dev, "Failed to add OPP %d\n", > >> + dvfs_table[i].freq); > > > > Hmm, shouldn't it be considered a failure instead? > > > > hmm, not really always. Theoretically system with some less(failed to add) > levels can work. Moreover I had prefered to keep it only warning, just to > keep the behaviour of asv_init_opp_table() similar to that of its > counter part of_init_opp_table(). I'm not quite convinced about it. If dev_pm_opp_add() fails, doesn't it mean that something broke seriously in upper layer and we should propagate the error down? Especially when looking at opp_add(), the only failure conditions I can find are memory allocation errors which mean that the system is unlikely to operate correctly anyway. > > >> + continue; > >> + } > >> + } > >> + > >> + return 0; > >> +} > >> + > >> +static struct asv_member *asv_init_member(struct asv_info *asv_info) > >> +{ > >> + struct asv_member *asv_mem; > >> + int ret = 0; > >> + > >> + if (!asv_info) { > >> + pr_err("No ASV info provided\n"); > >> + return NULL; > > > > I'd suggest adopting the ERR_PTR() convention, which allows returning more > > information about the error. > > > > Will it be really usefull here?, as we are not checking return value > of any function. Why not? Here you have ERR_PTR(-EINVAL), then... > Bur for some cases below, i will also like to get it used. > > >> + } > >> + > >> + asv_mem = kzalloc(sizeof(*asv_mem), GFP_KERNEL); > >> + if (!asv_mem) { > >> + pr_err("Allocation failed for member: %s\n", asv_info->name); > >> + return NULL; ERR_PTR(-ENOMEM) here. These are two completely different error cases. > >> + } > > [snip] > > > > Hmm, I don't see a point of these three separate callbacks above. > > > > In general, I'd suggest a different architecture. I'd see this more as: > > > > 1) Platform code registers static platform device to instantiate SoC ASV > > driver. > > 2) SoC specific ASV driver probes, reads group ID from hardware register, > > calls register_asv_member() with appropriate DVFS table for detected > > group. > > 3) Driver using ASV calls asv_init_opp_table() with its struct device and > > ASV member name, which causes the ASV code to fill device's operating > > point using OPP calls. > > > > Now client driver has all the information it needs and the work of ASV > > subsystem is done. The control flow between drivers would be much simpler > > and no callbacks would have to be called. > > > > Architecture stated above seems to be a subset(one possible way of use), > of the proposed architecture. If someone really have nothing much to do, > he can adopt the above stated approach using this framework also, > callbacks are not mandatory. I believe that kernel design principles are to first start with something simple and then if a real need for an extension shows up then extend existing code base with missing features. > > Since we usually have more things to do other than only reading > fused group value and simply parsing a table index, so in drivers we have to > implement functions to segregate stuff and different people do it in > different way. Its an attempt to provide a way to keep structure(functions) > similar for easy understanding and factoring out of common code. I fail to see those more things. Could you elaborate a bit about them? From what I see, all the potential ASV users need is a set of operating points (frequency:voltage pairs) appropriate for the SoC we are running on (i.e. matching our ASV group index). Is there anything more we need to do for ASV support? > > Moreover, I feels need of callbacks if we have to do something depending > upon(specific) the user/instance of ASV member. One thing came > in my mind was dev_node may be required if we may think of parsing > ASV table from DT and may be more things in future. We can always add such things later, if real need shows up. As I said above, we should rather start with something simple, to avoid overdesigning things without knowing real use cases that may show up later. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Tomasz, Sorry for being late. On Sun, Dec 15, 2013 at 10:51 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On Sunday 15 of December 2013 22:30:13 Yadwinder Singh Brar wrote: > [snip] >> >> + >> >> + return NULL; >> >> +} >> >> + >> >> +unsigned int asv_get_volt(enum asv_type_id target_type, >> >> + unsigned int target_freq) >> > >> > Do you need this function at all? I believe this is all about populating >> > device's OPP array with frequencies and voltages according to its ASV >> > level. Users will be able to query for required voltage using standard OPP >> > calls then, without a need for ASV specific functions like this one. >> > >> >> Yes, I had put a comment in initial version after commit message : >> "Hopefully asv_get_volt() can go out in future, once all users start using OPP >> library." , which seems to be missed in this version. >> I had kept it for the time being in initial version, to keep it >> usable(for testing) with >> existing cpufreq drivers, which need to reworked and may take time. > > Hmm, at the moment none of cpufreq drivers use ASV, so they need to be > reworked anyway to use it either by the means of a private get_volt > function or OPP framework. I agree that OPP may require more work, > though. > > If we decide to keep this function in final version, a comment should be > added saying that its usage is deprecated in favor of generic OPP helpers. > yes. >> >> [snip] >> >> + >> >> + for (i = 0; i < asv_info->nr_dvfs_level; i++) { >> >> + if (dev_pm_opp_add(dev, dvfs_table[i].freq * 1000, >> >> + dvfs_table[i].volt)) { >> >> + dev_warn(dev, "Failed to add OPP %d\n", >> >> + dvfs_table[i].freq); >> > >> > Hmm, shouldn't it be considered a failure instead? >> > >> >> hmm, not really always. Theoretically system with some less(failed to add) >> levels can work. Moreover I had prefered to keep it only warning, just to >> keep the behaviour of asv_init_opp_table() similar to that of its >> counter part of_init_opp_table(). > > I'm not quite convinced about it. If dev_pm_opp_add() fails, doesn't it mean > that something broke seriously in upper layer and we should propagate the > error down? Especially when looking at opp_add(), the only failure > conditions I can find are memory allocation errors which mean that the > system is unlikely to operate correctly anyway. > yes, for the time being i had prefered to keep it similar to of_init_opp_table() behaviour wise. If required both should be fixed. >> [snip] >> > >> > Hmm, I don't see a point of these three separate callbacks above. >> > >> > In general, I'd suggest a different architecture. I'd see this more as: >> > >> > 1) Platform code registers static platform device to instantiate SoC ASV >> > driver. >> > 2) SoC specific ASV driver probes, reads group ID from hardware register, >> > calls register_asv_member() with appropriate DVFS table for detected >> > group. >> > 3) Driver using ASV calls asv_init_opp_table() with its struct device and >> > ASV member name, which causes the ASV code to fill device's operating >> > point using OPP calls. >> > >> > Now client driver has all the information it needs and the work of ASV >> > subsystem is done. The control flow between drivers would be much simpler >> > and no callbacks would have to be called. >> > >> >> Architecture stated above seems to be a subset(one possible way of use), >> of the proposed architecture. If someone really have nothing much to do, >> he can adopt the above stated approach using this framework also, >> callbacks are not mandatory. > > I believe that kernel design principles are to first start with something > simple and then if a real need for an extension shows up then extend > existing code base with missing features. > Sorry, I can't see it complex as with architecture stated above also we have to implement similar structure in drivers as we are already doing now individually in each soc driver. >> >> Since we usually have more things to do other than only reading >> fused group value and simply parsing a table index, so in drivers we have to >> implement functions to segregate stuff and different people do it in >> different way. Its an attempt to provide a way to keep structure(functions) >> similar for easy understanding and factoring out of common code. > > I fail to see those more things. Could you elaborate a bit about them? Usually we need to implement functions in drivers clearly demarking following : 1- Reading chip info (which can be done at probe time only once for all). 2- Parse/Calculate(modify) ASV group. 3- Any Group specific one time setting. eg ABB settings. 4- Parsing and modifying table ( implementing Voltage locking, if required based on locking info bits). Best Regards, Yadwinder -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 5e2054afe840..09da1fd730cd 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -385,3 +385,4 @@ source "drivers/power/reset/Kconfig" endif # POWER_SUPPLY source "drivers/power/avs/Kconfig" +source "drivers/power/asv/Kconfig" diff --git a/drivers/power/Makefile b/drivers/power/Makefile index 372b4e8ab598..788e36d37d24 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -54,6 +54,7 @@ obj-$(CONFIG_CHARGER_BQ2415X) += bq2415x_charger.o obj-$(CONFIG_CHARGER_BQ24190) += bq24190_charger.o obj-$(CONFIG_CHARGER_BQ24735) += bq24735-charger.o obj-$(CONFIG_POWER_AVS) += avs/ +obj-$(CONFIG_POWER_ASV) += asv/ obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o obj-$(CONFIG_CHARGER_TPS65090) += tps65090-charger.o obj-$(CONFIG_POWER_RESET) += reset/ diff --git a/drivers/power/asv/Kconfig b/drivers/power/asv/Kconfig new file mode 100644 index 000000000000..761119d9f7f8 --- /dev/null +++ b/drivers/power/asv/Kconfig @@ -0,0 +1,10 @@ +menuconfig POWER_ASV + bool "Adaptive Supply Voltage (ASV) support" + help + ASV is a technique used on Samsung SoCs which provides the + recommended supply voltage for some specific parts(like CPU, MIF, etc) + that support DVFS. For a given operating frequency, the voltage is + recommended based on SoCs ASV group. ASV group info is provided in the + chip id info which depends on the chip manufacturing process. + + Say Y here to enable Adaptive Supply Voltage support. diff --git a/drivers/power/asv/Makefile b/drivers/power/asv/Makefile new file mode 100644 index 000000000000..366cb04f557b --- /dev/null +++ b/drivers/power/asv/Makefile @@ -0,0 +1 @@ +obj-$(CONFIG_POWER_ASV) += asv.o diff --git a/drivers/power/asv/asv.c b/drivers/power/asv/asv.c new file mode 100644 index 000000000000..3f2c31a0d3a9 --- /dev/null +++ b/drivers/power/asv/asv.c @@ -0,0 +1,176 @@ +/* + * ASV(Adaptive Supply Voltage) common core + * + * Copyright (c) 2013 Samsung Electronics Co., Ltd. + * http://www.samsung.com/ + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. +*/ + +#include <linux/io.h> +#include <linux/pm_opp.h> +#include <linux/slab.h> +#include <linux/device.h> +#include <linux/power/asv-driver.h> + +static LIST_HEAD(asv_list); +static DEFINE_MUTEX(asv_mutex); + +struct asv_member { + struct list_head node; + struct asv_info *asv_info; +}; + +static void add_asv_member(struct asv_member *asv_mem) +{ + mutex_lock(&asv_mutex); + list_add_tail(&asv_mem->node, &asv_list); + mutex_unlock(&asv_mutex); +} + +static struct asv_member *asv_get_mem(enum asv_type_id asv_type) +{ + struct asv_member *asv_mem; + struct asv_info *asv_info; + + list_for_each_entry(asv_mem, &asv_list, node) { + asv_info = asv_mem->asv_info; + if (asv_type == asv_info->type) + return asv_mem; + } + + return NULL; +} + +unsigned int asv_get_volt(enum asv_type_id target_type, + unsigned int target_freq) +{ + struct asv_member *asv_mem = asv_get_mem(target_type); + struct asv_freq_table *dvfs_table; + struct asv_info *asv_info; + unsigned int i; + + if (!asv_mem) + return 0; + + asv_info = asv_mem->asv_info; + dvfs_table = asv_info->dvfs_table; + + for (i = 0; i < asv_info->nr_dvfs_level; i++) { + if (dvfs_table[i].freq == target_freq) + return dvfs_table[i].volt; + } + + return 0; +} + +int asv_init_opp_table(struct device *dev, enum asv_type_id target_type) +{ + struct asv_member *asv_mem = asv_get_mem(target_type); + struct asv_info *asv_info; + struct asv_freq_table *dvfs_table; + unsigned int i; + + if (!asv_mem) + return -EINVAL; + + asv_info = asv_mem->asv_info; + dvfs_table = asv_info->dvfs_table; + + for (i = 0; i < asv_info->nr_dvfs_level; i++) { + if (dev_pm_opp_add(dev, dvfs_table[i].freq * 1000, + dvfs_table[i].volt)) { + dev_warn(dev, "Failed to add OPP %d\n", + dvfs_table[i].freq); + continue; + } + } + + return 0; +} + +static struct asv_member *asv_init_member(struct asv_info *asv_info) +{ + struct asv_member *asv_mem; + int ret = 0; + + if (!asv_info) { + pr_err("No ASV info provided\n"); + return NULL; + } + + asv_mem = kzalloc(sizeof(*asv_mem), GFP_KERNEL); + if (!asv_mem) { + pr_err("Allocation failed for member: %s\n", asv_info->name); + return NULL; + } + + asv_mem->asv_info = kmemdup(asv_info, sizeof(*asv_info), GFP_KERNEL); + if (!asv_mem->asv_info) { + pr_err("Copying asv_info failed for member: %s\n", + asv_info->name); + kfree(asv_mem); + return NULL; + } + asv_info = asv_mem->asv_info; + + if (asv_info->ops->get_asv_group) { + ret = asv_info->ops->get_asv_group(asv_info); + if (ret) { + pr_err("get_asv_group failed for %s : %d\n", + asv_info->name, ret); + goto err; + } + } + + if (asv_info->ops->init_asv) + ret = asv_info->ops->init_asv(asv_info); + if (ret) { + pr_err("asv_init failed for %s : %d\n", + asv_info->name, ret); + goto err; + } + + /* In case of parsing table from DT, we may need to add flag to identify + DT supporting members and call init_asv_table from asv_init_opp_table( + after getting dev_node from dev,if required), instead of calling here. + */ + + if (asv_info->ops->init_asv_table) { + ret = asv_info->ops->init_asv_table(asv_info); + if (ret) { + pr_err("init_asv_table failed for %s : %d\n", + asv_info->name, ret); + goto err; + } + } + + if (!asv_info->nr_dvfs_level || !asv_info->dvfs_table) { + pr_err("No dvfs_table for %s\n", asv_info->name); + goto err; + } + + pr_info("Registered asv member: %s with group: %d", + asv_info->name, asv_info->asv_grp); + + return asv_mem; +err: + kfree(asv_mem->asv_info); + kfree(asv_mem); + return NULL; +} + +void register_asv_member(struct asv_info *list, unsigned int nr_member) +{ + struct asv_member *asv_mem; + int cnt; + + for (cnt = 0; cnt < nr_member; cnt++) { + asv_mem = asv_init_member(&list[cnt]); + + if (asv_mem) + add_asv_member(asv_mem); + } +} diff --git a/include/linux/power/asv-driver.h b/include/linux/power/asv-driver.h new file mode 100644 index 000000000000..afe072cbd451 --- /dev/null +++ b/include/linux/power/asv-driver.h @@ -0,0 +1,62 @@ +/* + * Adaptive Supply Voltage Driver Header File + * + * copyright (c) 2013 samsung electronics co., ltd. + * http://www.samsung.com/ + * + * this program is free software; you can redistribute it and/or modify + * it under the terms of the gnu general public license version 2 as + * published by the free software foundation. +*/ + +#ifndef __ASV_D_H +#define __ASV_D_H __FILE__ + +#include <linux/power/asv.h> + +struct asv_freq_table { + unsigned int freq; /* KHz */ + unsigned int volt; /* uV */ +}; + +/* struct asv_info - information of ASV member for intialisation + * + * Each member to be registered should be described using this struct + * intialised with all required information for that member. + * + * @name: Name to use for member. + * @asv_type_id: Type to identify particular member. + * @asv_ops: Callbacks which can be used for SoC specific operations. + * @nr_dvfs_level: Number of dvfs levels supported by member. + * @dvfs_table: Table containing supported ASV freqs and corresponding volts. + * @asv_grp: ASV group of member. + * @flags: ASV flags + */ +struct asv_info { + const char *name; + enum asv_type_id type; + struct asv_ops *ops; + unsigned int nr_dvfs_level; + struct asv_freq_table *dvfs_table; + unsigned int asv_grp; + unsigned int flags; +}; + +/* struct asv_ops - SoC specific operation for ASV members + * @get_asv_group - Calculates and initializes asv_grp of asv_info. + * @init_asv - SoC specific initialisation (if required) based on asv_grp. + * @init_asv_table - Initializes linear array(dvfs_table) for corresponding + * asv_grp. + * + * All ops should return 0 on sucess. + */ +struct asv_ops { + int (*init_asv)(struct asv_info *); + int (*get_asv_group)(struct asv_info *); + int (*init_asv_table)(struct asv_info *); +}; + +/* function for registering ASV members */ +void register_asv_member(struct asv_info *list, unsigned int nr_member); + +#endif /* __ASV_D_H */ diff --git a/include/linux/power/asv.h b/include/linux/power/asv.h new file mode 100644 index 000000000000..bfc4e4fa8719 --- /dev/null +++ b/include/linux/power/asv.h @@ -0,0 +1,37 @@ +/* + * Adaptive Supply Voltage Header File + * + * copyright (c) 2013 samsung electronics co., ltd. + * http://www.samsung.com/ + * + * this program is free software; you can redistribute it and/or modify + * it under the terms of the gnu general public license version 2 as + * published by the free software foundation. +*/ + +#ifndef __ASV_H +#define __ASV_H __FILE__ + +enum asv_type_id { + ASV_ARM, + ASV_INT, + ASV_MIF, + ASV_G3D, +}; + +#ifdef CONFIG_POWER_ASV +/* asv_get_volt - get the ASV for target_freq for particular target_type. + * returns 0 if target_freq is not supported + */ +extern unsigned int asv_get_volt(enum asv_type_id target_type, + unsigned int target_freq); +extern int asv_init_opp_table(struct device *dev, + enum asv_type_id target_type); +#else +static inline unsigned int asv_get_volt(enum asv_type_id target_type, + unsigned int target_freq) { return 0; } +static int asv_init_opp_table(struct device *dev, enum asv_type_id target_type) + { return 0; } + +#endif /* CONFIG_POWER_EXYNOS_AVS */ +#endif /* __ASV_H */