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@xxxxxxx> >>> --- >>> .../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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&sdata=H2q5nQlKYyLIivkBYUTaRD1Nu3WcnphPJny3k%2BK%2BGFE%3D&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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C2d1f1868afa140702a6b08d781d6ab68%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637120631307418544&sdata=T6PgQ1DWI4OLOx3gifRRt%2FNImdVrgDUoswZ%2FNKw3oR8%3D&reserved=0 >>> + >>> +title: Generic i.MX bus frequency device >>> + >>> +maintainers: >>> + - Leonard Crestez <leonard.crestez@xxxxxxx> >>> + >>> +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>; >>> + }; >>> + }; >>> + };
