From: Sibi Sankar <[email protected]>
Add interconnect-tags bindings to enable passing of optional
tag information to the interconnect framework.
Signed-off-by: Sibi Sankar <[email protected]>
Signed-off-by: Georgi Djakov <[email protected]>
---
v8:
* New patch, picked from here:
https://lore.kernel.org/r/[email protected]
.../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 {
On 12-05-20, 15:53, Georgi Djakov wrote:
> From: Sibi Sankar <[email protected]>
>
> Add interconnect-tags bindings to enable passing of optional
> tag information to the interconnect framework.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> Signed-off-by: Georgi Djakov <[email protected]>
> ---
> v8:
> * New patch, picked from here:
> https://lore.kernel.org/r/[email protected]
>
> .../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).
--
viresh
On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
> From: Sibi Sankar <[email protected]>
>
> Add interconnect-tags bindings to enable passing of optional
> tag information to the interconnect framework.
>
> Signed-off-by: Sibi Sankar <[email protected]>
> Signed-off-by: Georgi Djakov <[email protected]>
> ---
> v8:
> * New patch, picked from here:
> https://lore.kernel.org/r/[email protected]
>
> .../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 <[email protected]> wrote:
>
> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
> > From: Sibi Sankar <[email protected]>
> >
> > Add interconnect-tags bindings to enable passing of optional
> > tag information to the interconnect framework.
> >
> > Signed-off-by: Sibi Sankar <[email protected]>
> > Signed-off-by: Georgi Djakov <[email protected]>
> > ---
> > v8:
> > * New patch, picked from here:
> > https://lore.kernel.org/r/[email protected]
> >
> > .../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 <[email protected]> wrote:
>>
>> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
>> > From: Sibi Sankar <[email protected]>
>> >
>> > Add interconnect-tags bindings to enable passing of optional
>> > tag information to the interconnect framework.
>> >
>> > Signed-off-by: Sibi Sankar <[email protected]>
>> > Signed-off-by: Georgi Djakov <[email protected]>
>> > ---
>> > v8:
>> > * New patch, picked from here:
>> > https://lore.kernel.org/r/[email protected]
>> >
>> > .../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
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
On Wed, May 20, 2020 at 11:51 AM Sibi Sankar <[email protected]> wrote:
>
> On 2020-05-20 01:27, Saravana Kannan wrote:
> > On Tue, May 19, 2020 at 11:58 AM Rob Herring <[email protected]> wrote:
> >>
> >> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
> >> > From: Sibi Sankar <[email protected]>
> >> >
> >> > Add interconnect-tags bindings to enable passing of optional
> >> > tag information to the interconnect framework.
> >> >
> >> > Signed-off-by: Sibi Sankar <[email protected]>
> >> > Signed-off-by: Georgi Djakov <[email protected]>
> >> > ---
> >> > v8:
> >> > * New patch, picked from here:
> >> > https://lore.kernel.org/r/[email protected]
> >> >
> >> > .../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 <[email protected]> wrote:
>>
>> On 2020-05-20 01:27, Saravana Kannan wrote:
>>> On Tue, May 19, 2020 at 11:58 AM Rob Herring <[email protected]> wrote:
>>>>
>>>> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
>>>>> From: Sibi Sankar <[email protected]>
>>>>>
>>>>> Add interconnect-tags bindings to enable passing of optional
>>>>> tag information to the interconnect framework.
>>>>>
>>>>> Signed-off-by: Sibi Sankar <[email protected]>
>>>>> Signed-off-by: Georgi Djakov <[email protected]>
>>>>> ---
>>>>> v8:
>>>>> * New patch, picked from here:
>>>>> https://lore.kernel.org/r/[email protected]
>>>>>
>>>>> .../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
>
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc, is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project