| Message ID | 20180507125223.18508-1-thierry.escande@linaro.org (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Andy Gross |
| Headers | show |
On 07-05-18, 14:52, Thierry Escande wrote: > For a given frequency and prop_name, if a voltage supply entry is > missing in the opp-table defined in DT, the frequency was added anyway > to the opp tables with a voltage setting of 0. > > With this patch, if a prop_name is set, frequencies with no > corresponding prop_name are ignored without failing. > > Signed-off-by: Thierry Escande <thierry.escande@linaro.org> > --- > > For Qualcomm apq8064 SoCs, the opp-tables defined in the dts file > contain all frequencies supported by all SoC versions. But not all SoCs > support all frequencies and they are filtered out by a prop_name > containing a speed bin value and a pvs value both read from a fuse > register sets in factory. The speed bin defines the SoC version and the > voltage setting is defined by the pvs value in the form: > > opp-384000000 { > opp-hz = /bits/ 64 <384000000>; > opp-microvolt-speed0-pvs0-v0 = <950000>; > opp-microvolt-speed0-pvs1-v0 = <900000>; > ... > opp-microvolt-speed1-pvs0-v0 = <950000>; > opp-microvolt-speed1-pvs1-v0 = <950000>; > ... > opp-microvolt-speed2-pvs0-v0 = <950000>; > opp-microvolt-speed2-pvs1-v0 = <950000>; > opp-microvolt-speed2-pvs2-v0 = <925000>; > opp-microvolt-speed2-pvs3-v0 = <900000>; > ... > }; That's not right. We shouldn't be overriding the prop-name feature to discard frequencies. We already have a special feature (which was added just for qcom to begin with) which much be used here: "opp-supported-hw". So you have to use "opp-supported-hw" for ignoring few of the OPP nodes on specific hardware platforms and "prop-name" for selecting the right voltage/current on them.
diff --git a/drivers/opp/of.c b/drivers/opp/of.c index cb716aa2f44b..2efe69d619f3 100644 --- a/drivers/opp/of.c +++ b/drivers/opp/of.c @@ -132,6 +132,14 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, /* Missing property isn't a problem, but an invalid entry is */ if (!prop) { + /* + * A prop_name is specified but no property found with + * or without prop_name. Return ENOENT so the caller + * knows the entry is missing. + */ + if (opp_table->prop_name) + return -ENOENT; + if (!opp_table->regulator_count) return 0; @@ -325,8 +333,11 @@ static int _opp_add_static_v2(struct opp_table *opp_table, struct device *dev, new_opp->clock_latency_ns = val; ret = opp_parse_supplies(new_opp, dev, opp_table); - if (ret) + if (ret) { + if (ret == -ENOENT) + ret = 0; goto free_opp; + } ret = _opp_add(dev, new_opp, opp_table); if (ret) {
For a given frequency and prop_name, if a voltage supply entry is missing in the opp-table defined in DT, the frequency was added anyway to the opp tables with a voltage setting of 0. With this patch, if a prop_name is set, frequencies with no corresponding prop_name are ignored without failing. Signed-off-by: Thierry Escande <thierry.escande@linaro.org> --- For Qualcomm apq8064 SoCs, the opp-tables defined in the dts file contain all frequencies supported by all SoC versions. But not all SoCs support all frequencies and they are filtered out by a prop_name containing a speed bin value and a pvs value both read from a fuse register sets in factory. The speed bin defines the SoC version and the voltage setting is defined by the pvs value in the form: opp-384000000 { opp-hz = /bits/ 64 <384000000>; opp-microvolt-speed0-pvs0-v0 = <950000>; opp-microvolt-speed0-pvs1-v0 = <900000>; ... opp-microvolt-speed1-pvs0-v0 = <950000>; opp-microvolt-speed1-pvs1-v0 = <950000>; ... opp-microvolt-speed2-pvs0-v0 = <950000>; opp-microvolt-speed2-pvs1-v0 = <950000>; opp-microvolt-speed2-pvs2-v0 = <925000>; opp-microvolt-speed2-pvs3-v0 = <900000>; ... }; Since some opp-freq entries may not have properties for a particular speed value the idea is to ignore a frequency if it does not contain a property as "opp-microvolt-prop_name" nor one as "opp-microvolt" without failing if the opp_table has a prop_name defined. Otherwise, there would be unsupported frequencies reported as supported with a voltage setting of zero. This could affect SoCs that have a prop_name defined and no "opp-microvolt" entry in the opp-table. I didn't spot any of these but I'd like to have your opinion. --- drivers/opp/of.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)