diff mbox series

[RFC,v6,1/9] dt-bindings: interconnect: Add bindings for imx8m noc

Message ID 6db2ce55ee62dd8548aa8e1e0ecdf8c06eda868f.1573761527.git.leonard.crestez@nxp.com (mailing list archive)
State Superseded
Headers show
Series interconnect: Add imx support via devfreq | expand

Commit Message

Leonard Crestez Nov. 14, 2019, 8:09 p.m. UTC
Add initial dt bindings for the interconnects inside i.MX chips.
Multiple external IPs are involved but SOC integration means the
software controllable interfaces are very similar.

Main NOC node acts as interconnect provider if #interconnect-cells is
present.

Multiple interconnects can be present, each with their own OPP table.

Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
---
 .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
 1 file changed, 104 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml

Comments

Chanwoo Choi Dec. 16, 2019, 1:12 a.m. UTC | #1
On 11/15/19 5:09 AM, Leonard Crestez wrote:
> Add initial dt bindings for the interconnects inside i.MX chips.
> Multiple external IPs are involved but SOC integration means the
> software controllable interfaces are very similar.
> 
> Main NOC node acts as interconnect provider if #interconnect-cells is
> present.
> 
> Multiple interconnects can be present, each with their own OPP table.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> ---
>  .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>  1 file changed, 104 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> new file mode 100644
> index 000000000000..5cd94185fec3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> @@ -0,0 +1,104 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: https://protect2.fireeye.com/url?k=0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd&u=http://devicetree.org/schemas/interconnect/fsl,imx8m-noc.yaml#
> +$schema: https://protect2.fireeye.com/url?k=87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a&u=http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic i.MX bus frequency device
> +
> +maintainers:
> +  - Leonard Crestez <leonard.crestez@nxp.com>
> +
> +description: |
> +  The i.MX SoC family has multiple buses for which clock frequency (and
> +  sometimes voltage) can be adjusted.
> +
> +  Some of those buses expose register areas mentioned in the memory maps as GPV
> +  ("Global Programmers View") but not all. Access to this area might be denied
> +  for normal (non-secure) world.
> +
> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
> +  interconnect IPs into imx SOCs.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +        - enum:
> +          - fsl,imx8mn-nic
> +          - fsl,imx8mm-nic
> +          - fsl,imx8mq-nic
> +        - const: fsl,imx8m-nic
> +      - items:
> +        - enum:
> +          - fsl,imx8mn-noc
> +          - fsl,imx8mm-noc
> +          - fsl,imx8mq-noc
> +        - const: fsl,imx8m-noc
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  operating-points-v2: true
> +  opp-table: true
> +
> +  devfreq:
> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> +    description:
> +      Phandle to another devfreq device to match OPPs with by using the

Better to use 'parent' instead of 'another' word for improving the understanding.

> +      passive governor.
> +
> +  '#interconnect-cells':
> +    description:
> +      If specified then also act as an interconnect provider. Should only be
> +      set once per soc on main noc.
> +    const: 1
> +
> +  fsl,scalable-node-ids:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Array of node ids for scalable nodes. Uses same numeric identifier
> +      namespace as the consumer "interconnects" binding.
> +
> +  fsl,scalable-nodes:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    description:
> +      Array of phandles to scalable nodes. Must be of same length as
> +      fsl,scalable-node-ids.
> +
> +required:
> +  - compatible
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |

Is it enough example to understand the relation between
imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?

In my case, if possible, hope to show the more detailed
example. This example seems that don't contain the ddrc
dt node (imx8m-ddrc.c).

> +    #include <dt-bindings/clock/imx8mq-clock.h>
> +    #include <dt-bindings/interconnect/imx8mq.h>
> +    noc: interconnect@32700000 {
> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
> +            reg = <0x32700000 0x100000>;
> +            clocks = <&clk IMX8MQ_CLK_NOC>;
> +            #interconnect-cells = <1>;
> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
> +                                    <IMX8MQ_ICS_DRAM>;
> +            fsl,scalable-nodes = <&noc>,
> +                                 <&ddrc>;
> +            operating-points-v2 = <&noc_opp_table>;
> +
> +            noc_opp_table: opp-table {
> +                    compatible = "operating-points-v2";
> +
> +                    opp-133M {
> +                            opp-hz = /bits/ 64 <133333333>;
> +                    };
> +                    opp-800M {
> +                            opp-hz = /bits/ 64 <800000000>;
> +                    };
> +            };
> +    };
>
Chanwoo Choi Dec. 16, 2019, 3:25 a.m. UTC | #2
Hi,

On 12/16/19 10:12 AM, Chanwoo Choi wrote:
> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>> Add initial dt bindings for the interconnects inside i.MX chips.
>> Multiple external IPs are involved but SOC integration means the
>> software controllable interfaces are very similar.
>>
>> Main NOC node acts as interconnect provider if #interconnect-cells is
>> present.
>>
>> Multiple interconnects can be present, each with their own OPP table.
>>
>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>> ---
>>  .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>>  1 file changed, 104 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>> new file mode 100644
>> index 000000000000..5cd94185fec3
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>> @@ -0,0 +1,104 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/url?k=0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd&u=http://devicetree.org/schemas/interconnect/fsl,imx8m-noc.yaml#
>> +$schema: https://protect2.fireeye.com/url?k=87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a&u=http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Generic i.MX bus frequency device
>> +
>> +maintainers:
>> +  - Leonard Crestez <leonard.crestez@nxp.com>
>> +
>> +description: |
>> +  The i.MX SoC family has multiple buses for which clock frequency (and
>> +  sometimes voltage) can be adjusted.
>> +
>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>> +  ("Global Programmers View") but not all. Access to this area might be denied
>> +  for normal (non-secure) world.
>> +
>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
>> +  interconnect IPs into imx SOCs.
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +        - enum:
>> +          - fsl,imx8mn-nic
>> +          - fsl,imx8mm-nic
>> +          - fsl,imx8mq-nic
>> +        - const: fsl,imx8m-nic
>> +      - items:
>> +        - enum:
>> +          - fsl,imx8mn-noc
>> +          - fsl,imx8mm-noc
>> +          - fsl,imx8mq-noc
>> +        - const: fsl,imx8m-noc
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  operating-points-v2: true
>> +  opp-table: true
>> +
>> +  devfreq:
>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>> +    description:
>> +      Phandle to another devfreq device to match OPPs with by using the
> 
> Better to use 'parent' instead of 'another' word for improving the understanding.

I think that 'devfreq' is not proper way to get the parent node
in devicetree. Because 'devfreq' name is linuxium. The property name
didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.

So, you better to make the specific property for this device driver
like as following: and use devfreq_get_devfreq_by_node() function
which is developed by you in order to get the devfreq instance node.

	fsl,parent-device = <&parent devfreq device>;

