| Message ID | 20190313090010.20534-1-georgi.djakov@linaro.org |
|---|---|
| Headers | show |
| Series | Introduce OPP bandwidth bindings | expand |
On 13-03-19, 11:00, Georgi Djakov wrote: > The OPP bindings now support bandwidth values, so add support to parse it > from device tree and store it into the new dev_pm_opp_icc_bw struct, which > is part of the dev_pm_opp. > > Also add and export the dev_pm_opp_set_path() and dev_pm_opp_put_path() > helpers, to set (and release) an interconnect path to a device. The > bandwidth of this path will be updated when the OPPs are switched. > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > drivers/opp/core.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > drivers/opp/of.c | 44 +++++++++++++++++++++++++++ > drivers/opp/opp.h | 6 ++++ > include/linux/pm_opp.h | 14 +++++++++ > 4 files changed, 131 insertions(+) > > diff --git a/drivers/opp/core.c b/drivers/opp/core.c > index e06a0ab05ad6..4b019cecaa07 100644 > --- a/drivers/opp/core.c > +++ b/drivers/opp/core.c > @@ -19,6 +19,7 @@ > #include <linux/slab.h> > #include <linux/device.h> > #include <linux/export.h> > +#include <linux/interconnect.h> > #include <linux/pm_domain.h> > #include <linux/regulator/consumer.h> > > @@ -1645,6 +1646,72 @@ void dev_pm_opp_put_clkname(struct opp_table *opp_table) > } > EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname); > > +/** > + * dev_pm_opp_set_path() - Set interconnect path for a device > + * @dev: Device for which interconnect path is being set. > + * @name: Interconnect path name or NULL. > + * > + * This must be called before any OPPs are initialized for the device. > + */ > +struct opp_table *dev_pm_opp_set_path(struct device *dev, const char *name) Maybe the OPP core can do it itself in a similar way to how we do clk_get() today ? > +{ > + struct opp_table *opp_table; > + int ret; > + > + opp_table = dev_pm_opp_get_opp_table(dev); > + if (!opp_table) > + return ERR_PTR(-ENOMEM); > + > + /* This should be called before OPPs are initialized */ > + if (WARN_ON(!list_empty(&opp_table->opp_list))) { > + ret = -EBUSY; > + goto err; > + } > + > + /* Another CPU that shares the OPP table has set the path */ > + if (opp_table->path) > + return opp_table; > + > + /* Find interconnect path for the device */ > + opp_table->path = of_icc_get(dev, name); > + if (IS_ERR(opp_table->path)) { > + ret = PTR_ERR(opp_table->clk); > + if (ret != -EPROBE_DEFER) { > + dev_err(dev, "%s: Couldn't find path: %d\n", __func__, > + ret); > + } > + goto err; > + } > + > + return opp_table; > + > +err: > + dev_pm_opp_put_opp_table(opp_table); > + > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL_GPL(dev_pm_opp_set_path); > + > +/** > + * dev_pm_opp_put_path() - Release interconnect path resources > + * @opp_table: OPP table returned from dev_pm_opp_set_path(). > + */ > +void dev_pm_opp_put_path(struct opp_table *opp_table) > +{ > + if (!opp_table->path) > + goto put_opp_table; > + > + /* Make sure there are no concurrent readers while updating opp_table */ > + WARN_ON(!list_empty(&opp_table->opp_list)); > + > + icc_put(opp_table->path); > + opp_table->path = NULL; > + > +put_opp_table: > + dev_pm_opp_put_opp_table(opp_table); > +} > +EXPORT_SYMBOL_GPL(dev_pm_opp_put_path); > + > /** > * dev_pm_opp_register_set_opp_helper() - Register custom set OPP helper > * @dev: Device for which the helper is getting registered. > diff --git a/drivers/opp/of.c b/drivers/opp/of.c > index 1779f2c93291..96fb7fdda8c7 100644 > --- a/drivers/opp/of.c > +++ b/drivers/opp/of.c > @@ -16,6 +16,7 @@ > #include <linux/cpu.h> > #include <linux/errno.h> > #include <linux/device.h> > +#include <linux/interconnect.h> > #include <linux/of_device.h> > #include <linux/pm_domain.h> > #include <linux/slab.h> > @@ -526,6 +527,45 @@ static int opp_parse_supplies(struct dev_pm_opp *opp, struct device *dev, > return ret; > } > > +static int opp_parse_icc_bw(struct dev_pm_opp *opp, struct device *dev, > + struct opp_table *opp_table) > +{ > + struct property *prop = NULL; > + char name[NAME_MAX]; > + int count; > + u32 avg = 0; > + u32 peak = 0; Why init to 0 ? > + > + /* Search for "opp-bw-MBs" */ > + sprintf(name, "opp-bw-MBs"); > + prop = of_find_property(opp->np, name, NULL); > + > + /* Missing property is not a problem */ > + if (!prop) { > + dev_dbg(dev, "%s: Missing opp-bw-MBs\n", __func__); > + return 0; > + } > + > + count = of_property_count_u32_elems(opp->np, name); > + if (count != 2) { > + dev_err(dev, "%s: Invalid number of elements in %s property\n", > + __func__, name); > + return -EINVAL; > + } > + > + opp->bandwidth = kzalloc(sizeof(*opp->bandwidth), GFP_KERNEL); You forgot to free it. > + if (!opp->bandwidth) > + return -ENOMEM; > + > + of_property_read_u32_index(opp->np, name, 0, &avg); > + of_property_read_u32_index(opp->np, name, 1, &peak); > + > + opp->bandwidth->avg = MBps_to_icc(avg); > + opp->bandwidth->peak = MBps_to_icc(peak); > + > + return 0; > +} > + > /** > * dev_pm_opp_of_remove_table() - Free OPP table entries created from static DT > * entries > @@ -619,6 +659,10 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table, > if (ret) > goto free_required_opps; > > + ret = opp_parse_icc_bw(new_opp, dev, opp_table); > + if (ret) > + goto free_required_opps; > + > if (opp_table->is_genpd) > new_opp->pstate = pm_genpd_opp_to_performance_state(dev, new_opp); > > diff --git a/drivers/opp/opp.h b/drivers/opp/opp.h > index 4458175aa661..b4287d065c24 100644 > --- a/drivers/opp/opp.h > +++ b/drivers/opp/opp.h > @@ -24,6 +24,7 @@ > > struct clk; > struct regulator; > +struct icc_path; > > /* Lock to allow exclusive modification to the device and opp lists */ > extern struct mutex opp_table_lock; > @@ -62,6 +63,7 @@ extern struct list_head opp_tables; > * @rate: Frequency in hertz > * @level: Performance level > * @supplies: Power supplies voltage/current values > + * @bandwidth: Interconnect bandwidth values > * @clock_latency_ns: Latency (in nanoseconds) of switching to this OPP's > * frequency from any other OPP's frequency. > * @required_opps: List of OPPs that are required by this OPP. > @@ -85,6 +87,8 @@ struct dev_pm_opp { > > struct dev_pm_opp_supply *supplies; > > + struct dev_pm_opp_icc_bw *bandwidth; > + > unsigned long clock_latency_ns; > > struct dev_pm_opp **required_opps; > @@ -152,6 +156,7 @@ enum opp_table_access { > * property). > * @genpd_performance_state: Device's power domain support performance state. > * @is_genpd: Marks if the OPP table belongs to a genpd. > + * @path: Interconnect path handle > * @set_opp: Platform specific set_opp callback > * @set_opp_data: Data to be passed to set_opp callback > * @dentry: debugfs dentry pointer of the real device directory (not links). > @@ -196,6 +201,7 @@ struct opp_table { > int regulator_count; > bool genpd_performance_state; > bool is_genpd; > + struct icc_path *path; > > int (*set_opp)(struct dev_pm_set_opp_data *data); > struct dev_pm_set_opp_data *set_opp_data; > diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h > index 900359342965..5edce71a15d6 100644 > --- a/include/linux/pm_opp.h > +++ b/include/linux/pm_opp.h > @@ -43,6 +43,18 @@ struct dev_pm_opp_supply { > unsigned long u_amp; > }; > > +/** > + * struct dev_pm_opp_icc_bw - Interconnect bandwidth values > + * @avg: Average bandwidth corresponding to this OPP (in icc units) > + * @peak: Peak bandwidth corresponding to this OPP (in icc units) > + * > + * This structure stores the bandwidth values for a single interconnect path. > + */ > +struct dev_pm_opp_icc_bw { > + u32 avg; > + u32 peak; > +}; There is only one user of this structure, maybe we can directly add the elements in teh dev_pm_opp structure. > + > /** > * struct dev_pm_opp_info - OPP freq/voltage/current values > * @rate: Target clk rate in hz > @@ -127,6 +139,8 @@ struct opp_table *dev_pm_opp_set_regulators(struct device *dev, const char * con > void dev_pm_opp_put_regulators(struct opp_table *opp_table); > struct opp_table *dev_pm_opp_set_clkname(struct device *dev, const char * name); > void dev_pm_opp_put_clkname(struct opp_table *opp_table); > +struct opp_table *dev_pm_opp_set_path(struct device *dev, const char *name); > +void dev_pm_opp_put_path(struct opp_table *opp_table); > struct opp_table *dev_pm_opp_register_set_opp_helper(struct device *dev, int (*set_opp)(struct dev_pm_set_opp_data *data)); > void dev_pm_opp_unregister_set_opp_helper(struct opp_table *opp_table); > struct opp_table *dev_pm_opp_set_genpd_virt_dev(struct device *dev, struct device *virt_dev, int index);
On 3/13/19 2:30 PM, Georgi Djakov wrote: > Here is a proposal to extend the OPP bindings with bandwidth based on > a previous discussion [1]. > > Every functional block on a SoC can contribute to the system power > efficiency by expressing its own bandwidth needs (to memory or other SoC > modules). This will allow the system to save power when high throughput > is not required (and also provide maximum throughput when needed). > > There are at least three ways for a device to determine its bandwidth > needs: > 1. The device can dynamically calculate the needed bandwidth > based on some known variable. For example: UART (baud rate), I2C (fast > mode, high-speed mode, etc), USB (specification version, data transfer > type), SDHC (SD standard, clock rate, bus-width), Video Encoder/Decoder > (video format, resolution, frame-rate) > > 2. There is a hardware specific value. For example: hardware > specific constant value (e.g. for PRNG) or use-case specific value that > is hard-coded. > > 3. Predefined SoC/board specific bandwidth values. For example: > CPU or GPU bandwidth is related to the current core frequency and both > bandwidth and frequency are scaled together. > > This patchset is trying to address point 3 above by extending the OPP > bindings to support predefined SoC/board bandwidth values and adds > support in cpufreq-dt to scale the interconnect between the CPU and the > DDR together with frequency and voltage. Hey Georgi, Having opp-bw-MBps as a part of cpu opp does greatly simplify the problem of scaling multiple interconnect devices with change in cpu frequency. But there is still a need to scale other devices (non interconnect based) according to cpu frequency. Having a devfreq governor for the same would help to have the same generic solution across SoCs (msm8916/8996/qcs405/sdm845). The devfreq maintainer did like the idea but wanted it incorporated into the passive governor. * https://lore.kernel.org/lkml/20180528060014epcms1p87ec68a4d44f9447b06f979a87e545b7d@epcms1p8/ * https://lore.kernel.org/lkml/20180802095608epcms1p33fb061543efc9ceb3ec12d5567ceffbc@epcms1p3/ I have a RFC series implementing ddr scaling with passive governor for sdm845 with the following bindings, will post it early next week. cpus { ... CPU0: cpu@0 { ... operating-points-v2 = <&cpu0_opp_table>; ... }; .... CPU4: cpu@400 { ... operating-points-v2 = <&cpu4_opp_table>; ... }; ... }; cpu0_opp_table: cpu0_opp_table { compatible = "operating-points-v2"; opp-shared; cpu0_opp1: opp-300000000 { opp-hz = /bits/ 64 <300000000>; }; ... cpu0_opp16: opp-1612800000 { opp-hz = /bits/ 64 <1612800000>; }; ... }; cpu4_opp_table: cpu4_opp_table { compatible = "operating-points-v2"; opp-shared; ... cpu4_opp4: opp-1056000000 { opp-hz = /bits/ 64 <1056000000>; }; cpu4_opp5: opp-1209600000 { opp-hz = /bits/ 64 <1209600000>; }; ... }; bw_opp_table: bw-opp-table { compatible = "operating-points-v2"; opp-200 { opp-hz = /bits/ 64 < 200000000 >; /* 200 MHz */ required-opps = <&cpu0_opp1>; /* 0 MB/s average and 762 MB/s peak bandwidth */ opp-bw-MBs = <0 762>; }; opp-300 { opp-hz = /bits/ 64 < 300000000 >; /* 300 MHz */ /* 0 MB/s average and 1144 MB/s peak bandwidth */ opp-bw-MBs = <0 1144>; }; ... opp-768 { opp-hz = /bits/ 64 < 768000000 >; /* 768 MHz */ /* 0 MB/s average and 2929 MB/s peak bandwidth */ opp-bw-MBs = <0 2929>; required-opps = <&cpu4_opp4>; }; opp-1017 { opp-hz = /bits/ 64 < 1017000000 >; /* 1017 MHz */ /* 0 MB/s average and 3879 MB/s peak bandwidth */ opp-bw-MBs = <0 3879>; required-opps = <&cpu0_opp16>, <&cpu4_opp5>; }; }; cpubw { compatible = "devfreq-icbw"; interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>; operating-points-v2 = <&bw_opp_table>; }; > > [1] https://patchwork.kernel.org/patch/10577315/ > > Georgi Djakov (4): > dt-bindings: opp: Introduce opp-bw-MBs bindings > OPP: Add support for parsing the interconnect bandwidth > OPP: Update the bandwidth on OPP frequency changes > cpufreq: dt: Add support for interconnect bandwidth scaling > > Documentation/devicetree/bindings/opp/opp.txt | 45 ++++++++++++ > drivers/cpufreq/cpufreq-dt.c | 27 ++++++- > drivers/opp/core.c | 71 +++++++++++++++++++ > drivers/opp/of.c | 44 ++++++++++++ > drivers/opp/opp.h | 6 ++ > include/linux/pm_opp.h | 14 ++++ > 6 files changed, 206 insertions(+), 1 deletion(-) >
On Sat, Mar 16, 2019 at 12:32:49AM +0530, Sibi Sankar wrote: > > > On 3/13/19 2:30 PM, Georgi Djakov wrote: > > Here is a proposal to extend the OPP bindings with bandwidth based on > > a previous discussion [1]. > > > > Every functional block on a SoC can contribute to the system power > > efficiency by expressing its own bandwidth needs (to memory or other SoC > > modules). This will allow the system to save power when high throughput > > is not required (and also provide maximum throughput when needed). > > > > There are at least three ways for a device to determine its bandwidth > > needs: > > 1. The device can dynamically calculate the needed bandwidth > > based on some known variable. For example: UART (baud rate), I2C (fast > > mode, high-speed mode, etc), USB (specification version, data transfer > > type), SDHC (SD standard, clock rate, bus-width), Video Encoder/Decoder > > (video format, resolution, frame-rate) > > > > 2. There is a hardware specific value. For example: hardware > > specific constant value (e.g. for PRNG) or use-case specific value that > > is hard-coded. > > > > 3. Predefined SoC/board specific bandwidth values. For example: > > CPU or GPU bandwidth is related to the current core frequency and both > > bandwidth and frequency are scaled together. > > > > This patchset is trying to address point 3 above by extending the OPP > > bindings to support predefined SoC/board bandwidth values and adds > > support in cpufreq-dt to scale the interconnect between the CPU and the > > DDR together with frequency and voltage. > > Hey Georgi, > Having opp-bw-MBps as a part of cpu opp does greatly simplify the > problem of scaling multiple interconnect devices with change in cpu > frequency. But there is still a need to scale other devices (non > interconnect based) according to cpu frequency. Having a devfreq > governor for the same would help to have the same generic solution > across SoCs (msm8916/8996/qcs405/sdm845). The devfreq maintainer did > like the idea but wanted it incorporated into the passive governor. > > * https://lore.kernel.org/lkml/20180528060014epcms1p87ec68a4d44f9447b06f979a87e545b7d@epcms1p8/ > > * https://lore.kernel.org/lkml/20180802095608epcms1p33fb061543efc9ceb3ec12d5567ceffbc@epcms1p3/ > > I have a RFC series implementing ddr scaling with passive governor for > sdm845 with the following bindings, will post it early next week. > > cpus { > ... > > CPU0: cpu@0 { > ... > operating-points-v2 = <&cpu0_opp_table>; > ... > }; > .... > > CPU4: cpu@400 { > ... > operating-points-v2 = <&cpu4_opp_table>; > ... > }; > ... > }; > > cpu0_opp_table: cpu0_opp_table { > compatible = "operating-points-v2"; > opp-shared; > > cpu0_opp1: opp-300000000 { > opp-hz = /bits/ 64 <300000000>; > }; > > ... > > cpu0_opp16: opp-1612800000 { > opp-hz = /bits/ 64 <1612800000>; > }; > > ... > }; > > cpu4_opp_table: cpu4_opp_table { > compatible = "operating-points-v2"; > opp-shared; > > ... > > cpu4_opp4: opp-1056000000 { > opp-hz = /bits/ 64 <1056000000>; > }; > > cpu4_opp5: opp-1209600000 { > opp-hz = /bits/ 64 <1209600000>; > }; > > ... > }; > > bw_opp_table: bw-opp-table { > compatible = "operating-points-v2"; > > opp-200 { > opp-hz = /bits/ 64 < 200000000 >; /* 200 MHz */ > required-opps = <&cpu0_opp1>; > /* 0 MB/s average and 762 MB/s peak bandwidth */ > opp-bw-MBs = <0 762>; > }; > > opp-300 { > opp-hz = /bits/ 64 < 300000000 >; /* 300 MHz */ > /* 0 MB/s average and 1144 MB/s peak bandwidth */ > opp-bw-MBs = <0 1144>; > }; > > ... > > opp-768 { > opp-hz = /bits/ 64 < 768000000 >; /* 768 MHz */ > /* 0 MB/s average and 2929 MB/s peak bandwidth */ > opp-bw-MBs = <0 2929>; > required-opps = <&cpu4_opp4>; > }; > > opp-1017 { > opp-hz = /bits/ 64 < 1017000000 >; /* 1017 MHz */ > /* 0 MB/s average and 3879 MB/s peak bandwidth */ > opp-bw-MBs = <0 3879>; > required-opps = <&cpu0_opp16>, <&cpu4_opp5>; > }; > }; > > cpubw { > compatible = "devfreq-icbw"; Most certainly not a h/w device, so it doesn't go in DT. > interconnects = <&snoc MASTER_APSS_1 &bimc SLAVE_EBI_CH0>; > operating-points-v2 = <&bw_opp_table>; > }; > > > > > [1] https://patchwork.kernel.org/patch/10577315/ > > > > Georgi Djakov (4): > > dt-bindings: opp: Introduce opp-bw-MBs bindings > > OPP: Add support for parsing the interconnect bandwidth > > OPP: Update the bandwidth on OPP frequency changes > > cpufreq: dt: Add support for interconnect bandwidth scaling > > > > Documentation/devicetree/bindings/opp/opp.txt | 45 ++++++++++++ > > drivers/cpufreq/cpufreq-dt.c | 27 ++++++- > > drivers/opp/core.c | 71 +++++++++++++++++++ > > drivers/opp/of.c | 44 ++++++++++++ > > drivers/opp/opp.h | 6 ++ > > include/linux/pm_opp.h | 14 ++++ > > 6 files changed, 206 insertions(+), 1 deletion(-) > > > > -- > Qualcomm Innovation Center, Inc. > Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project
Hi Viresh, On 3/14/19 08:30, Viresh Kumar wrote: > On 13-03-19, 11:00, Georgi Djakov wrote: >> The OPP bindings now support bandwidth values, so add support to parse it >> from device tree and store it into the new dev_pm_opp_icc_bw struct, which >> is part of the dev_pm_opp. >> >> Also add and export the dev_pm_opp_set_path() and dev_pm_opp_put_path() >> helpers, to set (and release) an interconnect path to a device. The >> bandwidth of this path will be updated when the OPPs are switched. >> >> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> >> --- >> drivers/opp/core.c | 67 ++++++++++++++++++++++++++++++++++++++++++ >> drivers/opp/of.c | 44 +++++++++++++++++++++++++++ >> drivers/opp/opp.h | 6 ++++ >> include/linux/pm_opp.h | 14 +++++++++ >> 4 files changed, 131 insertions(+) >> >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c >> index e06a0ab05ad6..4b019cecaa07 100644 >> --- a/drivers/opp/core.c >> +++ b/drivers/opp/core.c >> @@ -19,6 +19,7 @@ >> #include <linux/slab.h> >> #include <linux/device.h> >> #include <linux/export.h> >> +#include <linux/interconnect.h> >> #include <linux/pm_domain.h> >> #include <linux/regulator/consumer.h> >> >> @@ -1645,6 +1646,72 @@ void dev_pm_opp_put_clkname(struct opp_table *opp_table) >> } >> EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname); >> >> +/** >> + * dev_pm_opp_set_path() - Set interconnect path for a device >> + * @dev: Device for which interconnect path is being set. >> + * @name: Interconnect path name or NULL. >> + * >> + * This must be called before any OPPs are initialized for the device. >> + */ >> +struct opp_table *dev_pm_opp_set_path(struct device *dev, const char *name) > > Maybe the OPP core can do it itself in a similar way to how we do > clk_get() today ? Do you mean to directly call of_icc_get() in _allocate_opp_table()? [..] >> +static int opp_parse_icc_bw(struct dev_pm_opp *opp, struct device *dev, >> + struct opp_table *opp_table) >> +{ >> + struct property *prop = NULL; >> + char name[NAME_MAX]; >> + int count; >> + u32 avg = 0; >> + u32 peak = 0; > > Why init to 0 ? Right, seems not necessary. >> + >> + /* Search for "opp-bw-MBs" */ >> + sprintf(name, "opp-bw-MBs"); >> + prop = of_find_property(opp->np, name, NULL); >> + >> + /* Missing property is not a problem */ >> + if (!prop) { >> + dev_dbg(dev, "%s: Missing opp-bw-MBs\n", __func__); >> + return 0; >> + } >> + >> + count = of_property_count_u32_elems(opp->np, name); >> + if (count != 2) { >> + dev_err(dev, "%s: Invalid number of elements in %s property\n", >> + __func__, name); >> + return -EINVAL; >> + } >> + >> + opp->bandwidth = kzalloc(sizeof(*opp->bandwidth), GFP_KERNEL); > > You forgot to free it. Will fix. [..]>> +/** >> + * struct dev_pm_opp_icc_bw - Interconnect bandwidth values >> + * @avg: Average bandwidth corresponding to this OPP (in icc units) >> + * @peak: Peak bandwidth corresponding to this OPP (in icc units) >> + * >> + * This structure stores the bandwidth values for a single interconnect path. >> + */ >> +struct dev_pm_opp_icc_bw { >> + u32 avg; >> + u32 peak; >> +}; > > There is only one user of this structure, maybe we can directly add > the elements in teh dev_pm_opp structure. Ok, will do it. Thanks, Georgi
On 09-04-19, 17:37, Georgi Djakov wrote: > Hi Viresh, > > On 3/14/19 08:30, Viresh Kumar wrote: > > On 13-03-19, 11:00, Georgi Djakov wrote: > >> The OPP bindings now support bandwidth values, so add support to parse it > >> from device tree and store it into the new dev_pm_opp_icc_bw struct, which > >> is part of the dev_pm_opp. > >> > >> Also add and export the dev_pm_opp_set_path() and dev_pm_opp_put_path() > >> helpers, to set (and release) an interconnect path to a device. The > >> bandwidth of this path will be updated when the OPPs are switched. > >> > >> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > >> --- > >> drivers/opp/core.c | 67 ++++++++++++++++++++++++++++++++++++++++++ > >> drivers/opp/of.c | 44 +++++++++++++++++++++++++++ > >> drivers/opp/opp.h | 6 ++++ > >> include/linux/pm_opp.h | 14 +++++++++ > >> 4 files changed, 131 insertions(+) > >> > >> diff --git a/drivers/opp/core.c b/drivers/opp/core.c > >> index e06a0ab05ad6..4b019cecaa07 100644 > >> --- a/drivers/opp/core.c > >> +++ b/drivers/opp/core.c > >> @@ -19,6 +19,7 @@ > >> #include <linux/slab.h> > >> #include <linux/device.h> > >> #include <linux/export.h> > >> +#include <linux/interconnect.h> > >> #include <linux/pm_domain.h> > >> #include <linux/regulator/consumer.h> > >> > >> @@ -1645,6 +1646,72 @@ void dev_pm_opp_put_clkname(struct opp_table *opp_table) > >> } > >> EXPORT_SYMBOL_GPL(dev_pm_opp_put_clkname); > >> > >> +/** > >> + * dev_pm_opp_set_path() - Set interconnect path for a device > >> + * @dev: Device for which interconnect path is being set. > >> + * @name: Interconnect path name or NULL. > >> + * > >> + * This must be called before any OPPs are initialized for the device. > >> + */ > >> +struct opp_table *dev_pm_opp_set_path(struct device *dev, const char *name) > > > > Maybe the OPP core can do it itself in a similar way to how we do > > clk_get() today ? It took me a decade to understand my own comment ;) > Do you mean to directly call of_icc_get() in _allocate_opp_table()? I believe I wanted to say s/clk_get()/clk_set_rate()/ . i.e. if someone calls set-opp-rate, then the path should get set as well accordingly automagically.