On 22-06-15, 16:43, Lee Jones wrote: > +config ARM_ST_CPUFREQ > + bool "ST CPUFreq support" Isn't using ST just too generic? There are multiple SoCs ST has been involved with, I have worked on a completely different series. Probably a more relative string is required here, like stih407 ? > + depends on SOC_STIH407 > diff --git a/drivers/cpufreq/st-cpufreq.c b/drivers/cpufreq/st-cpufreq.c > +static int st_cpufreq_cmp(const void *a, const void *b) > +{ > + const struct st_dvfs_tab *a_tab = a, *b_tab = b; > + > + if (a_tab->freq > b_tab->freq) > + return -1; > + > + if (a_tab->freq < b_tab->freq) > + return 1; > + > + return 0; > +} > + > +static int st_cpufreq_check_if_matches(struct device_node *child, > + const char *prop, > + unsigned int match) > +{ > + unsigned int dt_major, dt_minor; > + unsigned int soc_major, soc_minor; > + const __be32 *tmp; > + unsigned int val; > + int len = 0; > + int i; > + > + tmp = of_get_property(child, prop , &len); > + if (!tmp || !len) > + return -EINVAL; > + > + val = be32_to_cpup(tmp); > + > + len /= sizeof(u32); > + if (len == 1 && val == 0xff) > + /* > + * If 'cuts' or 'substrate' value is 0xff, it means that > + * the entry is valid for ALL cuts and substrates > + */ > + goto matchfound; > + > + /* Check if this opp node is for us */ > + for (i = 0; i < len; i++) { > + if (match == val) > + goto matchfound; > + > + if (!strcmp(prop, "st,cuts")) { > + dt_major = val & 0xff;; > + dt_minor = val >> 8 & 0xff; > + soc_major = match & 0xff; > + soc_minor = match >> 8 & 0xff; > + > + if (dt_major == soc_major && > + (!dt_minor || (dt_minor == soc_minor))) > + goto matchfound; > + } > + val++; > + } > + > + /* No match found */ > + return -EINVAL; > + > +matchfound: > + return 0; > +} > + > +static int st_cpufreq_get_version(struct platform_device *pdev, > + unsigned int *minor, unsigned int *major) > +{ > + struct st_cpufreq_ddata *ddata = platform_get_drvdata(pdev); > + struct device_node *np = pdev->dev.of_node; > + struct regmap *syscfg_regmap; > + unsigned int minor_offset, major_offset; > + unsigned int socid, minid; > + int ret; > + > + /* Get Major */ > + syscfg_regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg"); > + if (IS_ERR(syscfg_regmap)) { > + dev_err(&pdev->dev, > + "No syscfg phandle specified in %s [%li]\n", > + np->full_name, PTR_ERR(syscfg_regmap)); > + return PTR_ERR(syscfg_regmap); > + } > + > + ret = of_property_read_u32_index(np, "st,syscfg", > + MAJOR_ID_INDEX, &major_offset); > + if (ret) { > + dev_err(&pdev->dev, > + "No minor number offset provided in %s [%d]\n", > + np->full_name, ret); > + return ret; > + } > + > + ret = regmap_read(syscfg_regmap, major_offset, &socid); > + if (ret) > + return ret; > + > + /* Get Minor */ > + ret = of_property_read_u32_index(np, "st,syscfg-eng", > + MINOR_ID_INDEX, &minor_offset); > + if (ret) { > + dev_err(&pdev->dev, "No minor number offset provided %s [%d]\n", > + np->full_name, ret); > + return ret; > + } > + > + ret = regmap_read(ddata->regmap_eng, minor_offset, &minid); > + if (ret) { > + dev_err(&pdev->dev, > + "Failed to read the minor number from syscon [%d]\n", > + ret); > + return ret; > + } > + > + *major = ((socid >> VERSION_SHIFT) & 0xf) + 1; > + *minor = minid & 0xf; > + > + return 0; > +} > + > +static int st_cpufreq_get_dvfs_info(struct platform_device *pdev) > +{ > + struct st_cpufreq_ddata *ddata = platform_get_drvdata(pdev); > + struct st_dvfs_tab *dvfs_tab = &ddata->dvfs_tab[0]; > + struct device_node *np = pdev->dev.of_node; > + struct device_node *opplist, *opp; > + unsigned int minor = 0, major = 0; > + int err, ret = 0; > + > + opplist = of_get_child_by_name(np, "opp-list"); st,opp-list ? > + if (!opplist) { > + dev_err(&pdev->dev, "opp-list node missing\n"); > + return -ENODATA; > + } > + > + ret = st_cpufreq_get_version(pdev, &minor, &major); > + if (ret) { > + dev_err(&pdev->dev, "No OPP match found for this platform\n"); > + return ret; > + } > + > + for_each_child_of_node(opplist, opp) { > + if (ddata->dvfs_tab_count == STI_DVFS_TAB_MAX) { > + dev_err(&pdev->dev, "Too many DVFS entries found\n"); > + ret = -EOVERFLOW; > + break; > + } > + > + /* Cut version e.g. 2.0 [major.minor] */ > + err = st_cpufreq_check_if_matches(opp, "st,cuts", > + (minor << 8) | major); > + if (err) > + continue; > + > + ret = st_cpufreq_check_if_matches(opp, "st,substrate", > + ddata->substrate); > + if (err) > + continue; > + > + ret = of_property_read_u32(opp, "st,freq", &dvfs_tab->freq); > + if (ret) { > + dev_err(&pdev->dev, "Can't read frequency: %d\n", ret); > + goto out; > + } > + dvfs_tab->freq *= 1000; > + > + ret = of_property_read_u32_index(opp, "st,avs", > + ddata->pcode, > + &dvfs_tab->avs); > + if (ret) { > + dev_err(&pdev->dev, "Can't read AVS: %d\n", ret); > + goto out; > + } > + > + dvfs_tab++; > + ddata->dvfs_tab_count++; > + } > + > + sort(&ddata->dvfs_tab[0], ddata->dvfs_tab_count, > + sizeof(struct st_dvfs_tab), st_cpufreq_cmp, NULL); > + > +out: > + of_node_put(opplist); > + > + if (!ddata->dvfs_tab_count) { > + dev_err(&pdev->dev, "No suitable AVS table found\n"); Why is this an error? I thought in this case you will go ahead with the normal OPP-table. > + return -EINVAL; > + } > + > + return ret; > +} > + > +static int sti_cpufreq_voltage_scaling_init(struct platform_device *pdev) > +{ > + struct st_cpufreq_ddata *ddata = platform_get_drvdata(pdev); > + struct st_dvfs_tab *dvfs_tab = &ddata->dvfs_tab[0]; > + struct device *cpu_dev; > + struct dev_pm_opp *opp; > + unsigned long highest_freq = 0; > + int ret; > + int i; > + > + cpu_dev = get_cpu_device(0); > + if (!cpu_dev) { > + dev_err(&pdev->dev, "Failed to get cpu0 device\n"); > + return -ENODEV; > + } > + > + /* Populate OPP table with default non-AVS frequency values */ > + of_init_opp_table(cpu_dev); > + > + /* > + * Disable, but keep default values -- this prevents the framework from > + * erroneously re-adding and enabling entries with missing voltage rates > + */ > + while (1) { > + highest_freq++; > + > + opp = dev_pm_opp_find_freq_ceil(cpu_dev, &highest_freq); > + if (IS_ERR(opp)) > + break; > + > + ret = dev_pm_opp_disable(cpu_dev, highest_freq); > + if (ret) { > + dev_err(&pdev->dev, "Failed to disable freq: %li\n", > + highest_freq); > + return ret; > + } > + } > + > + for (i = 0; i < ddata->dvfs_tab_count; i++, dvfs_tab++) { > + unsigned int f = dvfs_tab->freq * 1000; > + unsigned int v = dvfs_tab->avs * 1000; > + > + opp = dev_pm_opp_find_freq_exact(cpu_dev, f, false); > + > + /* Remove existing voltage-less OPP entry */ > + if (!IS_ERR(opp)) > + dev_pm_opp_remove(cpu_dev, f); > + > + /* Supply new fully populated OPP entry */ > + ret = dev_pm_opp_add(cpu_dev, f, v); > + if (ret) { > + dev_err(&pdev->dev, "Failed to add OPP %u\n", f); > + return ret; > + } > + } So you have added new OPPs here, but cpufreq-dt will try to add old OPPs. You must be getting lots of warnings ? > + > + return 0; > +} > + > +static int st_cpufreq_fetch_regmap_field(struct platform_device *pdev, > + const struct reg_field *reg_fields, > + int pcode_offset, int field) > +{ > + struct st_cpufreq_ddata *ddata = platform_get_drvdata(pdev); > + struct regmap_field *regmap_field; > + struct reg_field reg_field = reg_fields[field]; > + unsigned int value; > + int ret; > + > + reg_field.reg = pcode_offset; > + regmap_field = devm_regmap_field_alloc(&pdev->dev, > + ddata->regmap_eng, > + reg_field); > + if (IS_ERR(regmap_field)) { > + dev_err(&pdev->dev, "Failed to allocate reg field\n"); > + return PTR_ERR(regmap_field); > + } > + > + ret = regmap_field_read(regmap_field, &value); > + if (ret) { > + dev_err(&pdev->dev, "Failed to read %s code\n", > + field ? "SUBSTRATE" : "PCODE"); > + return ret; > + } > + > + return value; > +} > + > +static const struct reg_field sti_stih407_dvfs_regfields[DVFS_MAX_REGFIELDS] = { > + [PCODE] = REG_FIELD(0, 16, 19), > + [SUBSTRATE] = REG_FIELD(0, 0, 2), > +}; > + > +static struct of_device_id sti_cpufreq_of_match[] = { > + { > + .compatible = "st,stih407-cpufreq", > + .data = &sti_stih407_dvfs_regfields, > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, sti_cpufreq_of_match); > + > +/* Find process code -- calibrated per-SoC */ > +static void sti_cpufreq_get_pcode(struct platform_device *pdev) > +{ > + struct st_cpufreq_ddata *ddata = platform_get_drvdata(pdev); > + struct device_node *np = pdev->dev.of_node; > + const struct reg_field *reg_fields; > + const struct of_device_id *match; > + int pcode_offset; > + int ret; > + > + ddata->regmap_eng = syscon_regmap_lookup_by_phandle(np, "st,syscfg-eng"); > + if (IS_ERR(ddata->regmap_eng)) { > + dev_warn(&pdev->dev, "\"st,syscfg-eng\" not supplied\n"); > + goto set_default; > + } > + > + ret = of_property_read_u32_index(np, "st,syscfg-eng", > + PCODE_INDEX, &pcode_offset); > + if (ret) { > + dev_warn(&pdev->dev, "Process code offset is required\n"); > + goto set_default; > + } > + > + match = of_match_node(sti_cpufreq_of_match, np); Are you planning to add more entries to this table? We are here as probe() is called only after matching for this string. > + if (!match) { > + dev_warn(&pdev->dev, "Failed to match device\n"); > + goto set_default; > + } > + reg_fields = match->data; > + > + ddata->pcode = st_cpufreq_fetch_regmap_field(pdev, reg_fields, > + pcode_offset, > + PCODE); > + if (ddata->pcode < 0) > + goto set_default; > + > + ddata->substrate = st_cpufreq_fetch_regmap_field(pdev, reg_fields, > + pcode_offset, > + SUBSTRATE); > + if (ddata->substrate < 0) > + goto set_default; Maybe: if (ddata->substrate >= 0) return; > + > + return; > + > +set_default: > + dev_warn(&pdev->dev, > + "Setting pcode to highest tolerance/voltage for safety\n"); > + ddata->pcode = 0; > + ddata->substrate = 0; > +} > + > +static int sti_cpufreq_probe(struct platform_device *pdev) > +{ > + struct st_cpufreq_ddata *ddata; > + int ret; > + > + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); > + if (!ddata) > + return -ENOMEM; > + > + platform_set_drvdata(pdev, ddata); > + > + sti_cpufreq_get_pcode(pdev); > + > + ret = st_cpufreq_get_dvfs_info(pdev); > + if (ret) > + dev_warn(&pdev->dev, "Not doing voltage scaling [%d]\n", ret); > + else > + sti_cpufreq_voltage_scaling_init(pdev); > + > + platform_device_register_simple("cpufreq-dt", -1, NULL, 0); > + > + return 0; > +} -- To unsubscribe from this list: send the line "unsubscribe devicetree" in