[1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node

> 
>> +      passive governor.
>> +
>> +  '#interconnect-cells':
>> +    description:
>> +      If specified then also act as an interconnect provider. Should only be
>> +      set once per soc on main noc.
>> +    const: 1
>> +
>> +  fsl,scalable-node-ids:
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>> +    description:
>> +      Array of node ids for scalable nodes. Uses same numeric identifier
>> +      namespace as the consumer "interconnects" binding.
>> +
>> +  fsl,scalable-nodes:
>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>> +    description:
>> +      Array of phandles to scalable nodes. Must be of same length as
>> +      fsl,scalable-node-ids.
>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
> 
> Is it enough example to understand the relation between
> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
> 
> In my case, if possible, hope to show the more detailed
> example. This example seems that don't contain the ddrc
> dt node (imx8m-ddrc.c).
> 
>> +    #include <dt-bindings/clock/imx8mq-clock.h>
>> +    #include <dt-bindings/interconnect/imx8mq.h>
>> +    noc: interconnect@32700000 {
>> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
>> +            reg = <0x32700000 0x100000>;
>> +            clocks = <&clk IMX8MQ_CLK_NOC>;
>> +            #interconnect-cells = <1>;
>> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
>> +                                    <IMX8MQ_ICS_DRAM>;
>> +            fsl,scalable-nodes = <&noc>,
>> +                                 <&ddrc>;
>> +            operating-points-v2 = <&noc_opp_table>;
>> +
>> +            noc_opp_table: opp-table {
>> +                    compatible = "operating-points-v2";
>> +
>> +                    opp-133M {
>> +                            opp-hz = /bits/ 64 <133333333>;
>> +                    };
>> +                    opp-800M {
>> +                            opp-hz = /bits/ 64 <800000000>;
>> +                    };
>> +            };
>> +    };
>>
> 
>
Leonard Crestez Dec. 16, 2019, 3:09 p.m. UTC | #3
On 16.12.2019 05:18, Chanwoo Choi wrote:
> Hi,
> 
> On 12/16/19 10:12 AM, Chanwoo Choi wrote:
>> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>>> Add initial dt bindings for the interconnects inside i.MX chips.
>>> Multiple external IPs are involved but SOC integration means the
>>> software controllable interfaces are very similar.
>>>
>>> Main NOC node acts as interconnect provider if #interconnect-cells is
>>> present.
>>>
>>> Multiple interconnects can be present, each with their own OPP table.
>>>
>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>> ---
>>>   .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>>>   1 file changed, 104 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>> new file mode 100644
>>> index 000000000000..5cd94185fec3
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>> @@ -0,0 +1,104 @@
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +%YAML 1.2
>>> +---
>>> +$id: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd%26u%3Dhttp%3A%2F%2Fdevicetree.org%2Fschemas%2Finterconnect%2Ffsl%2Cimx8m-noc.yaml%23&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&amp;sdata=H2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%2BK%2BGFE%3D&amp;reserved=0
>>> +$schema: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a%26u%3Dhttp%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&amp;sdata=T6PgQ1DWI4OLOx3gifRRt%2FNImdVrgDUoswZ%2FNKw3oR8%3D&amp;reserved=0
>>> +
>>> +title: Generic i.MX bus frequency device
>>> +
>>> +maintainers:
>>> +  - Leonard Crestez <leonard.crestez@nxp.com>
>>> +
>>> +description: |
>>> +  The i.MX SoC family has multiple buses for which clock frequency (and
>>> +  sometimes voltage) can be adjusted.
>>> +
>>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>>> +  ("Global Programmers View") but not all. Access to this area might be denied
>>> +  for normal (non-secure) world.
>>> +
>>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
>>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
>>> +  interconnect IPs into imx SOCs.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - items:
>>> +        - enum:
>>> +          - fsl,imx8mn-nic
>>> +          - fsl,imx8mm-nic
>>> +          - fsl,imx8mq-nic
>>> +        - const: fsl,imx8m-nic
>>> +      - items:
>>> +        - enum:
>>> +          - fsl,imx8mn-noc
>>> +          - fsl,imx8mm-noc
>>> +          - fsl,imx8mq-noc
>>> +        - const: fsl,imx8m-noc
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  clocks:
>>> +    maxItems: 1
>>> +
>>> +  operating-points-v2: true
>>> +  opp-table: true
>>> +
>>> +  devfreq:
>>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>>> +    description:
>>> +      Phandle to another devfreq device to match OPPs with by using the
>>
>> Better to use 'parent' instead of 'another' word for improving the understanding.
> 
> I think that 'devfreq' is not proper way to get the parent node
> in devicetree. Because 'devfreq' name is linuxium. The property name
> didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.
> 
> So, you better to make the specific property for this device driver
> like as following: and use devfreq_get_devfreq_by_node() function
> which is developed by you in order to get the devfreq instance node.
> 
> 	fsl,parent-device = <&parent devfreq device>;

This is only a "parent" in the sense that it's assigned to 
devfreq_passive.data.parent. The "devfreq" name is indeed too generic.

The DDRC can measure bandwith usage and I want to use the passive 
governor to make the main NOC match OPPs. But at the bus level DDRC only 
has AXI and APB slave ports.

Buses on imx don't have a parent/child relationship; in fact there are 
even a few cycles.

> 
> [1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node
> 
>>
>>> +      passive governor.
>>> +
>>> +  '#interconnect-cells':
>>> +    description:
>>> +      If specified then also act as an interconnect provider. Should only be
>>> +      set once per soc on main noc.
>>> +    const: 1
>>> +
>>> +  fsl,scalable-node-ids:
>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>> +    description:
>>> +      Array of node ids for scalable nodes. Uses same numeric identifier
>>> +      namespace as the consumer "interconnects" binding.
>>> +
>>> +  fsl,scalable-nodes:
>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +    description:
>>> +      Array of phandles to scalable nodes. Must be of same length as
>>> +      fsl,scalable-node-ids.
>>> +
>>> +required:
>>> +  - compatible
>>> +  - clocks
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>
>> Is it enough example to understand the relation between
>> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
>>
>> In my case, if possible, hope to show the more detailed
>> example. This example seems that don't contain the ddrc
>> dt node (imx8m-ddrc.c).

OK, I'll elaborate explanation on noc binding.

>>
>>> +    #include <dt-bindings/clock/imx8mq-clock.h>
>>> +    #include <dt-bindings/interconnect/imx8mq.h>
>>> +    noc: interconnect@32700000 {
>>> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
>>> +            reg = <0x32700000 0x100000>;
>>> +            clocks = <&clk IMX8MQ_CLK_NOC>;
>>> +            #interconnect-cells = <1>;
>>> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
>>> +                                    <IMX8MQ_ICS_DRAM>;
>>> +            fsl,scalable-nodes = <&noc>,
>>> +                                 <&ddrc>;
>>> +            operating-points-v2 = <&noc_opp_table>;
>>> +
>>> +            noc_opp_table: opp-table {
>>> +                    compatible = "operating-points-v2";
>>> +
>>> +                    opp-133M {
>>> +                            opp-hz = /bits/ 64 <133333333>;
>>> +                    };
>>> +                    opp-800M {
>>> +                            opp-hz = /bits/ 64 <800000000>;
>>> +                    };
>>> +            };
>>> +    };
Chanwoo Choi Dec. 17, 2019, 12:15 a.m. UTC | #4
On 12/17/19 12:09 AM, Leonard Crestez wrote:
> On 16.12.2019 05:18, Chanwoo Choi wrote:
>> Hi,
>>
>> On 12/16/19 10:12 AM, Chanwoo Choi wrote:
>>> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>>>> Add initial dt bindings for the interconnects inside i.MX chips.
>>>> Multiple external IPs are involved but SOC integration means the
>>>> software controllable interfaces are very similar.
>>>>
>>>> Main NOC node acts as interconnect provider if #interconnect-cells is
>>>> present.
>>>>
>>>> Multiple interconnects can be present, each with their own OPP table.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>> ---
>>>>   .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>>>>   1 file changed, 104 insertions(+)
>>>>   create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>> new file mode 100644
>>>> index 000000000000..5cd94185fec3
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>> @@ -0,0 +1,104 @@
>>>> +# SPDX-License-Identifier: GPL-2.0
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: https://protect2.fireeye.com/url?k=8570eb5a-d8a45732-85716015-0cc47a3356b2-92a5b92cc514d07e&u=https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd%26u%3Dhttp%3A%2F%2Fdevicetree.org%2Fschemas%2Finterconnect%2Ffsl%2Cimx8m-noc.yaml%23&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&amp;sdata=H2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%2BK%2BGFE%3D&amp;reserved=0
>>>> +$schema: https://protect2.fireeye.com/url?k=f7cec483-aa1a78eb-f7cf4fcc-0cc47a3356b2-4154a3c43886f5ed&u=https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a%26u%3Dhttp%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&amp;sdata=T6PgQ1DWI4OLOx3gifRRt%2FNImdVrgDUoswZ%2FNKw3oR8%3D&amp;reserved=0
>>>> +
>>>> +title: Generic i.MX bus frequency device
>>>> +
>>>> +maintainers:
>>>> +  - Leonard Crestez <leonard.crestez@nxp.com>
>>>> +
>>>> +description: |
>>>> +  The i.MX SoC family has multiple buses for which clock frequency (and
>>>> +  sometimes voltage) can be adjusted.
>>>> +
>>>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>>>> +  ("Global Programmers View") but not all. Access to this area might be denied
>>>> +  for normal (non-secure) world.
>>>> +
>>>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
>>>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
>>>> +  interconnect IPs into imx SOCs.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - items:
>>>> +        - enum:
>>>> +          - fsl,imx8mn-nic
>>>> +          - fsl,imx8mm-nic
>>>> +          - fsl,imx8mq-nic
>>>> +        - const: fsl,imx8m-nic
>>>> +      - items:
>>>> +        - enum:
>>>> +          - fsl,imx8mn-noc
>>>> +          - fsl,imx8mm-noc
>>>> +          - fsl,imx8mq-noc
>>>> +        - const: fsl,imx8m-noc
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  clocks:
>>>> +    maxItems: 1
>>>> +
>>>> +  operating-points-v2: true
>>>> +  opp-table: true
>>>> +
>>>> +  devfreq:
>>>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>>>> +    description:
>>>> +      Phandle to another devfreq device to match OPPs with by using the
>>>
>>> Better to use 'parent' instead of 'another' word for improving the understanding.
>>
>> I think that 'devfreq' is not proper way to get the parent node
>> in devicetree. Because 'devfreq' name is linuxium. The property name
>> didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.
>>
>> So, you better to make the specific property for this device driver
>> like as following: and use devfreq_get_devfreq_by_node() function
>> which is developed by you in order to get the devfreq instance node.
>>
>> 	fsl,parent-device = <&parent devfreq device>;
> 
> This is only a "parent" in the sense that it's assigned to 
> devfreq_passive.data.parent. The "devfreq" name is indeed too generic.

I thought that 'devfreq' property name is generic.
But, it's not proper for DT binding because DT file show
the h/w and the relation of h/w. 'devfreq' property name
has not meant h/w.

So that devfreq core doesn't force to use the specific property
name to get the devfreq parent instance on DT. Just, devfreq core
will provide devfreq_get_devfreq_by_node() function.

I developed it on devfre-testing branch[2]. Before I'm sending
the these patches, you can check them.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=f3678b4e6b75dccfe8bb87d005da2d68c70fdeab
[2] https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/log/?h=devfreq-testing

> 
> The DDRC can measure bandwith usage and I want to use the passive 
> governor to make the main NOC match OPPs.

which one use the passive governor? And which one is the parent 
devfreq device for devfreq device using passive governor?

In my case, it is difficult to catch the relationship
between devices. I'd like you to explain the detailed relationship
on binding document for user.

> But at the bus level DDRC only has AXI and APB slave ports.

'DDRC' indicates the 'drivers/devfreq/imx8m-ddrc.c?

> 
> Buses on imx don't have a parent/child relationship; in fact there are 
> even a few cycles.

You mentioned that 'imx don't have a parent/child relationship',
Why do you use 'passive' governor? It is difficult to understand
the hierarchy of imx.

> 
>>
>> [1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node
>>
>>>
>>>> +      passive governor.
>>>> +
>>>> +  '#interconnect-cells':
>>>> +    description:
>>>> +      If specified then also act as an interconnect provider. Should only be
>>>> +      set once per soc on main noc.
>>>> +    const: 1
>>>> +
>>>> +  fsl,scalable-node-ids:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> +    description:
>>>> +      Array of node ids for scalable nodes. Uses same numeric identifier
>>>> +      namespace as the consumer "interconnects" binding.
>>>> +
>>>> +  fsl,scalable-nodes:
>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>> +    description:
>>>> +      Array of phandles to scalable nodes. Must be of same length as
>>>> +      fsl,scalable-node-ids.
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - clocks
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>
>>> Is it enough example to understand the relation between
>>> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
>>>
>>> In my case, if possible, hope to show the more detailed
>>> example. This example seems that don't contain the ddrc
>>> dt node (imx8m-ddrc.c).
> 
> OK, I'll elaborate explanation on noc binding.

Thanks. If possible, you better to add almost example cases.

> 
>>>
>>>> +    #include <dt-bindings/clock/imx8mq-clock.h>
>>>> +    #include <dt-bindings/interconnect/imx8mq.h>
>>>> +    noc: interconnect@32700000 {
>>>> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
>>>> +            reg = <0x32700000 0x100000>;
>>>> +            clocks = <&clk IMX8MQ_CLK_NOC>;
>>>> +            #interconnect-cells = <1>;
>>>> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
>>>> +                                    <IMX8MQ_ICS_DRAM>;
>>>> +            fsl,scalable-nodes = <&noc>,
>>>> +                                 <&ddrc>;
>>>> +            operating-points-v2 = <&noc_opp_table>;
>>>> +
>>>> +            noc_opp_table: opp-table {
>>>> +                    compatible = "operating-points-v2";
>>>> +
>>>> +                    opp-133M {
>>>> +                            opp-hz = /bits/ 64 <133333333>;
>>>> +                    };
>>>> +                    opp-800M {
>>>> +                            opp-hz = /bits/ 64 <800000000>;
>>>> +                    };
>>>> +            };
>>>> +    };
> 
>
Leonard Crestez Dec. 19, 2019, 2:31 p.m. UTC | #5
On 17.12.2019 02:08, Chanwoo Choi wrote:
> On 12/17/19 12:09 AM, Leonard Crestez wrote:
>> On 16.12.2019 05:18, Chanwoo Choi wrote:
>>> Hi,
>>>
>>> On 12/16/19 10:12 AM, Chanwoo Choi wrote:
>>>> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>>>>> Add initial dt bindings for the interconnects inside i.MX chips.
>>>>> Multiple external IPs are involved but SOC integration means the
>>>>> software controllable interfaces are very similar.
>>>>>
>>>>> Main NOC node acts as interconnect provider if #interconnect-cells is
>>>>> present.
>>>>>
>>>>> Multiple interconnects can be present, each with their own OPP table.
>>>>>
>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>> ---
>>>>>    .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>>>>>    1 file changed, 104 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..5cd94185fec3
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>> @@ -0,0 +1,104 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D8570eb5a-d8a45732-85716015-0cc47a3356b2-92a5b92cc514d07e%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fschemas%252Finterconnect%252Ffsl%252Cimx8m-noc.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DH2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%252BK%252BGFE%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=HYMJJHWyiKRhf7GDjKoOwjDpcZuYqlFlmRrDZnIRx5w%3D&amp;reserved=0
>>>>> +$schema: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3Df7cec483-aa1a78eb-f7cf4fcc-0cc47a3356b2-4154a3c43886f5ed%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fmeta-schemas%252Fcore.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DT6PgQ1DWI4OLOx3gifRRt%252FNImdVrgDUoswZ%252FNKw3oR8%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=fIbrUUOUtZ5nt%2FH1tm3dzaI1J%2FGn5Gc54ms93HnBnQg%3D&amp;reserved=0
>>>>> +
>>>>> +title: Generic i.MX bus frequency device
>>>>> +
>>>>> +maintainers:
>>>>> +  - Leonard Crestez <leonard.crestez@nxp.com>
>>>>> +
>>>>> +description: |
>>>>> +  The i.MX SoC family has multiple buses for which clock frequency (and
>>>>> +  sometimes voltage) can be adjusted.
>>>>> +
>>>>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>>>>> +  ("Global Programmers View") but not all. Access to this area might be denied
>>>>> +  for normal (non-secure) world.
>>>>> +
>>>>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
>>>>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
>>>>> +  interconnect IPs into imx SOCs.
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    oneOf:
>>>>> +      - items:
>>>>> +        - enum:
>>>>> +          - fsl,imx8mn-nic
>>>>> +          - fsl,imx8mm-nic
>>>>> +          - fsl,imx8mq-nic
>>>>> +        - const: fsl,imx8m-nic
>>>>> +      - items:
>>>>> +        - enum:
>>>>> +          - fsl,imx8mn-noc
>>>>> +          - fsl,imx8mm-noc
>>>>> +          - fsl,imx8mq-noc
>>>>> +        - const: fsl,imx8m-noc
>>>>> +
>>>>> +  reg:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  clocks:
>>>>> +    maxItems: 1
>>>>> +
>>>>> +  operating-points-v2: true
>>>>> +  opp-table: true
>>>>> +
>>>>> +  devfreq:
>>>>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>>>>> +    description:
>>>>> +      Phandle to another devfreq device to match OPPs with by using the
>>>>
>>>> Better to use 'parent' instead of 'another' word for improving the understanding.
>>>
>>> I think that 'devfreq' is not proper way to get the parent node
>>> in devicetree. Because 'devfreq' name is linuxium. The property name
>>> didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.
>>>
>>> So, you better to make the specific property for this device driver
>>> like as following: and use devfreq_get_devfreq_by_node() function
>>> which is developed by you in order to get the devfreq instance node.
>>>
>>> 	fsl,parent-device = <&parent devfreq device>;
>>
>> This is only a "parent" in the sense that it's assigned to
>> devfreq_passive.data.parent. The "devfreq" name is indeed too generic.
> 
> I thought that 'devfreq' property name is generic.
> But, it's not proper for DT binding because DT file show
> the h/w and the relation of h/w. 'devfreq' property name
> has not meant h/w.
> 
> So that devfreq core doesn't force to use the specific property
> name to get the devfreq parent instance on DT. Just, devfreq core
> will provide devfreq_get_devfreq_by_node() function.
> 
> I developed it on devfre-testing branch[2]. Before I'm sending
> the these patches, you can check them.
> 
> [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Fcommit%2F%3Fh%3Ddevfreq-testing%26id%3Df3678b4e6b75dccfe8bb87d005da2d68c70fdeab&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=R21Tv1QgofBvMYb2VaxFjKSerwQ3tl8kakcYRyALmgM%3D&amp;reserved=0
> [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Flog%2F%3Fh%3Ddevfreq-testing&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=uH0d9LvrbCHgZJdrPJNJ8c8w45J7x1QyM7t5I3j%2BpSw%3D&amp;reserved=0
> 
>>
>> The DDRC can measure bandwith usage and I want to use the passive
>> governor to make the main NOC match OPPs.
> 
> which one use the passive governor? And which one is the parent
> devfreq device for devfreq device using passive governor?
> 
> In my case, it is difficult to catch the relationship
> between devices. I'd like you to explain the detailed relationship
> on binding document for user.
> 
>> But at the bus level DDRC only has AXI and APB slave ports.
> 
> 'DDRC' indicates the 'drivers/devfreq/imx8m-ddrc.c?
> 
>>
>> Buses on imx don't have a parent/child relationship; in fact there are
>> even a few cycles.
> 
> You mentioned that 'imx don't have a parent/child relationship',
> Why do you use 'passive' governor? It is difficult to understand
> the hierarchy of imx.

The imx8m has a main NOC in the center between the CPU and the DDRC 
(dram controller) with many other peripheral buses around the NOC.

Here's /sys/kernel/debug/interconnect/interconnect_graph on imx8mm:
https://gist.github.com/cdleonard/84d103dafc9131fcb8ca9a494822a131#file-imx8mm-svg

A lot of stuff is omitted, it mostly just includes high-performance bus 
masters.

DDRC has a performance monitor attached which can measure current 
bandwith and feed it to an ondemand governor. I want to use passive 
governor on the NOC so that it matches frequencies with DDRC and scales 
proportionally, otherwise if NOC is at low frequency then dynamically 
scaling up the DDRC might be ineffective.

Perhaps you could explain how parent/child relationships work on exynos?

>>> [1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node
>>>
>>>>
>>>>> +      passive governor.
>>>>> +
>>>>> +  '#interconnect-cells':
>>>>> +    description:
>>>>> +      If specified then also act as an interconnect provider. Should only be
>>>>> +      set once per soc on main noc.
>>>>> +    const: 1
>>>>> +
>>>>> +  fsl,scalable-node-ids:
>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>> +    description:
>>>>> +      Array of node ids for scalable nodes. Uses same numeric identifier
>>>>> +      namespace as the consumer "interconnects" binding.
>>>>> +
>>>>> +  fsl,scalable-nodes:
>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>> +    description:
>>>>> +      Array of phandles to scalable nodes. Must be of same length as
>>>>> +      fsl,scalable-node-ids.
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>> +  - clocks
>>>>> +
>>>>> +additionalProperties: false
>>>>> +
>>>>> +examples:
>>>>> +  - |
>>>>
>>>> Is it enough example to understand the relation between
>>>> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
>>>>
>>>> In my case, if possible, hope to show the more detailed
>>>> example. This example seems that don't contain the ddrc
>>>> dt node (imx8m-ddrc.c).
>>
>> OK, I'll elaborate explanation on noc binding.
> 
> Thanks. If possible, you better to add almost example cases.
> 
>>
>>>>
>>>>> +    #include <dt-bindings/clock/imx8mq-clock.h>
>>>>> +    #include <dt-bindings/interconnect/imx8mq.h>
>>>>> +    noc: interconnect@32700000 {
>>>>> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
>>>>> +            reg = <0x32700000 0x100000>;
>>>>> +            clocks = <&clk IMX8MQ_CLK_NOC>;
>>>>> +            #interconnect-cells = <1>;
>>>>> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
>>>>> +                                    <IMX8MQ_ICS_DRAM>;
>>>>> +            fsl,scalable-nodes = <&noc>,
>>>>> +                                 <&ddrc>;
>>>>> +            operating-points-v2 = <&noc_opp_table>;
>>>>> +
>>>>> +            noc_opp_table: opp-table {
>>>>> +                    compatible = "operating-points-v2";
>>>>> +
>>>>> +                    opp-133M {
>>>>> +                            opp-hz = /bits/ 64 <133333333>;
>>>>> +                    };
>>>>> +                    opp-800M {
>>>>> +                            opp-hz = /bits/ 64 <800000000>;
>>>>> +                    };
>>>>> +            };
>>>>> +    };
>>
>>
> 
>
Chanwoo Choi Dec. 19, 2019, 3:55 p.m. UTC | #6
2019년 12월 19일 (목) 오후 11:33, Leonard Crestez <leonard.crestez@nxp.com>님이 작성:
>
> On 17.12.2019 02:08, Chanwoo Choi wrote:
> > On 12/17/19 12:09 AM, Leonard Crestez wrote:
> >> On 16.12.2019 05:18, Chanwoo Choi wrote:
> >>> Hi,
> >>>
> >>> On 12/16/19 10:12 AM, Chanwoo Choi wrote:
> >>>> On 11/15/19 5:09 AM, Leonard Crestez wrote:
> >>>>> Add initial dt bindings for the interconnects inside i.MX chips.
> >>>>> Multiple external IPs are involved but SOC integration means the
> >>>>> software controllable interfaces are very similar.
> >>>>>
> >>>>> Main NOC node acts as interconnect provider if #interconnect-cells is
> >>>>> present.
> >>>>>
> >>>>> Multiple interconnects can be present, each with their own OPP table.
> >>>>>
> >>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> >>>>> ---
> >>>>>    .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
> >>>>>    1 file changed, 104 insertions(+)
> >>>>>    create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000000..5cd94185fec3
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> >>>>> @@ -0,0 +1,104 @@
> >>>>> +# SPDX-License-Identifier: GPL-2.0
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D8570eb5a-d8a45732-85716015-0cc47a3356b2-92a5b92cc514d07e%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fschemas%252Finterconnect%252Ffsl%252Cimx8m-noc.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DH2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%252BK%252BGFE%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=HYMJJHWyiKRhf7GDjKoOwjDpcZuYqlFlmRrDZnIRx5w%3D&amp;reserved=0
> >>>>> +$schema: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3Df7cec483-aa1a78eb-f7cf4fcc-0cc47a3356b2-4154a3c43886f5ed%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fmeta-schemas%252Fcore.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DT6PgQ1DWI4OLOx3gifRRt%252FNImdVrgDUoswZ%252FNKw3oR8%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=fIbrUUOUtZ5nt%2FH1tm3dzaI1J%2FGn5Gc54ms93HnBnQg%3D&amp;reserved=0
> >>>>> +
> >>>>> +title: Generic i.MX bus frequency device
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Leonard Crestez <leonard.crestez@nxp.com>
> >>>>> +
> >>>>> +description: |
> >>>>> +  The i.MX SoC family has multiple buses for which clock frequency (and
> >>>>> +  sometimes voltage) can be adjusted.
> >>>>> +
> >>>>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
> >>>>> +  ("Global Programmers View") but not all. Access to this area might be denied
> >>>>> +  for normal (non-secure) world.
> >>>>> +
> >>>>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
> >>>>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
> >>>>> +  interconnect IPs into imx SOCs.
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    oneOf:
> >>>>> +      - items:
> >>>>> +        - enum:
> >>>>> +          - fsl,imx8mn-nic
> >>>>> +          - fsl,imx8mm-nic
> >>>>> +          - fsl,imx8mq-nic
> >>>>> +        - const: fsl,imx8m-nic
> >>>>> +      - items:
> >>>>> +        - enum:
> >>>>> +          - fsl,imx8mn-noc
> >>>>> +          - fsl,imx8mm-noc
> >>>>> +          - fsl,imx8mq-noc
> >>>>> +        - const: fsl,imx8m-noc
> >>>>> +
> >>>>> +  reg:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  clocks:
> >>>>> +    maxItems: 1
> >>>>> +
> >>>>> +  operating-points-v2: true
> >>>>> +  opp-table: true
> >>>>> +
> >>>>> +  devfreq:
> >>>>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
> >>>>> +    description:
> >>>>> +      Phandle to another devfreq device to match OPPs with by using the
> >>>>
> >>>> Better to use 'parent' instead of 'another' word for improving the understanding.
> >>>
> >>> I think that 'devfreq' is not proper way to get the parent node
> >>> in devicetree. Because 'devfreq' name is linuxium. The property name
> >>> didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.
> >>>
> >>> So, you better to make the specific property for this device driver
> >>> like as following: and use devfreq_get_devfreq_by_node() function
> >>> which is developed by you in order to get the devfreq instance node.
> >>>
> >>>     fsl,parent-device = <&parent devfreq device>;
> >>
> >> This is only a "parent" in the sense that it's assigned to
> >> devfreq_passive.data.parent. The "devfreq" name is indeed too generic.
> >
> > I thought that 'devfreq' property name is generic.
> > But, it's not proper for DT binding because DT file show
> > the h/w and the relation of h/w. 'devfreq' property name
> > has not meant h/w.
> >
> > So that devfreq core doesn't force to use the specific property
> > name to get the devfreq parent instance on DT. Just, devfreq core
> > will provide devfreq_get_devfreq_by_node() function.
> >
> > I developed it on devfre-testing branch[2]. Before I'm sending
> > the these patches, you can check them.
> >
> > [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Fcommit%2F%3Fh%3Ddevfreq-testing%26id%3Df3678b4e6b75dccfe8bb87d005da2d68c70fdeab&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=R21Tv1QgofBvMYb2VaxFjKSerwQ3tl8kakcYRyALmgM%3D&amp;reserved=0
> > [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Flog%2F%3Fh%3Ddevfreq-testing&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&amp;sdata=uH0d9LvrbCHgZJdrPJNJ8c8w45J7x1QyM7t5I3j%2BpSw%3D&amp;reserved=0
> >
> >>
> >> The DDRC can measure bandwith usage and I want to use the passive
> >> governor to make the main NOC match OPPs.
> >
> > which one use the passive governor? And which one is the parent
> > devfreq device for devfreq device using passive governor?
> >
> > In my case, it is difficult to catch the relationship
> > between devices. I'd like you to explain the detailed relationship
> > on binding document for user.
> >
> >> But at the bus level DDRC only has AXI and APB slave ports.
> >
> > 'DDRC' indicates the 'drivers/devfreq/imx8m-ddrc.c?
> >
> >>
> >> Buses on imx don't have a parent/child relationship; in fact there are
> >> even a few cycles.
> >
> > You mentioned that 'imx don't have a parent/child relationship',
> > Why do you use 'passive' governor? It is difficult to understand
> > the hierarchy of imx.
>
> The imx8m has a main NOC in the center between the CPU and the DDRC
> (dram controller) with many other peripheral buses around the NOC.

Actually, if this dt binding document contains the relationship
between ddrc(imx8m-ddrc.c)
, imx-bus.c(bus) and interconnect node, it is more easy to understand
the hierarchy
of bus/ddrc. In my case, it is difficult because the binding document doesn't
include the enough example. But, I'll expect them as you mentioned on
your reply.

>
> Here's /sys/kernel/debug/interconnect/interconnect_graph on imx8mm:
> https://gist.github.com/cdleonard/84d103dafc9131fcb8ca9a494822a131#file-imx8mm-svg

It is the interconnect node graph. I hope to know the relationship
when bus(imx-bus.c)
uses the 'passive governor' and which is the

>
> A lot of stuff is omitted, it mostly just includes high-performance bus
> masters.
>
> DDRC has a performance monitor attached which can measure current
> bandwith and feed it to an ondemand governor. I want to use passive
> governor on the NOC so that it matches frequencies with DDRC and scales

I have the following questions. If you explain the more detailed descritpion
and add multiple dt bidning example, I'll expect that I can understand
the following my questions.
- Which one will use the 'passive governor'?
- DDRC is parent devfreq device for imx bus(imx-bus.c) using passive governor?
- Why imx bus(imx-bus.c) use the userspace governor?
- Which the relationship between imx bus(imx-bus.c) using userspace
governor and imx bus(imx-bus.c) using passive governor?

> proportionally, otherwise if NOC is at low frequency then dynamically
> scaling up the DDRC might be ineffective.
>
> Perhaps you could explain how parent/child relationships work on exynos?

Right. So, I wrote[1] why exynos-bus.c have to use the passive governor
and how to make the hierarchy/relationship between exynos-bus(parent
devfreq device
using devfreq governor except for passive governor)
and exynos-bus (child devfreq device using only passive governor).

[1] Documentation/devicetree/bindings/devfreq/exynos-bus.txt

>
> >>> [1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node
> >>>
> >>>>
> >>>>> +      passive governor.
> >>>>> +
> >>>>> +  '#interconnect-cells':
> >>>>> +    description:
> >>>>> +      If specified then also act as an interconnect provider. Should only be
> >>>>> +      set once per soc on main noc.
> >>>>> +    const: 1
> >>>>> +
> >>>>> +  fsl,scalable-node-ids:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >>>>> +    description:
> >>>>> +      Array of node ids for scalable nodes. Uses same numeric identifier
> >>>>> +      namespace as the consumer "interconnects" binding.
> >>>>> +
> >>>>> +  fsl,scalable-nodes:
> >>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> >>>>> +    description:
> >>>>> +      Array of phandles to scalable nodes. Must be of same length as
> >>>>> +      fsl,scalable-node-ids.
> >>>>> +
> >>>>> +required:
> >>>>> +  - compatible
> >>>>> +  - clocks
> >>>>> +
> >>>>> +additionalProperties: false
> >>>>> +
> >>>>> +examples:
> >>>>> +  - |
> >>>>
> >>>> Is it enough example to understand the relation between
> >>>> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
> >>>>
> >>>> In my case, if possible, hope to show the more detailed
> >>>> example. This example seems that don't contain the ddrc
> >>>> dt node (imx8m-ddrc.c).
> >>
> >> OK, I'll elaborate explanation on noc binding.
> >
> > Thanks. If possible, you better to add almost example cases.
> >
> >>
> >>>>
> >>>>> +    #include <dt-bindings/clock/imx8mq-clock.h>
> >>>>> +    #include <dt-bindings/interconnect/imx8mq.h>
> >>>>> +    noc: interconnect@32700000 {
> >>>>> +            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
> >>>>> +            reg = <0x32700000 0x100000>;
> >>>>> +            clocks = <&clk IMX8MQ_CLK_NOC>;
> >>>>> +            #interconnect-cells = <1>;
> >>>>> +            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
> >>>>> +                                    <IMX8MQ_ICS_DRAM>;
> >>>>> +            fsl,scalable-nodes = <&noc>,
> >>>>> +                                 <&ddrc>;
> >>>>> +            operating-points-v2 = <&noc_opp_table>;
> >>>>> +
> >>>>> +            noc_opp_table: opp-table {
> >>>>> +                    compatible = "operating-points-v2";
> >>>>> +
> >>>>> +                    opp-133M {
> >>>>> +                            opp-hz = /bits/ 64 <133333333>;
> >>>>> +                    };
> >>>>> +                    opp-800M {
> >>>>> +                            opp-hz = /bits/ 64 <800000000>;
> >>>>> +                    };
> >>>>> +            };
> >>>>> +    };
> >>
> >>
> >
> >
>
>
Leonard Crestez Dec. 19, 2019, 7:11 p.m. UTC | #7
On 19.12.2019 17:56, Chanwoo Choi wrote:
> 2019년 12월 19일 (목) 오후 11:33, Leonard Crestez <leonard.crestez@nxp.com>님이 작성:
>>
>> On 17.12.2019 02:08, Chanwoo Choi wrote:
>>> On 12/17/19 12:09 AM, Leonard Crestez wrote:
>>>> On 16.12.2019 05:18, Chanwoo Choi wrote:
>>>>> Hi,
>>>>>
>>>>> On 12/16/19 10:12 AM, Chanwoo Choi wrote:
>>>>>> On 11/15/19 5:09 AM, Leonard Crestez wrote:
>>>>>>> Add initial dt bindings for the interconnects inside i.MX chips.
>>>>>>> Multiple external IPs are involved but SOC integration means the
>>>>>>> software controllable interfaces are very similar.
>>>>>>>
>>>>>>> Main NOC node acts as interconnect provider if #interconnect-cells is
>>>>>>> present.
>>>>>>>
>>>>>>> Multiple interconnects can be present, each with their own OPP table.
>>>>>>>
>>>>>>> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
>>>>>>> ---
>>>>>>>     .../bindings/interconnect/fsl,imx8m-noc.yaml  | 104 ++++++++++++++++++
>>>>>>>     1 file changed, 104 insertions(+)
>>>>>>>     create mode 100644 Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000000..5cd94185fec3
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
>>>>>>> @@ -0,0 +1,104 @@
>>>>>>> +# SPDX-License-Identifier: GPL-2.0
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3D8570eb5a-d8a45732-85716015-0cc47a3356b2-92a5b92cc514d07e%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D0c13f3e0-51df3f45-0c1278af-0cc47a30d446-77e809543b673ffd%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fschemas%252Finterconnect%252Ffsl%252Cimx8m-noc.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DH2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%252BK%252BGFE%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C40406bfd0f134eeddb3208d7849c0968%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637123678022512623&amp;sdata=IYSYlNtA0fYKLkTr4OjyHkKa6FAi2m8UFr4nXZ9pr7s%3D&amp;reserved=0
>>>>>>> +$schema: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Furl%3Fk%3Df7cec483-aa1a78eb-f7cf4fcc-0cc47a3356b2-4154a3c43886f5ed%26u%3Dhttps%3A%2F%2Feur01.safelinks.protection.outlook.com%2F%3Furl%3Dhttps%253A%252F%252Fprotect2.fireeye.com%252Furl%253Fk%253D87c672dc-da0abe79-87c7f993-0cc47a30d446-414d3b4d0127419a%2526u%253Dhttp%253A%252F%252Fdevicetree.org%252Fmeta-schemas%252Fcore.yaml%2523%26amp%3Bdata%3D02%257C01%257Cleonard.crestez%2540nxp.com%257C2d1f1868afa140702a6b08d781d6ab68%257C686ea1d3bc2b4c6fa92cd99c5c301635%257C0%257C0%257C637120631307418544%26amp%3Bsdata%3DT6PgQ1DWI4OLOx3gifRRt%252FNImdVrgDUoswZ%252FNKw3oR8%253D%26amp%3Breserved%3D0&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C40406bfd0f134eeddb3208d7849c0968%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637123678022512623&amp;sdata=nrbPGpgBci5DDm6qRROeFa78hARR9UCXeP59TnPtCuI%3D&amp;reserved=0
>>>>>>> +
>>>>>>> +title: Generic i.MX bus frequency device
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Leonard Crestez <leonard.crestez@nxp.com>
>>>>>>> +
>>>>>>> +description: |
>>>>>>> +  The i.MX SoC family has multiple buses for which clock frequency (and
>>>>>>> +  sometimes voltage) can be adjusted.
>>>>>>> +
>>>>>>> +  Some of those buses expose register areas mentioned in the memory maps as GPV
>>>>>>> +  ("Global Programmers View") but not all. Access to this area might be denied
>>>>>>> +  for normal (non-secure) world.
>>>>>>> +
>>>>>>> +  The buses are based on externally licensed IPs such as ARM NIC-301 and
>>>>>>> +  Arteris FlexNOC but DT bindings are specific to the integration of these bus
>>>>>>> +  interconnect IPs into imx SOCs.
>>>>>>> +
>>>>>>> +properties:
>>>>>>> +  compatible:
>>>>>>> +    oneOf:
>>>>>>> +      - items:
>>>>>>> +        - enum:
>>>>>>> +          - fsl,imx8mn-nic
>>>>>>> +          - fsl,imx8mm-nic
>>>>>>> +          - fsl,imx8mq-nic
>>>>>>> +        - const: fsl,imx8m-nic
>>>>>>> +      - items:
>>>>>>> +        - enum:
>>>>>>> +          - fsl,imx8mn-noc
>>>>>>> +          - fsl,imx8mm-noc
>>>>>>> +          - fsl,imx8mq-noc
>>>>>>> +        - const: fsl,imx8m-noc
>>>>>>> +
>>>>>>> +  reg:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +  clocks:
>>>>>>> +    maxItems: 1
>>>>>>> +
>>>>>>> +  operating-points-v2: true
>>>>>>> +  opp-table: true
>>>>>>> +
>>>>>>> +  devfreq:
>>>>>>> +    $ref: "/schemas/types.yaml#/definitions/phandle"
>>>>>>> +    description:
>>>>>>> +      Phandle to another devfreq device to match OPPs with by using the
>>>>>>
>>>>>> Better to use 'parent' instead of 'another' word for improving the understanding.
>>>>>
>>>>> I think that 'devfreq' is not proper way to get the parent node
>>>>> in devicetree. Because 'devfreq' name is linuxium. The property name
>>>>> didn't indicate the any h/w device. So, I'll make 'devfreq' property deprecated.
>>>>>
>>>>> So, you better to make the specific property for this device driver
>>>>> like as following: and use devfreq_get_devfreq_by_node() function
>>>>> which is developed by you in order to get the devfreq instance node.
>>>>>
>>>>>      fsl,parent-device = <&parent devfreq device>;
>>>>
>>>> This is only a "parent" in the sense that it's assigned to
>>>> devfreq_passive.data.parent. The "devfreq" name is indeed too generic.
>>>
>>> I thought that 'devfreq' property name is generic.
>>> But, it's not proper for DT binding because DT file show
>>> the h/w and the relation of h/w. 'devfreq' property name
>>> has not meant h/w.
>>>
>>> So that devfreq core doesn't force to use the specific property
>>> name to get the devfreq parent instance on DT. Just, devfreq core
>>> will provide devfreq_get_devfreq_by_node() function.
>>>
>>> I developed it on devfre-testing branch[2]. Before I'm sending
>>> the these patches, you can check them.
>>>
>>> [1] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Fcommit%2F%3Fh%3Ddevfreq-testing%26id%3Df3678b4e6b75dccfe8bb87d005da2d68c70fdeab&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C40406bfd0f134eeddb3208d7849c0968%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637123678022512623&amp;sdata=AhMMx8MZB7HIvYaChWFdSu9JMFGPHyz%2B8W%2Fk7pevofY%3D&amp;reserved=0
>>> [2] https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fchanwoo%2Flinux.git%2Flog%2F%3Fh%3Ddevfreq-testing&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C40406bfd0f134eeddb3208d7849c0968%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637123678022522615&amp;sdata=29FP1He7PT%2B2NdYeyBIjAk2reqCeOmM5aJgnXotFQzI%3D&amp;reserved=0
>>>
>>>>
>>>> The DDRC can measure bandwith usage and I want to use the passive
>>>> governor to make the main NOC match OPPs.
>>>
>>> which one use the passive governor? And which one is the parent
>>> devfreq device for devfreq device using passive governor?
>>>
>>> In my case, it is difficult to catch the relationship
>>> between devices. I'd like you to explain the detailed relationship
>>> on binding document for user.
>>>
>>>> But at the bus level DDRC only has AXI and APB slave ports.
>>>
>>> 'DDRC' indicates the 'drivers/devfreq/imx8m-ddrc.c?
>>>
>>>>
>>>> Buses on imx don't have a parent/child relationship; in fact there are
>>>> even a few cycles.
>>>
>>> You mentioned that 'imx don't have a parent/child relationship',
>>> Why do you use 'passive' governor? It is difficult to understand
>>> the hierarchy of imx.
>>
>> The imx8m has a main NOC in the center between the CPU and the DDRC
>> (dram controller) with many other peripheral buses around the NOC.
> 
> Actually, if this dt binding document contains the relationship
> between ddrc(imx8m-ddrc.c)
> , imx-bus.c(bus) and interconnect node, it is more easy to understand
> the hierarchy
> of bus/ddrc. In my case, it is difficult because the binding document doesn't
> include the enough example. But, I'll expect them as you mentioned on
> your reply.
> 
>>
>> Here's /sys/kernel/debug/interconnect/interconnect_graph on imx8mm:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgist.github.com%2Fcdleonard%2F84d103dafc9131fcb8ca9a494822a131%23file-imx8mm-svg&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C40406bfd0f134eeddb3208d7849c0968%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637123678022522615&amp;sdata=anAPK7CpjdRtE7S%2FRPo0TY08OgC2EWmuHk58nBfZ1og%3D&amp;reserved=0
> 
> It is the interconnect node graph. I hope to know the relationship
> when bus(imx-bus.c)
> uses the 'passive governor' and which is the
> 
>>
>> A lot of stuff is omitted, it mostly just includes high-performance bus
>> masters.
>>
>> DDRC has a performance monitor attached which can measure current
>> bandwith and feed it to an ondemand governor. I want to use passive
>> governor on the NOC so that it matches frequencies with DDRC and scales
> 
> I have the following questions. If you explain the more detailed descritpion
> and add multiple dt bidning example, I'll expect that I can understand
> the following my questions.
> - Which one will use the 'passive governor'?
> - DDRC is parent devfreq device for imx bus(imx-bus.c) using passive governor?
> - Why imx bus(imx-bus.c) use the userspace governor?
> - Which the relationship between imx bus(imx-bus.c) using userspace
> governor and imx bus(imx-bus.c) using passive governor?

NOC would use the passive governor with devfreq_passive_data.parent sent 
to DDRC so that it matches rates when DDRC uses ondemand governor. This 
is a minor feature meant to make ondemand scaling behave more usefully, 
otherwise NOC might be a bottleneck for when CPU uses lots of ram 
bandwidth. It would make sense to split it into a separate patch.

The same imx-bus driver can also be used for peripheral buses (for 
example PL301_MIPI using imx8m-nic compatible) and in that case it will 
just use the userspace governor by default.

Useful scaling for the mipi bus would be performed via interconenct: the 
display controller (LCDIF) could request bandwidth along an icc_path 
that goes: LCDIF->PL301_MIPI->NOC->DDRC and then the interconnect 
provider would aggregate requests and translate them into separate 
minimum frequencies for PL301_MIPI, NOC and DDRC.

Camera devices would share this path while GPU and VPUs would only share 
NOC and DDRC. This detail is specific to 8mm, interconnect graph is 
different on 8mq and 8mn.

The "interconnect" driver is very loosely coupled to "devfreq" drivers: 
it just makes QOS_MIN_FREQUENCY requests. There is already a distinct 
driver for imx8m-ddrc and in theory fsl,imx8m-noc and fsl,imx8m-nic 
could have distinct drivers as well. It would be pointless because all 
they currently do is set a single clock rate so they're all handled by a 
single imx-bus driver, similar to exynos-bus.

The interconnect driver shares the imx8m-noc binding with imx-bus 
because "virtual nodes" are strongly discouraged as they don't "describe 
hardware". Earlier versions of the series had a virtual node.

>> proportionally, otherwise if NOC is at low frequency then dynamically
>> scaling up the DDRC might be ineffective.
>>
>> Perhaps you could explain how parent/child relationships work on exynos?
> 
> Right. So, I wrote[1] why exynos-bus.c have to use the passive governor
> and how to make the hierarchy/relationship between exynos-bus(parent
> devfreq device
> using devfreq governor except for passive governor)
> and exynos-bus (child devfreq device using only passive governor).
> 
> [1] Documentation/devicetree/bindings/devfreq/exynos-bus.txt
> 
>>
>>>>> [1] [PATCH RFC v5 04/10] PM / devfreq: Add devfreq_get_devfreq_by_node
>>>>>
>>>>>>
>>>>>>> +      passive governor.
>>>>>>> +
>>>>>>> +  '#interconnect-cells':
>>>>>>> +    description:
>>>>>>> +      If specified then also act as an interconnect provider. Should only be
>>>>>>> +      set once per soc on main noc.
>>>>>>> +    const: 1
>>>>>>> +
>>>>>>> +  fsl,scalable-node-ids:
>>>>>>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>>>>>>> +    description:
>>>>>>> +      Array of node ids for scalable nodes. Uses same numeric identifier
>>>>>>> +      namespace as the consumer "interconnects" binding.
>>>>>>> +
>>>>>>> +  fsl,scalable-nodes:
>>>>>>> +    $ref: /schemas/types.yaml#/definitions/phandle-array
>>>>>>> +    description:
>>>>>>> +      Array of phandles to scalable nodes. Must be of same length as
>>>>>>> +      fsl,scalable-node-ids.
>>>>>>> +
>>>>>>> +required:
>>>>>>> +  - compatible
>>>>>>> +  - clocks
>>>>>>> +
>>>>>>> +additionalProperties: false
>>>>>>> +
>>>>>>> +examples:
>>>>>>> +  - |
>>>>>>
>>>>>> Is it enough example to understand the relation between
>>>>>> imx8m-ddrc.c, imx-devfreq.c and imx interconnect driver?
>>>>>>
>>>>>> In my case, if possible, hope to show the more detailed
>>>>>> example. This example seems that don't contain the ddrc
>>>>>> dt node (imx8m-ddrc.c).
>>>>
>>>> OK, I'll elaborate explanation on noc binding.
>>>
>>> Thanks. If possible, you better to add almost example cases.

Here's a more elaborate example:

#include <dt-bindings/clock/imx8mm-clock.h>
#include <dt-bindings/interconnect/imx8mm.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>

noc: interconnect@32700000 {
         compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
         reg = <0x32700000 0x100000>;
         clocks = <&clk IMX8MM_CLK_NOC>;
         #interconnect-cells = <1>;

         /* Used to make requests for DEV_PM_QOS_MIN_FREQUENCY: */
         fsl,scalable-node-ids = <IMX8MM_ICN_NOC>,
                                 <IMX8MM_ICN_MIPI>,
                                 <IMX8MM_ICS_DRAM>;
         fsl,scalable-nodes = <&noc>,
                              <&pl301_mipi>,
                              <&ddrc>;

         /* For passive governor: */
         devfreq = <&ddrc>;

         operating-points-v2 = <&noc_opp_table>;
         noc_opp_table: opp-table {
                 compatible = "operating-points-v2";

                 opp-133M {
                         opp-hz = /bits/ 64 <133333333>;
                 };
                 opp-800M {
                         opp-hz = /bits/ 64 <800000000>;
                 };
         };
};

pl301_mipi: interconnect@32500000 {
         compatible = "fsl,imx8m-nic";
         reg = <0x32500000 0x100000>;
         clocks = <&clk IMX8MM_CLK_DISP_AXI>;
         operating-points-v2 = <&pl301_mipi_opp_table>;

         pl301_mipi_opp_table: opp-table {
                 compatible = "operating-points-v2";

                 opp-200M {
                         opp-hz = /bits/ 64 <200000000>;
                 };
                 opp-500M {
                         opp-hz = /bits/ 64 <500000000>;
                 };
         };
};

ddrc: memory-controller@3d400000 {
         compatible = "fsl,imx8mm-ddrc", "fsl,imx8m-ddrc";
         reg = <0x3d400000 0x400000>;
         glock-names = "core", "pll", "alt", "apb";
         clocks = <&clk IMX8MM_CLK_DRAM_CORE>,
                  <&clk IMX8MM_DRAM_PLL>,
                  <&clk IMX8MM_CLK_DRAM_ALT>,
                  <&clk IMX8MM_CLK_DRAM_APB>;
         /* For ondemand governor: */
         devfreq-events = <&ddr_pmu>;
};

ddr_pmu: ddr-pmu@3d800000 {
         compatible = "fsl,imx8mm-ddr-pmu", "fsl,imx8m-ddr-pmu";
         reg = <0x3d800000 0x400000>;
         interrupt-parent = <&gic>;
         interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
};
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
new file mode 100644
index 000000000000..5cd94185fec3
--- /dev/null
+++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
@@ -0,0 +1,104 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interconnect/fsl,imx8m-noc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Generic i.MX bus frequency device
+
+maintainers:
+  - Leonard Crestez <leonard.crestez@nxp.com>
+
+description: |
+  The i.MX SoC family has multiple buses for which clock frequency (and
+  sometimes voltage) can be adjusted.
+
+  Some of those buses expose register areas mentioned in the memory maps as GPV
+  ("Global Programmers View") but not all. Access to this area might be denied
+  for normal (non-secure) world.
+
+  The buses are based on externally licensed IPs such as ARM NIC-301 and
+  Arteris FlexNOC but DT bindings are specific to the integration of these bus
+  interconnect IPs into imx SOCs.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - fsl,imx8mn-nic
+          - fsl,imx8mm-nic
+          - fsl,imx8mq-nic
+        - const: fsl,imx8m-nic
+      - items:
+        - enum:
+          - fsl,imx8mn-noc
+          - fsl,imx8mm-noc
+          - fsl,imx8mq-noc
+        - const: fsl,imx8m-noc
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  operating-points-v2: true
+  opp-table: true
+
+  devfreq:
+    $ref: "/schemas/types.yaml#/definitions/phandle"
+    description:
+      Phandle to another devfreq device to match OPPs with by using the
+      passive governor.
+
+  '#interconnect-cells':
+    description:
+      If specified then also act as an interconnect provider. Should only be
+      set once per soc on main noc.
+    const: 1
+
+  fsl,scalable-node-ids:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      Array of node ids for scalable nodes. Uses same numeric identifier
+      namespace as the consumer "interconnects" binding.
+
+  fsl,scalable-nodes:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    description:
+      Array of phandles to scalable nodes. Must be of same length as
+      fsl,scalable-node-ids.
+
+required:
+  - compatible
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx8mq-clock.h>
+    #include <dt-bindings/interconnect/imx8mq.h>
+    noc: interconnect@32700000 {
+            compatible = "fsl,imx8mq-noc", "fsl,imx8m-noc";
+            reg = <0x32700000 0x100000>;
+            clocks = <&clk IMX8MQ_CLK_NOC>;
+            #interconnect-cells = <1>;
+            fsl,scalable-node-ids = <IMX8MQ_ICN_NOC>,
+                                    <IMX8MQ_ICS_DRAM>;
+            fsl,scalable-nodes = <&noc>,
+                                 <&ddrc>;
+            operating-points-v2 = <&noc_opp_table>;
+
+            noc_opp_table: opp-table {
+                    compatible = "operating-points-v2";
+
+                    opp-133M {
+                            opp-hz = /bits/ 64 <133333333>;
+                    };
+                    opp-800M {
+                            opp-hz = /bits/ 64 <800000000>;
+                    };
+            };
+    };