On 11/13/19 9:35 PM, Leonard Crestez wrote: > On 13.11.2019 04:32, Chanwoo Choi wrote: >> On 11/13/19 6:50 AM, Leonard Crestez wrote: >>> Add devicetree bindings for the i.MX DDR Controller on imx8m series >>> chips. It supports dynamic frequency switching between multiple data >>> rates and this is exposed to Linux via the devfreq subsystem. >>> >>> Signed-off-by: Leonard Crestez <leonard.crestez@xxxxxxx> >>> --- >>> .../memory-controllers/fsl/imx8m-ddrc.yaml | 57 +++++++++++++++++++ >>> 1 file changed, 57 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/memory-controllers/fsl/imx8m-ddrc.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/imx8m-ddrc.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/imx8m-ddrc.yaml >>> new file mode 100644 >>> index 000000000000..7c98e3509f75 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/imx8m-ddrc.yaml >>> @@ -0,0 +1,57 @@ >>> +# SPDX-License-Identifier: GPL-2.0 >>> +%YAML 1.2 >>> +--- >>> +$id: https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fmemory-controllers%2Ffsl%2Fimx8m-ddrc.yaml%23&data=02%7C01%7Cleonard.crestez%40nxp.com%7C23e819d42b664965975808d767e1c084%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637092091602846215&sdata=frWd1MENZm%2FsPjQp%2FWbphMgkkCMtwsgV8hLQyIhC3%2BI%3D&reserved=0 >>> +$schema: https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=02%7C01%7Cleonard.crestez%40nxp.com%7C23e819d42b664965975808d767e1c084%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637092091602846215&sdata=4IweKQJO9ZsB%2B9QxixSQjfYOFm3%2FY7iMHFBSsquK1B0%3D&reserved=0 >>> + >>> +title: i.MX8M DDR Controller >>> + >>> +maintainers: >>> + - Leonard Crestez <leonard.crestez@xxxxxxx> >>> + >>> +properties: >>> + compatible: >>> + items: >>> + - enum: >>> + - fsl,imx8mn-ddrc >>> + - fsl,imx8mm-ddrc >>> + - fsl,imx8mq-ddrc >>> + - const: fsl,imx8m-ddrc >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + clocks: >>> + maxItems: 4 >>> + >>> + clock-names: >>> + items: >>> + - const: core >>> + - const: pll >>> + - const: alt >>> + - const: apb >>> + >>> + operating-points-v2: true >>> + opp-table: true >>> + >>> +required: >>> + - reg >>> + - compatible >>> + - clocks >>> + - clock-names >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/clock/imx8mm-clock.h> >>> + ddrc: memory-controller@3d400000 { >>> + compatible = "fsl,imx8mm-ddrc", "fsl,imx8m-ddrc"; >>> + reg = <0x3d400000 0x400000>; >> >> The probe() function doesn't get the IORESOURCE_MEM from dt? >> Is it needed? > > This area is not currently mapped by the driver. As far as I understand > it's acceptable to "describe hardware" even if you don't use the full > description in driver code. > > If I were to remove the "reg" area wouldn't I also have to move the node > outside of the bus to keep DT validation? It's better to keep the address. The dt bidning documentation and device driver have not any explanation about this. It makes the confusion to user who don't know the history. I'd like you to add the explanation on dt bidning documentation in order to remove the confusion. Actually, you get the confirm from DT maintainer. I'm OK. > > Maybe it will be mapped in the future or maybe firmware will start to > parse linux DT instead of hardcoding SOC-specific addresses (this > already happens in some cases). OK. Do you implement them in the future? If you have a plan, you better to do this on this time. > >>> + clock-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>; >>> + operating-points-v2 = <&ddrc_opp_table>; >>> + }; > > -- Best Regards, Chanwoo Choi Samsung Electronics
