| Message ID | 20200512125327.1868-10-georgi.djakov@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | viresh kumar |
| Headers | show |
| Series | Introduce OPP bandwidth bindings | expand |
On 12-05-20, 15:53, Georgi Djakov wrote: > From: Sibi Sankar <sibis@codeaurora.org> > > Add interconnect-tags bindings to enable passing of optional > tag information to the interconnect framework. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > v8: > * New patch, picked from here: > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org > > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt > index 6f5d23a605b7..c1a226a934e5 100644 > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same > * dma-mem: Path from the device to the main memory of > the system > > +interconnect-tags : List of interconnect path tags sorted in the same order as the > + interconnects property. Consumers can append a specific tag to > + the path and pass this information to the interconnect framework > + to do aggregation based on the attached tag. > + > Example: > > sdhci@7864000 { @Rob: Though I have applied the patch to my branch for now, I can revert it just fine if you aren't okay with the bindings. Please lemme know about your feedback on this (sorry for missing that earlier).
On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote: > From: Sibi Sankar <sibis@codeaurora.org> > > Add interconnect-tags bindings to enable passing of optional > tag information to the interconnect framework. > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > --- > v8: > * New patch, picked from here: > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org > > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt > index 6f5d23a605b7..c1a226a934e5 100644 > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same > * dma-mem: Path from the device to the main memory of > the system > > +interconnect-tags : List of interconnect path tags sorted in the same order as the > + interconnects property. Consumers can append a specific tag to > + the path and pass this information to the interconnect framework > + to do aggregation based on the attached tag. Why isn't this information in the 'interconnect' arg cells? We have 'interconnect-names' because strings don't mix with cells. An expanding list of 'interconnect-.*' is not a good pattern IMO. Rob
On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote: > > On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote: > > From: Sibi Sankar <sibis@codeaurora.org> > > > > Add interconnect-tags bindings to enable passing of optional > > tag information to the interconnect framework. > > > > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > > --- > > v8: > > * New patch, picked from here: > > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org > > > > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt > > index 6f5d23a605b7..c1a226a934e5 100644 > > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt > > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt > > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same > > * dma-mem: Path from the device to the main memory of > > the system > > > > +interconnect-tags : List of interconnect path tags sorted in the same order as the > > + interconnects property. Consumers can append a specific tag to > > + the path and pass this information to the interconnect framework > > + to do aggregation based on the attached tag. > > Why isn't this information in the 'interconnect' arg cells? > > We have 'interconnect-names' because strings don't mix with cells. An > expanding list of 'interconnect-.*' is not a good pattern IMO. Also, is there an example for interconnect-tags that I missed? Is it a list of strings, numbers, etc? -Saravana
On 2020-05-20 01:27, Saravana Kannan wrote: > On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote: >> >> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote: >> > From: Sibi Sankar <sibis@codeaurora.org> >> > >> > Add interconnect-tags bindings to enable passing of optional >> > tag information to the interconnect framework. >> > >> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> >> > --- >> > v8: >> > * New patch, picked from here: >> > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org >> > >> > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt >> > index 6f5d23a605b7..c1a226a934e5 100644 >> > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt >> > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt >> > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same >> > * dma-mem: Path from the device to the main memory of >> > the system >> > >> > +interconnect-tags : List of interconnect path tags sorted in the same order as the >> > + interconnects property. Consumers can append a specific tag to >> > + the path and pass this information to the interconnect framework >> > + to do aggregation based on the attached tag. >> >> Why isn't this information in the 'interconnect' arg cells? >> >> We have 'interconnect-names' because strings don't mix with cells. An >> expanding list of 'interconnect-.*' is not a good pattern IMO. Rob, Currently the interconnect paths assume a default tag and only few icc paths require tags that differ from the default ones. Encoding the tags in the interconnect arg cells would force all paths to specify the tags. I guess that's okay. > > Also, is there an example for interconnect-tags that I missed? Is it a > list of strings, numbers, etc? Saravana, https://patchwork.kernel.org/patch/11527589/ ^^ is an example of interconnect-tag useage. > > -Saravana
On Wed, May 20, 2020 at 11:51 AM Sibi Sankar <sibis@codeaurora.org> wrote: > > On 2020-05-20 01:27, Saravana Kannan wrote: > > On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote: > >> > >> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote: > >> > From: Sibi Sankar <sibis@codeaurora.org> > >> > > >> > Add interconnect-tags bindings to enable passing of optional > >> > tag information to the interconnect framework. > >> > > >> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > >> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> > >> > --- > >> > v8: > >> > * New patch, picked from here: > >> > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org > >> > > >> > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ > >> > 1 file changed, 5 insertions(+) > >> > > >> > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt > >> > index 6f5d23a605b7..c1a226a934e5 100644 > >> > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt > >> > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt > >> > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same > >> > * dma-mem: Path from the device to the main memory of > >> > the system > >> > > >> > +interconnect-tags : List of interconnect path tags sorted in the same order as the > >> > + interconnects property. Consumers can append a specific tag to > >> > + the path and pass this information to the interconnect framework > >> > + to do aggregation based on the attached tag. > >> > >> Why isn't this information in the 'interconnect' arg cells? > >> > >> We have 'interconnect-names' because strings don't mix with cells. An > >> expanding list of 'interconnect-.*' is not a good pattern IMO. > > Rob, > Currently the interconnect paths > assume a default tag and only few > icc paths require tags that differ > from the default ones. Encoding the > tags in the interconnect arg cells > would force all paths to specify > the tags. I guess that's okay. I think that's the right thing. Those cells are meant to be "args" to the provider. > > > > Also, is there an example for interconnect-tags that I missed? Is it a > > list of strings, numbers, etc? > > Saravana, > https://patchwork.kernel.org/patch/11527589/ > ^^ is an example of interconnect-tag useage. If we actually merge interconnect-tags, I think the doc should be updated. Instead of having to grep around. -Saravana
On 5/21/20 12:43 AM, Saravana Kannan wrote: > On Wed, May 20, 2020 at 11:51 AM Sibi Sankar <sibis@codeaurora.org> wrote: >> >> On 2020-05-20 01:27, Saravana Kannan wrote: >>> On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote: >>>> >>>> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote: >>>>> From: Sibi Sankar <sibis@codeaurora.org> >>>>> >>>>> Add interconnect-tags bindings to enable passing of optional >>>>> tag information to the interconnect framework. >>>>> >>>>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >>>>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org> >>>>> --- >>>>> v8: >>>>> * New patch, picked from here: >>>>> https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org >>>>> >>>>> .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++ >>>>> 1 file changed, 5 insertions(+) >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt >>>>> index 6f5d23a605b7..c1a226a934e5 100644 >>>>> --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt >>>>> +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt >>>>> @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same >>>>> * dma-mem: Path from the device to the main memory of >>>>> the system >>>>> >>>>> +interconnect-tags : List of interconnect path tags sorted in the same order as the >>>>> + interconnects property. Consumers can append a specific tag to >>>>> + the path and pass this information to the interconnect framework >>>>> + to do aggregation based on the attached tag. >>>> >>>> Why isn't this information in the 'interconnect' arg cells? >>>> >>>> We have 'interconnect-names' because strings don't mix with cells. An >>>> expanding list of 'interconnect-.*' is not a good pattern IMO. >> >> Rob, >> Currently the interconnect paths >> assume a default tag and only few >> icc paths require tags that differ >> from the default ones. Encoding the >> tags in the interconnect arg cells >> would force all paths to specify >> the tags. I guess that's okay. > > I think that's the right thing. Those cells are meant to be "args" to > the provider. diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index ea4764f06a901..b34f024d4ab63 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -132,9 +132,8 @@ &LITTLE_CPU_SLEEP_1 capacity-dmips-mhz = <1024>; dynamic-power-coefficient = <100>; operating-points-v2 = <&cpu0_opp_table>; - interconnects = <&gem_noc MASTER_APPSS_PROC &mc_virt SLAVE_EBI1>, + interconnects = <&gem_noc MASTER_APPSS_PROC 3 &mc_virt SLAVE_EBI1 3>, <&osm_l3 MASTER_OSM_L3_APPS &osm_l3 SLAVE_OSM_L3>; next-level-cache = <&L2_0>; #cooling-cells = <2>; qcom,freq-domain = <&cpufreq_hw 0>; .... mc_virt: interconnect@1638000 { compatible = "qcom,sc7180-mc-virt"; reg = <0 0x01638000 0 0x1000>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <&apps_bcm_voter>; }; .... @@ -2216,14 +2208,14 @@ system-cache-controller@9200000 { gem_noc: interconnect@9680000 { compatible = "qcom,sc7180-gem-noc"; reg = <0 0x09680000 0 0x3e200>; - #interconnect-cells = <1>; + #interconnect-cells = <2>; qcom,bcm-voters = <&apps_bcm_voter>; }; .... diff --git a/drivers/interconnect/core.c b/drivers/interconnect/core.c index 294e9c58565bb..6a7a785bd90b9 100644 --- a/drivers/interconnect/core.c +++ b/drivers/interconnect/core.c @@ -340,7 +340,7 @@ static struct icc_node *of_icc_get_from_provider(struct of_phandle_args *spec) struct icc_node *node = ERR_PTR(-EPROBE_DEFER); struct icc_provider *provider; - if (!spec || spec->args_count != 1) + if (!spec || spec->args_count < 1) return ERR_PTR(-EINVAL); mutex_lock(&icc_lock); @@ -469,6 +469,9 @@ struct icc_path *of_icc_get_by_index(struct device *dev, int idx) return ERR_PTR(-ENOMEM); } + if (src_args.args_count == 2) + icc_set_tag(path, src_args.args[1]); diff: https://paste.ubuntu.com/p/sRRYhxQjsV/ Saravana/Georgi, A few concerns here, I feel tag info as the second arg to the provider may not be true for all socs. Does introducing soc specific of_icc_get functions make sense? > >>> >>> Also, is there an example for interconnect-tags that I missed? Is it a >>> list of strings, numbers, etc? >> >> Saravana, >> https://patchwork.kernel.org/patch/11527589/ >> ^^ is an example of interconnect-tag useage. > > If we actually merge interconnect-tags, I think the doc should be > updated. Instead of having to grep around. > > -Saravana >
diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt index 6f5d23a605b7..c1a226a934e5 100644 --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same * dma-mem: Path from the device to the main memory of the system +interconnect-tags : List of interconnect path tags sorted in the same order as the + interconnects property. Consumers can append a specific tag to + the path and pass this information to the interconnect framework + to do aggregation based on the attached tag. + Example: sdhci@7864000 {