2019년 12월 19일 (목) 오후 11:33, Leonard Crestez <leonard.crestez@xxxxxxx>님이 작성: > > 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@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%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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&sdata=HYMJJHWyiKRhf7GDjKoOwjDpcZuYqlFlmRrDZnIRx5w%3D&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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&sdata=fIbrUUOUtZ5nt%2FH1tm3dzaI1J%2FGn5Gc54ms93HnBnQg%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. > > > > 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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&sdata=R21Tv1QgofBvMYb2VaxFjKSerwQ3tl8kakcYRyALmgM%3D&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&data=02%7C01%7Cleonard.crestez%40nxp.com%7C769d8e354f3b4d00b84508d782854a17%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637121381290437871&sdata=uH0d9LvrbCHgZJdrPJNJ8c8w45J7x1QyM7t5I3j%2BpSw%3D&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>; > >>>>> + }; > >>>>> + }; > >>>>> + }; > >> > >> > > > > > > -- Best Regards, Chanwoo Choi
