| Message ID | 20190404171735.12815-5-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Exynos Adaptive Supply Voltage support | expand |
Hi Sylwester, Nice work, thank you! I have a lot of minor nits all around and few more architecture-like comments. I think this should be a driver. You initialize it quite late and it is required by other drivers (cpufreq, devfreq), not by core. I would also prefer it to be slightly more generic. I mean, now you get the tables from "samsung,exynos-asv-v1" node and then, depending on many different compatibles, you do this or that. If it is samsung,exynos5800, you call 542x code. If it is hardkernel, you assume it is bin2. This will be not easily portable to other Exynos chips and other boards (e.g. you need to update the driver for new board which is forced to be in bin2). Instead all this information should come from chipid and/or DT. I could imagine that the device binds to ASV node and parses its properties. Optionally it could bind to chipid... but then you would have two devices on same node. Then depending on the of_device_id match, you have all necessary static configuration data (the same as PMU driver or all other typical drivers). Dynamic data you read from chip id. There are some comments below but I guess the code will change a lot so there is no point to do through code review. On Thu, 4 Apr 2019 at 19:22, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > The Adaptive Supply Voltage (ASV) on Exynos SoCs is a technique of adjusting > operating points of devices in order to better match actual capabilities > of the hardware and optimize power consumption. > > This patch adds common code for parsing the ASV tables from devicetree > and updating CPU OPPs. > > Also support for Exynos5422/5800 SoC is added, the Exynos5422 specific part > of the patch is partially based on code from > https://github.com/hardkernel/linux repository, branch odroidxu4-4.14.y, > files: arch/arm/mach-exynos/exynos5422-asv.[ch]. > > Tested on Odroid XU3, XU4, XU3 Lite. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/soc/samsung/Kconfig | 11 ++ > drivers/soc/samsung/Makefile | 3 + > drivers/soc/samsung/exynos-asv.c | 279 +++++++++++++++++++++++++++ > drivers/soc/samsung/exynos-asv.h | 114 +++++++++++ > drivers/soc/samsung/exynos5422-asv.c | 209 ++++++++++++++++++++ > drivers/soc/samsung/exynos5422-asv.h | 25 +++ > 6 files changed, 641 insertions(+) > create mode 100644 drivers/soc/samsung/exynos-asv.c > create mode 100644 drivers/soc/samsung/exynos-asv.h > create mode 100644 drivers/soc/samsung/exynos5422-asv.c > create mode 100644 drivers/soc/samsung/exynos5422-asv.h > > diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig > index 2905f5262197..4d121984f71a 100644 > --- a/drivers/soc/samsung/Kconfig > +++ b/drivers/soc/samsung/Kconfig > @@ -7,6 +7,17 @@ menuconfig SOC_SAMSUNG > > if SOC_SAMSUNG > > +config EXYNOS_ASV > + bool "Exynos Adaptive Supply Voltage support" if COMPILE_TEST > + depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST) Why it cannot be compile tested outside of ARM? The PMU driver has this constraint because of asm/cputype.h but you probably do not need it. > + depends on EXYNOS_CHIPID > + select EXYNOS_ASV_ARM if ARM && ARCH_EXYNOS > + > +# There is no need to enable these drivers for ARMv8 > +config EXYNOS_ASV_ARM > + bool "Exynos ASV ARMv7-specific driver extensions" if COMPILE_TEST > + depends on EXYNOS_ASV > + > config EXYNOS_CHIPID > bool "Exynos Chipid controller driver" if COMPILE_TEST > depends on ARCH_EXYNOS || COMPILE_TEST > diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile > index 3b6a8797416c..edd1d6ea064d 100644 > --- a/drivers/soc/samsung/Makefile > +++ b/drivers/soc/samsung/Makefile > @@ -1,5 +1,8 @@ > # SPDX-License-Identifier: GPL-2.0 > > +obj-$(CONFIG_EXYNOS_ASV) += exynos-asv.o > +obj-$(CONFIG_EXYNOS_ASV_ARM) += exynos5422-asv.o > + > obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o > obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o > > diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c > new file mode 100644 > index 000000000000..60532c7eb3aa > --- /dev/null > +++ b/drivers/soc/samsung/exynos-asv.c > @@ -0,0 +1,279 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. Since it is first publish date, copyright should be only 2019.... unless this was copied from hardkernel repo but then original copyright should be included as well. > + * http://www.samsung.com/ > + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> > + * > + * Samsung Exynos SoC Adaptive Supply Voltage support > + */ > + > +#include <linux/cpu.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/pm_opp.h> > +#include <linux/slab.h> > + > +#include "exynos-asv.h" > +#include "exynos5422-asv.h" > +#include "exynos5433-asv.h" > + > +#ifndef MHZ There is no existing definition, so just #define. It will also easily be noticed by preprocessor if someone adds such define later. > +#define MHZ 1000000U > +#endif > + > +static struct exynos_asv *exynos_asv; Please avoid it. This should be a driver which registers it's state as drvdata. > + > +int exynos_asv_parse_of_table(struct device_node *node, A nit: node->np because it is more popular. And shorter. > + struct exynos_asv_table *table, > + int index) > +{ > + u32 table_size, table_id = 0; > + unsigned int len1, len2 = 0; Please move the initialized variables to separate lines (first probably). > + unsigned int num_cols, num_rows; > + u32 tmp[20]; > + u32 *buf; > + int ret; > + > + ret = of_property_read_u32(node, "samsung,asv-table-id", > + &table_id); > + if (ret < 0) { > + pr_err("ASV: Missing table id in %pOF\n", node); > + return ret; > + } > + table->id = table_id; > + > + if (index >= 0 && asv_table_to_index(table_id) != index) > + return -EBADR; > + > + ret = of_property_read_u32_array(node, "samsung,asv-table-size", > + tmp, 2); > + if (ret < 0) > + return ret; > + > + num_rows = tmp[0]; > + num_cols = tmp[1]; > + if (num_rows > EXYNOS_ASV_MAX_LEVELS || > + num_cols > (EXYNOS_ASV_MAX_GROUPS + 1)) { > + pr_err("ASV: Unsupported ASV table size (%d x %d)\n", > + num_rows, num_cols); > + return -EINVAL; > + } > + > + table_size = num_rows * num_cols; > + buf = kcalloc(table_size, sizeof(u32), GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + ret = of_property_read_variable_u32_array(node, "samsung,asv-data", > + buf, 0, table_size); > + if (ret < 0) > + goto err_free_buf; > + > + len1 = ret; > + > + if (ret < table_size) { > + u32 *dst, *src; > + ret = of_property_read_variable_u32_array(node, > + "samsung,asv-common-data", > + (u32 *)tmp, 0, > + ARRAY_SIZE(tmp)); > + if (ret < 0) { > + pr_err("ASV: Not enough data for table #%x (%d x %d)\n", > + table_id, num_rows, num_cols); > + goto err_free_buf; > + } > + len2 = ret; > + > + if (len1 + ((len2 / 2) * num_cols) > table_size) { > + pr_err("ASV: Incorrect table %#x definition\n", > + table_id); > + ret = -EINVAL; > + goto err_free_buf; > + } > + /* > + * Copy data common to all ASV levels to first and second column > + * in the main buffer. We don't replicate data to further > + * columns, instead they are left initialized to 0 (invalid, > + * unused frequency value). We assume that users of the table > + * will refer to voltage data in column 1 if 0 is encountered > + * in any further column (2, 3,...). > + */ > + dst = buf + len1; > + src = tmp; > + > + while (len2 >= 2) { > + memcpy(dst, src, 2 * sizeof(u32)); > + dst += num_cols; > + src += 2; > + len2 -= 2; > + } > + } > + > + table->num_cols = num_cols; > + table->num_rows = num_rows; > + table->buf = buf; > + return 0; > + > +err_free_buf: > + kfree(buf); > + return ret; > +} > + > +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, > + int table_index) > +{ > + struct exynos_asv_subsys *subsys; > + struct exynos_asv_table table; > + struct device_node *child; > + int ret; > + > + for_each_child_of_node(np, child) { > + ret = exynos_asv_parse_of_table(child, &table, table_index); > + if (ret < 0) { > + if (ret == -EBADR) > + continue; > + of_node_put(child); > + return ret; > + } > + > + pr_debug("%s: Matching table: id: %#x at %pOF\n", > + __func__, table.id, child); > + > + switch(asv_table_to_subsys_id(table.id)) { > + case EXYNOS_ASV_SUBSYS_ID_ARM: > + subsys = &asv->arm; > + break; > + case EXYNOS_ASV_SUBSYS_ID_KFC: > + subsys = &asv->kfc; > + break; > + default: > + of_node_put(child); > + return -EINVAL; > + } > + > + subsys->num_asv_levels = table.num_rows; > + subsys->num_asv_groups = table.num_cols - 1; > + subsys->table = table; > + subsys->id = asv_table_to_subsys_id(table.id); > + } > + > + return 0; > +} > + > +static int exynos_asv_update_cpu_opps(struct device *cpu) > +{ > + struct exynos_asv_subsys *subsys = NULL; > + struct dev_pm_opp *opp; > + unsigned int opp_freq; > + int i; > + > + if (of_device_is_compatible(cpu->of_node, > + exynos_asv->arm.cpu_dt_compat)) > + subsys = &exynos_asv->arm; > + else if (of_device_is_compatible(cpu->of_node, > + exynos_asv->kfc.cpu_dt_compat)) > + subsys = &exynos_asv->kfc; > + > + if (!subsys) > + return -EINVAL; > + > + for (i = 0; i < subsys->num_asv_levels; i++) { > + unsigned int new_voltage; > + unsigned int voltage; > + int timeout = 1000; > + int err; > + > + opp_freq = exynos_asv_opp_get_frequency(subsys, i); > + > + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq * MHZ, true); > + if (IS_ERR(opp)) { > + pr_info("%s cpu%d opp%d, freq: %u missing\n", > + __func__, cpu->id, i, opp_freq); > + > + continue; > + } > + > + voltage = dev_pm_opp_get_voltage(opp); > + new_voltage = exynos_asv->opp_get_voltage(subsys, i, voltage); > + dev_pm_opp_put(opp); > + > + opp_freq *= MHZ; > + dev_pm_opp_remove(cpu, opp_freq); > + > + while (--timeout) { > + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq, true); > + if (IS_ERR(opp)) > + break; > + dev_pm_opp_put(opp); > + msleep(1); > + } > + > + err = dev_pm_opp_add(cpu, opp_freq, new_voltage); > + if (err < 0) > + pr_err("%s: Failed to add OPP %u Hz/%u uV for cpu%d\n", > + __func__, opp_freq, new_voltage, cpu->id); > + } > + > + return 0; > +} > + > +static int exynos_asv_update_opps(void) > +{ > + struct opp_table *last_opp_table = NULL; > + struct device *cpu; > + int ret, cpuid; > + > + for_each_possible_cpu(cpuid) { > + struct opp_table *opp_table; > + > + cpu = get_cpu_device(cpuid); > + if (!cpu) > + continue; > + > + opp_table = dev_pm_opp_get_opp_table(cpu); > + if (IS_ERR(opp_table)) > + continue; > + > + if (!last_opp_table || opp_table != last_opp_table) { > + last_opp_table = opp_table; > + > + ret = exynos_asv_update_cpu_opps(cpu); > + if (ret < 0) > + pr_err("%s: Couldn't udate OPPs for cpu%d\n", > + __func__, cpuid); > + } > + > + dev_pm_opp_put_opp_table(opp_table); > + } > + > + return 0; > +} > + > +static int __init exynos_asv_init(void) > +{ > + int ret; > + > + exynos_asv = kcalloc(1, sizeof(struct exynos_asv), GFP_KERNEL); sizeof(*exynos_asv) > + if (!exynos_asv) > + return -ENOMEM; > + > + if (of_machine_is_compatible("samsung,exynos5800") || > + of_machine_is_compatible("samsung,exynos5420")) > + ret = exynos5422_asv_init(exynos_asv); No, as I mentioned at beginning, bind to specific node and differentiate behavior based on of_device_id. > + else > + return 0; > + > + if (ret < 0) > + return ret; > + > + ret = exynos_asv_update_opps(); > + > + if (exynos_asv->release) > + exynos_asv->release(exynos_asv); > + > + return ret; > +} > +late_initcall(exynos_asv_init) > diff --git a/drivers/soc/samsung/exynos-asv.h b/drivers/soc/samsung/exynos-asv.h > new file mode 100644 > index 000000000000..3444b361e5e3 > --- /dev/null > +++ b/drivers/soc/samsung/exynos-asv.h > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: GPL-2.0 In headers this goes with /* */ > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. Date. > + * http://www.samsung.com/ > + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> > + * > + * Samsung Exynos SoC Adaptive Supply Voltage support > + */ > +#ifndef __EXYNOS_ASV_H > +#define __EXYNOS_ASV_H > + > +#define EXYNOS_ASV_MAX_GROUPS 16 > +#define EXYNOS_ASV_MAX_LEVELS 24 > + > +#define EXYNOS_ASV_TABLE_INDEX_MASK (0xff) > +#define EXYNOS_ASV_SUBSYS_ID_MASK (0xff << 8) > + > +#define asv_table_to_subsys_id(_id) ((_id >> 8) & 0xff) > +#define asv_table_to_index(_id) ((_id) & 0xff) > + > +#define EXYNOS_ASV_SUBSYS_ID_ARM 0x0 > +#define EXYNOS_ASV_SUBSYS_ID_EGL EXYNOS_ASV_SUBSYS_ID_ARM > +#define EXYNOS_ASV_SUBSYS_ID_KFC 0x1 > +#define EXYNOS_ASV_SUBSYS_ID_INT 0x2 > +#define EXYNOS_ASV_SUBSYS_ID_MIF 0x3 > +#define EXYNOS_ASV_SUBSYS_ID_G3D 0x4 > +#define EXYNOS_ASV_SUBSYS_ID_CAM 0x5 > +#define EXYNOS_ASV_SUBSYS_ID_MAX 0x6 > + > +struct device_node; > + > +/* HPM, IDS values to select target group */ > +struct asv_limit_entry { > + unsigned int hpm; > + unsigned int ids; > +}; > + > +struct exynos_asv_table { > + unsigned int num_rows; > + unsigned int num_cols; > + unsigned int id; > + u32 *buf; > +}; > + > +struct exynos_asv_subsys { > + int id; > + char *cpu_dt_compat; > + > + unsigned int base_volt; > + unsigned int num_asv_levels; > + unsigned int num_asv_groups; > + unsigned int offset_volt_h; > + unsigned int offset_volt_l; > + struct exynos_asv_table table; > +}; > + > +struct exynos_asv { > + struct device_node *chipid_node; > + struct exynos_asv_subsys arm; > + struct exynos_asv_subsys kfc; > + > + int (*opp_get_voltage)(struct exynos_asv_subsys *subs, int level, > + unsigned int voltage); > + void (*release)(struct exynos_asv *asv); > + > + unsigned int group; > + unsigned int table; > + > + /* TODO: Move member fields below to SoC type specific data structure */ > + unsigned int is_bin2; > + unsigned int is_special_lot; > +}; > + > +static inline u32 __exynos_asv_get_item(struct exynos_asv_table *table, > + unsigned int row, unsigned int col) > +{ > + return table->buf[row * (table->num_cols) + col]; > +} > + > +static inline u32 exynos_asv_opp_get_frequency(struct exynos_asv_subsys *subsys, > + unsigned int index) > +{ > + return __exynos_asv_get_item(&subsys->table, index, 0); > +} Empty line needed. > +/** > + * exynos_asv_parse_of_table - Parse ASV table from devicetree > + * > + * @node: DT node containing the table > + * @table: The parsed table data > + * @index: Used to select table with specific index to parse, if >= 0. > + * This argument is ignored if the value is less than 0. > + * > + * Returns: 0 on success, or negative value on failure. EBADR is returned > + * when @index is >= 0 but the index value of the parsed table ID is different > + * than @index. > + */ > +int exynos_asv_parse_of_table(struct device_node *node, > + struct exynos_asv_table *table, > + int index); > + > +/** > + * exynos_asv_parse_cpu_tables - Parse ARM and KFC ASV tables from DT > + * > + * @asv: data structure to store the parsed data to > + * @node: DT node containing the tables > + * @index: Used to select table with specific index to parse, if >= 0. > + * This argument is ignored if the value is less than 0. > + * > + * Returns: 0 on success, or negative value on failure. > + */ > +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, > + int table_index); > + > +#endif /* __EXYNOS_ASV_H */ > diff --git a/drivers/soc/samsung/exynos5422-asv.c b/drivers/soc/samsung/exynos5422-asv.c > new file mode 100644 > index 000000000000..f0b7bdd873a9 > --- /dev/null > +++ b/drivers/soc/samsung/exynos5422-asv.c > @@ -0,0 +1,209 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. The same - copyright date. > + * http://www.samsung.com/ > + * > + * Samsung Exynos 5422 SoC Adaptive Supply Voltage support > + */ > + > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/slab.h> > + > +#include "exynos-asv.h" > +#include "exynos-chipid.h" > + > +#define EXYNOS5422_NUM_ASV_GROUPS 14 > + > +static struct exynos_asv *exynos_asv; > + > +static int exynos5422_asv_get_table(void) > +{ > + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > + > + return (reg >> EXYNOS5422_TABLE_OFFSET) & EXYNOS5422_TABLE_MASK; > +} > + > +static int exynos5422_asv_check_bin2(void) > +{ > + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > + > + return (reg >> EXYNOS5422_BIN2_OFFSET) & EXYNOS5422_BIN2_MASK; > +} > + > +static bool exynos5422_asv_check_lot_id(void) > +{ > + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > + > + return ((reg >> EXYNOS5422_USESG_OFFSET) & EXYNOS5422_USESG_MASK); > +} > + > +static const struct asv_limit_entry __asv_limits[EXYNOS5422_NUM_ASV_GROUPS] = { > + { 13, 55 }, > + { 21, 65 }, > + { 25, 69 }, > + { 30, 72 }, > + { 36, 74 }, > + { 43, 76 }, > + { 51, 78 }, > + { 65, 80 }, > + { 81, 82 }, > + { 98, 84 }, > + { 119, 87 }, > + { 135, 89 }, > + { 150, 92 }, > + { 999, 999 }, > +}; > + > +static int exynos5422_asv_get_group(struct exynos_asv *asv) > +{ > + unsigned int pkgid_reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > + unsigned int auxi_reg = exynos_chipid_read(EXYNOS_CHIPID_AUX_INFO); > + int hpm, ids, i; > + > + if (asv->is_special_lot) { > + u32 sga = (pkgid_reg >> EXYNOS5422_SG_A_OFFSET) & > + EXYNOS5422_SG_A_MASK; > + > + u32 sgb = (pkgid_reg >> EXYNOS5422_SG_B_OFFSET) & > + EXYNOS5422_SG_B_MASK; > + > + if ((pkgid_reg >> EXYNOS5422_SG_BSIGN_OFFSET) & > + EXYNOS5422_SG_BSIGN_MASK) > + return sga + sgb; > + else > + return sga - sgb; > + } > + > + hpm = (auxi_reg >> EXYNOS5422_TMCB_OFFSET) & EXYNOS5422_TMCB_MASK; > + ids = (pkgid_reg >> EXYNOS5422_IDS_OFFSET) & EXYNOS5422_IDS_MASK; > + > + for (i = 0; i < EXYNOS5422_NUM_ASV_GROUPS; i++) { > + if (ids <= __asv_limits[i].ids) > + break; > + if (hpm <= __asv_limits[i].hpm) > + break; > + } > + if (i < EXYNOS5422_NUM_ASV_GROUPS) > + return i; > + > + return 0; > +} > + > +static int __asv_offset_voltage(unsigned int index) > +{ > + static const unsigned int offset_table[] = { 12500, 50000, 25000 }; > + > + if (index == 0 || index > 3) > + return 0; > + > + return offset_table[index - 1]; > +} > + > +static void exynos5422_asv_offset_voltage_setup(struct exynos_asv *asv) > +{ > + struct exynos_asv_subsys *subsys; > + unsigned int reg, value; > + > + reg = exynos_chipid_read(EXYNOS_CHIPID_AUX_INFO); > + > + /* ARM offset voltage setup */ > + subsys = &asv->arm; > + > + subsys->base_volt = 1000000; > + > + value = (reg >> EXYNOS5422_ARM_UP_OFFSET) & EXYNOS5422_ARM_UP_MASK; > + subsys->offset_volt_h = __asv_offset_voltage(value); > + > + value = (reg >> EXYNOS5422_ARM_DN_OFFSET) & EXYNOS5422_ARM_DN_MASK; > + subsys->offset_volt_l = __asv_offset_voltage(value); > + > + /* KFC offset voltage setup */ > + subsys = &asv->kfc; > + > + subsys->base_volt = 1000000; > + > + value = (reg >> EXYNOS5422_KFC_UP_OFFSET) & EXYNOS5422_KFC_UP_MASK; > + subsys->offset_volt_h = __asv_offset_voltage(value); > + > + value = (reg >> EXYNOS5422_KFC_DN_OFFSET) & EXYNOS5422_KFC_DN_MASK; > + subsys->offset_volt_l = __asv_offset_voltage(value); > +} > + > +static int exynos5422_asv_opp_get_voltage(struct exynos_asv_subsys *subsys, > + int level, unsigned int volt) > +{ > + unsigned int asv_volt; > + > + if (level >= subsys->num_asv_levels) > + return volt; > + > + asv_volt = __exynos_asv_get_item(&subsys->table, level, > + exynos_asv->group + 1); > + if (asv_volt == 0 && exynos_asv->group > 1) > + asv_volt = __exynos_asv_get_item(&subsys->table, level, 1); > + > + if (volt > subsys->base_volt) > + asv_volt += subsys->offset_volt_h; > + else > + asv_volt += subsys->offset_volt_l; > + > + return asv_volt; > +} > + > +static void __init exynos5422_asv_release(struct exynos_asv *asv) > +{ > + kfree(asv->arm.table.buf); > + asv->arm.table.buf = NULL; > + kfree(asv->kfc.table.buf); > + asv->kfc.table.buf = NULL; > +} > + > +int __init exynos5422_asv_init(struct exynos_asv *asv) > +{ > + struct device_node *node; > + unsigned int table_index; > + int ret; > + > + exynos_asv = asv; > + > + asv->is_bin2 = exynos5422_asv_check_bin2(); > + asv->is_special_lot = exynos5422_asv_check_lot_id(); > + > + if (of_machine_is_compatible("hardkernel,odroid-xu3-lite")) { > + asv->is_bin2 = true; > + asv->is_special_lot = false; > + } > + > + asv->group = exynos5422_asv_get_group(asv); > + asv->table = exynos5422_asv_get_table(); > + > + exynos5422_asv_offset_voltage_setup(asv); > + > + node = of_find_compatible_node(NULL, NULL, "samsung,exynos-asv-v1"); > + if (!node) > + return -ENODEV; > + > + if (!asv->is_bin2) { > + if (asv->table == 2 || asv->table == 3) > + table_index = asv->table; > + else > + table_index = 0; > + } else { > + table_index = 4; > + } > + > + ret = exynos_asv_parse_cpu_tables(asv, node, table_index); > + of_node_put(node); > + if (ret < 0) > + return ret; > + > + asv->arm.cpu_dt_compat = "arm,cortex-a15"; > + asv->kfc.cpu_dt_compat = "arm,cortex-a7"; > + > + asv->opp_get_voltage = exynos5422_asv_opp_get_voltage; > + asv->release = exynos5422_asv_release; > + > + return ret; > +} > diff --git a/drivers/soc/samsung/exynos5422-asv.h b/drivers/soc/samsung/exynos5422-asv.h > new file mode 100644 > index 000000000000..f0d9107d6d0a > --- /dev/null > +++ b/drivers/soc/samsung/exynos5422-asv.h > @@ -0,0 +1,25 @@ > +// SPDX-License-Identifier: GPL-2.0 /**/ needed > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. Date Best regards, Krzysztof > + * http://www.samsung.com/ > + * > + * Samsung Exynos 5422 SoC Adaptive Supply Voltage support > + */ > + > +#ifndef __EXYNOS5422_ASV_H > +#define __EXYNOS5422_ASV_H > + > +#include <linux/errno.h> > + > +struct exynos_asv; > + > +#ifdef CONFIG_EXYNOS_ASV_ARM > +int exynos5422_asv_init(struct exynos_asv *asv); > +#else > +static inline int exynos5422_asv_init(struct exynos_asv *asv) > +{ > + return -ENOTSUPP; > +} > +#endif > + > +#endif /* __EXYNOS5422_ASV_H */ > -- > 2.17.1 >
On 4/5/19 12:24, Krzysztof Kozlowski wrote: > I have a lot of minor nits all around and few more architecture-like comments. > > I think this should be a driver. You initialize it quite late and it > is required by other drivers (cpufreq, devfreq), not by core. Thanks for your comments, it helped me to finally get around and convert this code to proper driver. The advantage is that it can be more reliably ensured that the ASV driver updates OPPs properly after they have been parsed from DT by cpufreq-dt or other drivers. And if we add support for the Body Bias through regulator API it would be easier to acquire the regulator resource, i.e. deferred probing could ensure proper driver probe ordering. > I would also prefer it to be slightly more generic. I mean, now you > get the tables from "samsung,exynos-asv-v1" node and then, depending > on many different compatibles, you do this or that. If it is > samsung,exynos5800, you call 542x code. If it is hardkernel, you > assume it is bin2. This will be not easily portable to other Exynos > chips and other boards (e.g. you need to update the driver for new > board which is forced to be in bin2). The bin2 part for Odroid XU3 Lite is really a hack and I wish it was not there. It seems there is not enough data fused in the SoC to properly detect chips used on XU3 Lite. > Instead all this information should come from chipid and/or DT. I Perhaps we could try to be more generic but I'm afraid in practice we will need to have some logic coded in the driver per each SoC type anyway. We could probably add DT property in chipid node to provide missing data that is normally read from CHIPID registers. The BIN2 flag is really about forcing specific ASV table. The "special group" quirk means different thing for each SoC type, in case of exynos5422 it's selecting different method of the ASV group decoding. It's a bit hard to try to put anything in DT with sparse or missing documentation of such quirks. > could imagine that the device binds to ASV node and parses its > properties. Optionally it could bind to chipid... but then you would The ASV node on older SoCs doesn't correspond to real hardware so we shouldn't try to bind drivers to it I'd say. The chipid node seems a better alternative to me. I'm thinking about dropping 'asv' node completely. But then the table sub-nodes would need to be put into chipid node. > have two devices on same node. Then depending on the of_device_id > match, you have all necessary static configuration data (the same as > PMU driver or all other typical drivers). Dynamic data you read from > chip id. I will post next version of the patch series and we could continue discussion from there, I have switched to of_device_id matching but left per SoC init function. > On Thu, 4 Apr 2019 at 19:22, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: >> >> The Adaptive Supply Voltage (ASV) on Exynos SoCs is a technique of adjusting >> operating points of devices in order to better match actual capabilities >> of the hardware and optimize power consumption. >> >> This patch adds common code for parsing the ASV tables from devicetree >> and updating CPU OPPs. >> >> Also support for Exynos5422/5800 SoC is added, the Exynos5422 specific part >> of the patch is partially based on code from >> https://github.com/hardkernel/linux repository, branch odroidxu4-4.14.y, >> files: arch/arm/mach-exynos/exynos5422-asv.[ch]. >> >> Tested on Odroid XU3, XU4, XU3 Lite. >> >> Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> --- >> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig >> index 2905f5262197..4d121984f71a 100644 >> --- a/drivers/soc/samsung/Kconfig >> +++ b/drivers/soc/samsung/Kconfig >> @@ -7,6 +7,17 @@ menuconfig SOC_SAMSUNG >> >> if SOC_SAMSUNG >> >> +config EXYNOS_ASV >> + bool "Exynos Adaptive Supply Voltage support" if COMPILE_TEST >> + depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST) > > Why it cannot be compile tested outside of ARM? The PMU driver has > this constraint because of asm/cputype.h but you probably do not need > it. Yes, it's not needed, I will drop the (ARM || ARM64) part in next iteration. >> diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c >> new file mode 100644 >> index 000000000000..60532c7eb3aa >> --- /dev/null >> +++ b/drivers/soc/samsung/exynos-asv.c >> @@ -0,0 +1,279 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. > > Since it is first publish date, copyright should be only 2019.... > unless this was copied from hardkernel repo but then original > copyright should be included as well. I will change it to 2019. I have written code in this file from scratch by myself. Some reused code is in exynos5422-asv.c, I will copy the original copyright notice there, however it is same Samsung's copyright notice, only with 2012 date.
On Thu, 4 Apr 2019 at 19:22, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > The Adaptive Supply Voltage (ASV) on Exynos SoCs is a technique of adjusting > operating points of devices in order to better match actual capabilities > of the hardware and optimize power consumption. > > This patch adds common code for parsing the ASV tables from devicetree > and updating CPU OPPs. > > Also support for Exynos5422/5800 SoC is added, the Exynos5422 specific part > of the patch is partially based on code from > https://github.com/hardkernel/linux repository, branch odroidxu4-4.14.y, > files: arch/arm/mach-exynos/exynos5422-asv.[ch]. > > Tested on Odroid XU3, XU4, XU3 Lite. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/soc/samsung/Kconfig | 11 ++ > drivers/soc/samsung/Makefile | 3 + > drivers/soc/samsung/exynos-asv.c | 279 +++++++++++++++++++++++++++ > drivers/soc/samsung/exynos-asv.h | 114 +++++++++++ > drivers/soc/samsung/exynos5422-asv.c | 209 ++++++++++++++++++++ > drivers/soc/samsung/exynos5422-asv.h | 25 +++ > 6 files changed, 641 insertions(+) > create mode 100644 drivers/soc/samsung/exynos-asv.c > create mode 100644 drivers/soc/samsung/exynos-asv.h > create mode 100644 drivers/soc/samsung/exynos5422-asv.c > create mode 100644 drivers/soc/samsung/exynos5422-asv.h > > diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig > index 2905f5262197..4d121984f71a 100644 > --- a/drivers/soc/samsung/Kconfig > +++ b/drivers/soc/samsung/Kconfig > @@ -7,6 +7,17 @@ menuconfig SOC_SAMSUNG > > if SOC_SAMSUNG > > +config EXYNOS_ASV > + bool "Exynos Adaptive Supply Voltage support" if COMPILE_TEST > + depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST) > + depends on EXYNOS_CHIPID > + select EXYNOS_ASV_ARM if ARM && ARCH_EXYNOS > + > +# There is no need to enable these drivers for ARMv8 > +config EXYNOS_ASV_ARM > + bool "Exynos ASV ARMv7-specific driver extensions" if COMPILE_TEST > + depends on EXYNOS_ASV > + > config EXYNOS_CHIPID > bool "Exynos Chipid controller driver" if COMPILE_TEST > depends on ARCH_EXYNOS || COMPILE_TEST > diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile > index 3b6a8797416c..edd1d6ea064d 100644 > --- a/drivers/soc/samsung/Makefile > +++ b/drivers/soc/samsung/Makefile > @@ -1,5 +1,8 @@ > # SPDX-License-Identifier: GPL-2.0 > > +obj-$(CONFIG_EXYNOS_ASV) += exynos-asv.o > +obj-$(CONFIG_EXYNOS_ASV_ARM) += exynos5422-asv.o > + > obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o > obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o > > diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c > new file mode 100644 > index 000000000000..60532c7eb3aa > --- /dev/null > +++ b/drivers/soc/samsung/exynos-asv.c > @@ -0,0 +1,279 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. > + * http://www.samsung.com/ > + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> > + * > + * Samsung Exynos SoC Adaptive Supply Voltage support > + */ > + > +#include <linux/cpu.h> > +#include <linux/delay.h> > +#include <linux/device.h> > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/pm_opp.h> > +#include <linux/slab.h> > + > +#include "exynos-asv.h" > +#include "exynos5422-asv.h" > +#include "exynos5433-asv.h" > + > +#ifndef MHZ > +#define MHZ 1000000U > +#endif > + > +static struct exynos_asv *exynos_asv; > + > +int exynos_asv_parse_of_table(struct device_node *node, > + struct exynos_asv_table *table, > + int index) > +{ > + u32 table_size, table_id = 0; > + unsigned int len1, len2 = 0; > + unsigned int num_cols, num_rows; > + u32 tmp[20]; > + u32 *buf; > + int ret; > + > + ret = of_property_read_u32(node, "samsung,asv-table-id", > + &table_id); > + if (ret < 0) { > + pr_err("ASV: Missing table id in %pOF\n", node); > + return ret; > + } > + table->id = table_id; > + > + if (index >= 0 && asv_table_to_index(table_id) != index) > + return -EBADR; > + > + ret = of_property_read_u32_array(node, "samsung,asv-table-size", > + tmp, 2); > + if (ret < 0) > + return ret; > + > + num_rows = tmp[0]; > + num_cols = tmp[1]; > + if (num_rows > EXYNOS_ASV_MAX_LEVELS || > + num_cols > (EXYNOS_ASV_MAX_GROUPS + 1)) { > + pr_err("ASV: Unsupported ASV table size (%d x %d)\n", > + num_rows, num_cols); > + return -EINVAL; > + } > + > + table_size = num_rows * num_cols; > + buf = kcalloc(table_size, sizeof(u32), GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + ret = of_property_read_variable_u32_array(node, "samsung,asv-data", > + buf, 0, table_size); > + if (ret < 0) > + goto err_free_buf; > + > + len1 = ret; > + > + if (ret < table_size) { > + u32 *dst, *src; > + ret = of_property_read_variable_u32_array(node, > + "samsung,asv-common-data", > + (u32 *)tmp, 0, > + ARRAY_SIZE(tmp)); > + if (ret < 0) { > + pr_err("ASV: Not enough data for table #%x (%d x %d)\n", > + table_id, num_rows, num_cols); > + goto err_free_buf; > + } > + len2 = ret; > + > + if (len1 + ((len2 / 2) * num_cols) > table_size) { > + pr_err("ASV: Incorrect table %#x definition\n", > + table_id); > + ret = -EINVAL; > + goto err_free_buf; > + } > + /* > + * Copy data common to all ASV levels to first and second column > + * in the main buffer. We don't replicate data to further > + * columns, instead they are left initialized to 0 (invalid, > + * unused frequency value). We assume that users of the table > + * will refer to voltage data in column 1 if 0 is encountered > + * in any further column (2, 3,...). > + */ > + dst = buf + len1; > + src = tmp; > + > + while (len2 >= 2) { > + memcpy(dst, src, 2 * sizeof(u32)); > + dst += num_cols; > + src += 2; > + len2 -= 2; > + } > + } > + > + table->num_cols = num_cols; > + table->num_rows = num_rows; > + table->buf = buf; > + return 0; > + > +err_free_buf: > + kfree(buf); > + return ret; > +} > + > +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, > + int table_index) > +{ > + struct exynos_asv_subsys *subsys; > + struct exynos_asv_table table; > + struct device_node *child; > + int ret; > + > + for_each_child_of_node(np, child) { > + ret = exynos_asv_parse_of_table(child, &table, table_index); > + if (ret < 0) { > + if (ret == -EBADR) > + continue; > + of_node_put(child); > + return ret; > + } > + > + pr_debug("%s: Matching table: id: %#x at %pOF\n", > + __func__, table.id, child); > + > + switch(asv_table_to_subsys_id(table.id)) { > + case EXYNOS_ASV_SUBSYS_ID_ARM: > + subsys = &asv->arm; > + break; > + case EXYNOS_ASV_SUBSYS_ID_KFC: > + subsys = &asv->kfc; > + break; > + default: > + of_node_put(child); > + return -EINVAL; > + } > + > + subsys->num_asv_levels = table.num_rows; > + subsys->num_asv_groups = table.num_cols - 1; > + subsys->table = table; > + subsys->id = asv_table_to_subsys_id(table.id); > + } > + > + return 0; > +} > + > +static int exynos_asv_update_cpu_opps(struct device *cpu) > +{ > + struct exynos_asv_subsys *subsys = NULL; > + struct dev_pm_opp *opp; > + unsigned int opp_freq; > + int i; > + > + if (of_device_is_compatible(cpu->of_node, > + exynos_asv->arm.cpu_dt_compat)) > + subsys = &exynos_asv->arm; > + else if (of_device_is_compatible(cpu->of_node, > + exynos_asv->kfc.cpu_dt_compat)) > + subsys = &exynos_asv->kfc; > + > + if (!subsys) > + return -EINVAL; > + > + for (i = 0; i < subsys->num_asv_levels; i++) { > + unsigned int new_voltage; > + unsigned int voltage; > + int timeout = 1000; > + int err; > + > + opp_freq = exynos_asv_opp_get_frequency(subsys, i); > + > + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq * MHZ, true); > + if (IS_ERR(opp)) { > + pr_info("%s cpu%d opp%d, freq: %u missing\n", > + __func__, cpu->id, i, opp_freq); > + > + continue; > + } > + > + voltage = dev_pm_opp_get_voltage(opp); > + new_voltage = exynos_asv->opp_get_voltage(subsys, i, voltage); > + dev_pm_opp_put(opp); > + > + opp_freq *= MHZ; > + dev_pm_opp_remove(cpu, opp_freq); > + > + while (--timeout) { > + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq, true); > + if (IS_ERR(opp)) > + break; > + dev_pm_opp_put(opp); > + msleep(1); > + } > + > + err = dev_pm_opp_add(cpu, opp_freq, new_voltage); > + if (err < 0) > + pr_err("%s: Failed to add OPP %u Hz/%u uV for cpu%d\n", > + __func__, opp_freq, new_voltage, cpu->id); > + } > + > + return 0; > +} > + > +static int exynos_asv_update_opps(void) > +{ > + struct opp_table *last_opp_table = NULL; > + struct device *cpu; > + int ret, cpuid; > + > + for_each_possible_cpu(cpuid) { > + struct opp_table *opp_table; > + > + cpu = get_cpu_device(cpuid); > + if (!cpu) > + continue; > + > + opp_table = dev_pm_opp_get_opp_table(cpu); > + if (IS_ERR(opp_table)) > + continue; > + > + if (!last_opp_table || opp_table != last_opp_table) { > + last_opp_table = opp_table; > + > + ret = exynos_asv_update_cpu_opps(cpu); > + if (ret < 0) > + pr_err("%s: Couldn't udate OPPs for cpu%d\n", > + __func__, cpuid); > + } > + > + dev_pm_opp_put_opp_table(opp_table); > + } > + > + return 0; > +} > + > +static int __init exynos_asv_init(void) > +{ > + int ret; > + > + exynos_asv = kcalloc(1, sizeof(struct exynos_asv), GFP_KERNEL); > + if (!exynos_asv) > + return -ENOMEM; > + > + if (of_machine_is_compatible("samsung,exynos5800") || > + of_machine_is_compatible("samsung,exynos5420")) > + ret = exynos5422_asv_init(exynos_asv); > + else > + return 0; > + > + if (ret < 0) > + return ret; > + > + ret = exynos_asv_update_opps(); > + > + if (exynos_asv->release) > + exynos_asv->release(exynos_asv); > + > + return ret; > +} > +late_initcall(exynos_asv_init) > diff --git a/drivers/soc/samsung/exynos-asv.h b/drivers/soc/samsung/exynos-asv.h > new file mode 100644 > index 000000000000..3444b361e5e3 > --- /dev/null > +++ b/drivers/soc/samsung/exynos-asv.h > @@ -0,0 +1,114 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. > + * http://www.samsung.com/ > + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> > + * > + * Samsung Exynos SoC Adaptive Supply Voltage support > + */ > +#ifndef __EXYNOS_ASV_H > +#define __EXYNOS_ASV_H > + > +#define EXYNOS_ASV_MAX_GROUPS 16 > +#define EXYNOS_ASV_MAX_LEVELS 24 > + > +#define EXYNOS_ASV_TABLE_INDEX_MASK (0xff) > +#define EXYNOS_ASV_SUBSYS_ID_MASK (0xff << 8) > + > +#define asv_table_to_subsys_id(_id) ((_id >> 8) & 0xff) > +#define asv_table_to_index(_id) ((_id) & 0xff) > + > +#define EXYNOS_ASV_SUBSYS_ID_ARM 0x0 > +#define EXYNOS_ASV_SUBSYS_ID_EGL EXYNOS_ASV_SUBSYS_ID_ARM > +#define EXYNOS_ASV_SUBSYS_ID_KFC 0x1 > +#define EXYNOS_ASV_SUBSYS_ID_INT 0x2 > +#define EXYNOS_ASV_SUBSYS_ID_MIF 0x3 > +#define EXYNOS_ASV_SUBSYS_ID_G3D 0x4 > +#define EXYNOS_ASV_SUBSYS_ID_CAM 0x5 > +#define EXYNOS_ASV_SUBSYS_ID_MAX 0x6 > + > +struct device_node; > + > +/* HPM, IDS values to select target group */ > +struct asv_limit_entry { > + unsigned int hpm; > + unsigned int ids; > +}; > + > +struct exynos_asv_table { > + unsigned int num_rows; > + unsigned int num_cols; > + unsigned int id; > + u32 *buf; > +}; > + > +struct exynos_asv_subsys { > + int id; > + char *cpu_dt_compat; > + > + unsigned int base_volt; > + unsigned int num_asv_levels; > + unsigned int num_asv_groups; > + unsigned int offset_volt_h; > + unsigned int offset_volt_l; > + struct exynos_asv_table table; > +}; > + > +struct exynos_asv { > + struct device_node *chipid_node; > + struct exynos_asv_subsys arm; > + struct exynos_asv_subsys kfc; > + > + int (*opp_get_voltage)(struct exynos_asv_subsys *subs, int level, > + unsigned int voltage); > + void (*release)(struct exynos_asv *asv); > + > + unsigned int group; > + unsigned int table; > + > + /* TODO: Move member fields below to SoC type specific data structure */ > + unsigned int is_bin2; > + unsigned int is_special_lot; > +}; > + > +static inline u32 __exynos_asv_get_item(struct exynos_asv_table *table, > + unsigned int row, unsigned int col) > +{ > + return table->buf[row * (table->num_cols) + col]; > +} > + > +static inline u32 exynos_asv_opp_get_frequency(struct exynos_asv_subsys *subsys, > + unsigned int index) > +{ > + return __exynos_asv_get_item(&subsys->table, index, 0); > +} > +/** > + * exynos_asv_parse_of_table - Parse ASV table from devicetree > + * > + * @node: DT node containing the table > + * @table: The parsed table data > + * @index: Used to select table with specific index to parse, if >= 0. > + * This argument is ignored if the value is less than 0. > + * > + * Returns: 0 on success, or negative value on failure. EBADR is returned > + * when @index is >= 0 but the index value of the parsed table ID is different > + * than @index. > + */ > +int exynos_asv_parse_of_table(struct device_node *node, > + struct exynos_asv_table *table, > + int index); > + > +/** > + * exynos_asv_parse_cpu_tables - Parse ARM and KFC ASV tables from DT > + * > + * @asv: data structure to store the parsed data to > + * @node: DT node containing the tables > + * @index: Used to select table with specific index to parse, if >= 0. > + * This argument is ignored if the value is less than 0. > + * > + * Returns: 0 on success, or negative value on failure. > + */ > +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, > + int table_index); > + > +#endif /* __EXYNOS_ASV_H */ > diff --git a/drivers/soc/samsung/exynos5422-asv.c b/drivers/soc/samsung/exynos5422-asv.c > new file mode 100644 > index 000000000000..f0b7bdd873a9 > --- /dev/null > +++ b/drivers/soc/samsung/exynos5422-asv.c > @@ -0,0 +1,209 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. > + * http://www.samsung.com/ > + * > + * Samsung Exynos 5422 SoC Adaptive Supply Voltage support > + */ > + > +#include <linux/errno.h> > +#include <linux/init.h> > +#include <linux/of.h> > +#include <linux/slab.h> > + > +#include "exynos-asv.h" > +#include "exynos-chipid.h" > + > +#define EXYNOS5422_NUM_ASV_GROUPS 14 > + > +static struct exynos_asv *exynos_asv; > + > +static int exynos5422_asv_get_table(void) > +{ > + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); One more thought: you read this register multiple times in the same function. I think it is not needed - just read once, store the value and use the helpers to parse it. Best regards, Krzysztof
On 4/23/19 12:50, Krzysztof Kozlowski wrote: >> +static int exynos5422_asv_get_table(void) >> +{ >> + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > > One more thought: you read this register multiple times in the same > function. I think it is not needed - just read once, store the value > and use the helpers to parse it. Yes, I have noticed that as well. I'm not sure though if it is worth to additionally cache registers manually like this when we use the regmap. I have already converted that code to use the regmap API for v2. And these are barely few registers reads at the driver initialization time, not any hot path.
On Wed, 24 Apr 2019 at 10:10, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > > On 4/23/19 12:50, Krzysztof Kozlowski wrote: > >> +static int exynos5422_asv_get_table(void) > >> +{ > >> + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); > > > > One more thought: you read this register multiple times in the same > > function. I think it is not needed - just read once, store the value > > and use the helpers to parse it. > > Yes, I have noticed that as well. I'm not sure though if it is worth > to additionally cache registers manually like this when we use the > regmap. I have already converted that code to use the regmap API for > v2. And these are barely few registers reads at the driver > initialization time, not any hot path. By default regmap does not use caching so there is no benefit out of it. In the same time reading with regmap involves its layer of abstraction with locks, indirect calls etc... I agree that there is no point for implementing specific "caching" but with the same code readability/simplicity you can have it without multiple regmap accesses one after another. Consider this: unsigned int pkgid = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); asv->table = exynos5422_asv_get_table(pkgid); asv->is_bin2 = exynos5422_asv_check_bin2(pkgid); (and probably renaming the helpers) The code is equivalent. The same readability. Best regards, Krzysztof
diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig index 2905f5262197..4d121984f71a 100644 --- a/drivers/soc/samsung/Kconfig +++ b/drivers/soc/samsung/Kconfig @@ -7,6 +7,17 @@ menuconfig SOC_SAMSUNG if SOC_SAMSUNG +config EXYNOS_ASV + bool "Exynos Adaptive Supply Voltage support" if COMPILE_TEST + depends on ARCH_EXYNOS || ((ARM || ARM64) && COMPILE_TEST) + depends on EXYNOS_CHIPID + select EXYNOS_ASV_ARM if ARM && ARCH_EXYNOS + +# There is no need to enable these drivers for ARMv8 +config EXYNOS_ASV_ARM + bool "Exynos ASV ARMv7-specific driver extensions" if COMPILE_TEST + depends on EXYNOS_ASV + config EXYNOS_CHIPID bool "Exynos Chipid controller driver" if COMPILE_TEST depends on ARCH_EXYNOS || COMPILE_TEST diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile index 3b6a8797416c..edd1d6ea064d 100644 --- a/drivers/soc/samsung/Makefile +++ b/drivers/soc/samsung/Makefile @@ -1,5 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 +obj-$(CONFIG_EXYNOS_ASV) += exynos-asv.o +obj-$(CONFIG_EXYNOS_ASV_ARM) += exynos5422-asv.o + obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o diff --git a/drivers/soc/samsung/exynos-asv.c b/drivers/soc/samsung/exynos-asv.c new file mode 100644 index 000000000000..60532c7eb3aa --- /dev/null +++ b/drivers/soc/samsung/exynos-asv.c @@ -0,0 +1,279 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. + * http://www.samsung.com/ + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> + * + * Samsung Exynos SoC Adaptive Supply Voltage support + */ + +#include <linux/cpu.h> +#include <linux/delay.h> +#include <linux/device.h> +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/of.h> +#include <linux/pm_opp.h> +#include <linux/slab.h> + +#include "exynos-asv.h" +#include "exynos5422-asv.h" +#include "exynos5433-asv.h" + +#ifndef MHZ +#define MHZ 1000000U +#endif + +static struct exynos_asv *exynos_asv; + +int exynos_asv_parse_of_table(struct device_node *node, + struct exynos_asv_table *table, + int index) +{ + u32 table_size, table_id = 0; + unsigned int len1, len2 = 0; + unsigned int num_cols, num_rows; + u32 tmp[20]; + u32 *buf; + int ret; + + ret = of_property_read_u32(node, "samsung,asv-table-id", + &table_id); + if (ret < 0) { + pr_err("ASV: Missing table id in %pOF\n", node); + return ret; + } + table->id = table_id; + + if (index >= 0 && asv_table_to_index(table_id) != index) + return -EBADR; + + ret = of_property_read_u32_array(node, "samsung,asv-table-size", + tmp, 2); + if (ret < 0) + return ret; + + num_rows = tmp[0]; + num_cols = tmp[1]; + if (num_rows > EXYNOS_ASV_MAX_LEVELS || + num_cols > (EXYNOS_ASV_MAX_GROUPS + 1)) { + pr_err("ASV: Unsupported ASV table size (%d x %d)\n", + num_rows, num_cols); + return -EINVAL; + } + + table_size = num_rows * num_cols; + buf = kcalloc(table_size, sizeof(u32), GFP_KERNEL); + if (!buf) + return -ENOMEM; + + ret = of_property_read_variable_u32_array(node, "samsung,asv-data", + buf, 0, table_size); + if (ret < 0) + goto err_free_buf; + + len1 = ret; + + if (ret < table_size) { + u32 *dst, *src; + ret = of_property_read_variable_u32_array(node, + "samsung,asv-common-data", + (u32 *)tmp, 0, + ARRAY_SIZE(tmp)); + if (ret < 0) { + pr_err("ASV: Not enough data for table #%x (%d x %d)\n", + table_id, num_rows, num_cols); + goto err_free_buf; + } + len2 = ret; + + if (len1 + ((len2 / 2) * num_cols) > table_size) { + pr_err("ASV: Incorrect table %#x definition\n", + table_id); + ret = -EINVAL; + goto err_free_buf; + } + /* + * Copy data common to all ASV levels to first and second column + * in the main buffer. We don't replicate data to further + * columns, instead they are left initialized to 0 (invalid, + * unused frequency value). We assume that users of the table + * will refer to voltage data in column 1 if 0 is encountered + * in any further column (2, 3,...). + */ + dst = buf + len1; + src = tmp; + + while (len2 >= 2) { + memcpy(dst, src, 2 * sizeof(u32)); + dst += num_cols; + src += 2; + len2 -= 2; + } + } + + table->num_cols = num_cols; + table->num_rows = num_rows; + table->buf = buf; + return 0; + +err_free_buf: + kfree(buf); + return ret; +} + +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, + int table_index) +{ + struct exynos_asv_subsys *subsys; + struct exynos_asv_table table; + struct device_node *child; + int ret; + + for_each_child_of_node(np, child) { + ret = exynos_asv_parse_of_table(child, &table, table_index); + if (ret < 0) { + if (ret == -EBADR) + continue; + of_node_put(child); + return ret; + } + + pr_debug("%s: Matching table: id: %#x at %pOF\n", + __func__, table.id, child); + + switch(asv_table_to_subsys_id(table.id)) { + case EXYNOS_ASV_SUBSYS_ID_ARM: + subsys = &asv->arm; + break; + case EXYNOS_ASV_SUBSYS_ID_KFC: + subsys = &asv->kfc; + break; + default: + of_node_put(child); + return -EINVAL; + } + + subsys->num_asv_levels = table.num_rows; + subsys->num_asv_groups = table.num_cols - 1; + subsys->table = table; + subsys->id = asv_table_to_subsys_id(table.id); + } + + return 0; +} + +static int exynos_asv_update_cpu_opps(struct device *cpu) +{ + struct exynos_asv_subsys *subsys = NULL; + struct dev_pm_opp *opp; + unsigned int opp_freq; + int i; + + if (of_device_is_compatible(cpu->of_node, + exynos_asv->arm.cpu_dt_compat)) + subsys = &exynos_asv->arm; + else if (of_device_is_compatible(cpu->of_node, + exynos_asv->kfc.cpu_dt_compat)) + subsys = &exynos_asv->kfc; + + if (!subsys) + return -EINVAL; + + for (i = 0; i < subsys->num_asv_levels; i++) { + unsigned int new_voltage; + unsigned int voltage; + int timeout = 1000; + int err; + + opp_freq = exynos_asv_opp_get_frequency(subsys, i); + + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq * MHZ, true); + if (IS_ERR(opp)) { + pr_info("%s cpu%d opp%d, freq: %u missing\n", + __func__, cpu->id, i, opp_freq); + + continue; + } + + voltage = dev_pm_opp_get_voltage(opp); + new_voltage = exynos_asv->opp_get_voltage(subsys, i, voltage); + dev_pm_opp_put(opp); + + opp_freq *= MHZ; + dev_pm_opp_remove(cpu, opp_freq); + + while (--timeout) { + opp = dev_pm_opp_find_freq_exact(cpu, opp_freq, true); + if (IS_ERR(opp)) + break; + dev_pm_opp_put(opp); + msleep(1); + } + + err = dev_pm_opp_add(cpu, opp_freq, new_voltage); + if (err < 0) + pr_err("%s: Failed to add OPP %u Hz/%u uV for cpu%d\n", + __func__, opp_freq, new_voltage, cpu->id); + } + + return 0; +} + +static int exynos_asv_update_opps(void) +{ + struct opp_table *last_opp_table = NULL; + struct device *cpu; + int ret, cpuid; + + for_each_possible_cpu(cpuid) { + struct opp_table *opp_table; + + cpu = get_cpu_device(cpuid); + if (!cpu) + continue; + + opp_table = dev_pm_opp_get_opp_table(cpu); + if (IS_ERR(opp_table)) + continue; + + if (!last_opp_table || opp_table != last_opp_table) { + last_opp_table = opp_table; + + ret = exynos_asv_update_cpu_opps(cpu); + if (ret < 0) + pr_err("%s: Couldn't udate OPPs for cpu%d\n", + __func__, cpuid); + } + + dev_pm_opp_put_opp_table(opp_table); + } + + return 0; +} + +static int __init exynos_asv_init(void) +{ + int ret; + + exynos_asv = kcalloc(1, sizeof(struct exynos_asv), GFP_KERNEL); + if (!exynos_asv) + return -ENOMEM; + + if (of_machine_is_compatible("samsung,exynos5800") || + of_machine_is_compatible("samsung,exynos5420")) + ret = exynos5422_asv_init(exynos_asv); + else + return 0; + + if (ret < 0) + return ret; + + ret = exynos_asv_update_opps(); + + if (exynos_asv->release) + exynos_asv->release(exynos_asv); + + return ret; +} +late_initcall(exynos_asv_init) diff --git a/drivers/soc/samsung/exynos-asv.h b/drivers/soc/samsung/exynos-asv.h new file mode 100644 index 000000000000..3444b361e5e3 --- /dev/null +++ b/drivers/soc/samsung/exynos-asv.h @@ -0,0 +1,114 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. + * http://www.samsung.com/ + * Author: Sylwester Nawrocki <s.nawrocki@samsung.com> + * + * Samsung Exynos SoC Adaptive Supply Voltage support + */ +#ifndef __EXYNOS_ASV_H +#define __EXYNOS_ASV_H + +#define EXYNOS_ASV_MAX_GROUPS 16 +#define EXYNOS_ASV_MAX_LEVELS 24 + +#define EXYNOS_ASV_TABLE_INDEX_MASK (0xff) +#define EXYNOS_ASV_SUBSYS_ID_MASK (0xff << 8) + +#define asv_table_to_subsys_id(_id) ((_id >> 8) & 0xff) +#define asv_table_to_index(_id) ((_id) & 0xff) + +#define EXYNOS_ASV_SUBSYS_ID_ARM 0x0 +#define EXYNOS_ASV_SUBSYS_ID_EGL EXYNOS_ASV_SUBSYS_ID_ARM +#define EXYNOS_ASV_SUBSYS_ID_KFC 0x1 +#define EXYNOS_ASV_SUBSYS_ID_INT 0x2 +#define EXYNOS_ASV_SUBSYS_ID_MIF 0x3 +#define EXYNOS_ASV_SUBSYS_ID_G3D 0x4 +#define EXYNOS_ASV_SUBSYS_ID_CAM 0x5 +#define EXYNOS_ASV_SUBSYS_ID_MAX 0x6 + +struct device_node; + +/* HPM, IDS values to select target group */ +struct asv_limit_entry { + unsigned int hpm; + unsigned int ids; +}; + +struct exynos_asv_table { + unsigned int num_rows; + unsigned int num_cols; + unsigned int id; + u32 *buf; +}; + +struct exynos_asv_subsys { + int id; + char *cpu_dt_compat; + + unsigned int base_volt; + unsigned int num_asv_levels; + unsigned int num_asv_groups; + unsigned int offset_volt_h; + unsigned int offset_volt_l; + struct exynos_asv_table table; +}; + +struct exynos_asv { + struct device_node *chipid_node; + struct exynos_asv_subsys arm; + struct exynos_asv_subsys kfc; + + int (*opp_get_voltage)(struct exynos_asv_subsys *subs, int level, + unsigned int voltage); + void (*release)(struct exynos_asv *asv); + + unsigned int group; + unsigned int table; + + /* TODO: Move member fields below to SoC type specific data structure */ + unsigned int is_bin2; + unsigned int is_special_lot; +}; + +static inline u32 __exynos_asv_get_item(struct exynos_asv_table *table, + unsigned int row, unsigned int col) +{ + return table->buf[row * (table->num_cols) + col]; +} + +static inline u32 exynos_asv_opp_get_frequency(struct exynos_asv_subsys *subsys, + unsigned int index) +{ + return __exynos_asv_get_item(&subsys->table, index, 0); +} +/** + * exynos_asv_parse_of_table - Parse ASV table from devicetree + * + * @node: DT node containing the table + * @table: The parsed table data + * @index: Used to select table with specific index to parse, if >= 0. + * This argument is ignored if the value is less than 0. + * + * Returns: 0 on success, or negative value on failure. EBADR is returned + * when @index is >= 0 but the index value of the parsed table ID is different + * than @index. + */ +int exynos_asv_parse_of_table(struct device_node *node, + struct exynos_asv_table *table, + int index); + +/** + * exynos_asv_parse_cpu_tables - Parse ARM and KFC ASV tables from DT + * + * @asv: data structure to store the parsed data to + * @node: DT node containing the tables + * @index: Used to select table with specific index to parse, if >= 0. + * This argument is ignored if the value is less than 0. + * + * Returns: 0 on success, or negative value on failure. + */ +int exynos_asv_parse_cpu_tables(struct exynos_asv *asv, struct device_node *np, + int table_index); + +#endif /* __EXYNOS_ASV_H */ diff --git a/drivers/soc/samsung/exynos5422-asv.c b/drivers/soc/samsung/exynos5422-asv.c new file mode 100644 index 000000000000..f0b7bdd873a9 --- /dev/null +++ b/drivers/soc/samsung/exynos5422-asv.c @@ -0,0 +1,209 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. + * http://www.samsung.com/ + * + * Samsung Exynos 5422 SoC Adaptive Supply Voltage support + */ + +#include <linux/errno.h> +#include <linux/init.h> +#include <linux/of.h> +#include <linux/slab.h> + +#include "exynos-asv.h" +#include "exynos-chipid.h" + +#define EXYNOS5422_NUM_ASV_GROUPS 14 + +static struct exynos_asv *exynos_asv; + +static int exynos5422_asv_get_table(void) +{ + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); + + return (reg >> EXYNOS5422_TABLE_OFFSET) & EXYNOS5422_TABLE_MASK; +} + +static int exynos5422_asv_check_bin2(void) +{ + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); + + return (reg >> EXYNOS5422_BIN2_OFFSET) & EXYNOS5422_BIN2_MASK; +} + +static bool exynos5422_asv_check_lot_id(void) +{ + unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); + + return ((reg >> EXYNOS5422_USESG_OFFSET) & EXYNOS5422_USESG_MASK); +} + +static const struct asv_limit_entry __asv_limits[EXYNOS5422_NUM_ASV_GROUPS] = { + { 13, 55 }, + { 21, 65 }, + { 25, 69 }, + { 30, 72 }, + { 36, 74 }, + { 43, 76 }, + { 51, 78 }, + { 65, 80 }, + { 81, 82 }, + { 98, 84 }, + { 119, 87 }, + { 135, 89 }, + { 150, 92 }, + { 999, 999 }, +}; + +static int exynos5422_asv_get_group(struct exynos_asv *asv) +{ + unsigned int pkgid_reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID); + unsigned int auxi_reg = exynos_chipid_read(EXYNOS_CHIPID_AUX_INFO); + int hpm, ids, i; + + if (asv->is_special_lot) { + u32 sga = (pkgid_reg >> EXYNOS5422_SG_A_OFFSET) & + EXYNOS5422_SG_A_MASK; + + u32 sgb = (pkgid_reg >> EXYNOS5422_SG_B_OFFSET) & + EXYNOS5422_SG_B_MASK; + + if ((pkgid_reg >> EXYNOS5422_SG_BSIGN_OFFSET) & + EXYNOS5422_SG_BSIGN_MASK) + return sga + sgb; + else + return sga - sgb; + } + + hpm = (auxi_reg >> EXYNOS5422_TMCB_OFFSET) & EXYNOS5422_TMCB_MASK; + ids = (pkgid_reg >> EXYNOS5422_IDS_OFFSET) & EXYNOS5422_IDS_MASK; + + for (i = 0; i < EXYNOS5422_NUM_ASV_GROUPS; i++) { + if (ids <= __asv_limits[i].ids) + break; + if (hpm <= __asv_limits[i].hpm) + break; + } + if (i < EXYNOS5422_NUM_ASV_GROUPS) + return i; + + return 0; +} + +static int __asv_offset_voltage(unsigned int index) +{ + static const unsigned int offset_table[] = { 12500, 50000, 25000 }; + + if (index == 0 || index > 3) + return 0; + + return offset_table[index - 1]; +} + +static void exynos5422_asv_offset_voltage_setup(struct exynos_asv *asv) +{ + struct exynos_asv_subsys *subsys; + unsigned int reg, value; + + reg = exynos_chipid_read(EXYNOS_CHIPID_AUX_INFO); + + /* ARM offset voltage setup */ + subsys = &asv->arm; + + subsys->base_volt = 1000000; + + value = (reg >> EXYNOS5422_ARM_UP_OFFSET) & EXYNOS5422_ARM_UP_MASK; + subsys->offset_volt_h = __asv_offset_voltage(value); + + value = (reg >> EXYNOS5422_ARM_DN_OFFSET) & EXYNOS5422_ARM_DN_MASK; + subsys->offset_volt_l = __asv_offset_voltage(value); + + /* KFC offset voltage setup */ + subsys = &asv->kfc; + + subsys->base_volt = 1000000; + + value = (reg >> EXYNOS5422_KFC_UP_OFFSET) & EXYNOS5422_KFC_UP_MASK; + subsys->offset_volt_h = __asv_offset_voltage(value); + + value = (reg >> EXYNOS5422_KFC_DN_OFFSET) & EXYNOS5422_KFC_DN_MASK; + subsys->offset_volt_l = __asv_offset_voltage(value); +} + +static int exynos5422_asv_opp_get_voltage(struct exynos_asv_subsys *subsys, + int level, unsigned int volt) +{ + unsigned int asv_volt; + + if (level >= subsys->num_asv_levels) + return volt; + + asv_volt = __exynos_asv_get_item(&subsys->table, level, + exynos_asv->group + 1); + if (asv_volt == 0 && exynos_asv->group > 1) + asv_volt = __exynos_asv_get_item(&subsys->table, level, 1); + + if (volt > subsys->base_volt) + asv_volt += subsys->offset_volt_h; + else + asv_volt += subsys->offset_volt_l; + + return asv_volt; +} + +static void __init exynos5422_asv_release(struct exynos_asv *asv) +{ + kfree(asv->arm.table.buf); + asv->arm.table.buf = NULL; + kfree(asv->kfc.table.buf); + asv->kfc.table.buf = NULL; +} + +int __init exynos5422_asv_init(struct exynos_asv *asv) +{ + struct device_node *node; + unsigned int table_index; + int ret; + + exynos_asv = asv; + + asv->is_bin2 = exynos5422_asv_check_bin2(); + asv->is_special_lot = exynos5422_asv_check_lot_id(); + + if (of_machine_is_compatible("hardkernel,odroid-xu3-lite")) { + asv->is_bin2 = true; + asv->is_special_lot = false; + } + + asv->group = exynos5422_asv_get_group(asv); + asv->table = exynos5422_asv_get_table(); + + exynos5422_asv_offset_voltage_setup(asv); + + node = of_find_compatible_node(NULL, NULL, "samsung,exynos-asv-v1"); + if (!node) + return -ENODEV; + + if (!asv->is_bin2) { + if (asv->table == 2 || asv->table == 3) + table_index = asv->table; + else + table_index = 0; + } else { + table_index = 4; + } + + ret = exynos_asv_parse_cpu_tables(asv, node, table_index); + of_node_put(node); + if (ret < 0) + return ret; + + asv->arm.cpu_dt_compat = "arm,cortex-a15"; + asv->kfc.cpu_dt_compat = "arm,cortex-a7"; + + asv->opp_get_voltage = exynos5422_asv_opp_get_voltage; + asv->release = exynos5422_asv_release; + + return ret; +} diff --git a/drivers/soc/samsung/exynos5422-asv.h b/drivers/soc/samsung/exynos5422-asv.h new file mode 100644 index 000000000000..f0d9107d6d0a --- /dev/null +++ b/drivers/soc/samsung/exynos5422-asv.h @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2018 - 2019 Samsung Electronics Co., Ltd. + * http://www.samsung.com/ + * + * Samsung Exynos 5422 SoC Adaptive Supply Voltage support + */ + +#ifndef __EXYNOS5422_ASV_H +#define __EXYNOS5422_ASV_H + +#include <linux/errno.h> + +struct exynos_asv; + +#ifdef CONFIG_EXYNOS_ASV_ARM +int exynos5422_asv_init(struct exynos_asv *asv); +#else +static inline int exynos5422_asv_init(struct exynos_asv *asv) +{ + return -ENOTSUPP; +} +#endif + +#endif /* __EXYNOS5422_ASV_H */
The Adaptive Supply Voltage (ASV) on Exynos SoCs is a technique of adjusting operating points of devices in order to better match actual capabilities of the hardware and optimize power consumption. This patch adds common code for parsing the ASV tables from devicetree and updating CPU OPPs. Also support for Exynos5422/5800 SoC is added, the Exynos5422 specific part of the patch is partially based on code from https://github.com/hardkernel/linux repository, branch odroidxu4-4.14.y, files: arch/arm/mach-exynos/exynos5422-asv.[ch]. Tested on Odroid XU3, XU4, XU3 Lite. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- drivers/soc/samsung/Kconfig | 11 ++ drivers/soc/samsung/Makefile | 3 + drivers/soc/samsung/exynos-asv.c | 279 +++++++++++++++++++++++++++ drivers/soc/samsung/exynos-asv.h | 114 +++++++++++ drivers/soc/samsung/exynos5422-asv.c | 209 ++++++++++++++++++++ drivers/soc/samsung/exynos5422-asv.h | 25 +++ 6 files changed, 641 insertions(+) create mode 100644 drivers/soc/samsung/exynos-asv.c create mode 100644 drivers/soc/samsung/exynos-asv.h create mode 100644 drivers/soc/samsung/exynos5422-asv.c create mode 100644 drivers/soc/samsung/exynos5422-asv.h