| Message ID | 20190414202009.31268-3-digetx@gmail.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | memory: tegra: Introduce Tegra30 EMC driver | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: > Add device-tree binding for NVIDIA Tegra30 External Memory Controller. > The binding is based on the Tegra124 EMC binding since hardware is > similar, although there are couple significant differences. My comments on Tegra124 binding apply here. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > .../memory-controllers/nvidia,tegra30-emc.txt | 257 ++++++++++++++++++ > 1 file changed, 257 insertions(+) > create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt
30.04.2019 1:05, Rob Herring пишет: > On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: >> Add device-tree binding for NVIDIA Tegra30 External Memory Controller. >> The binding is based on the Tegra124 EMC binding since hardware is >> similar, although there are couple significant differences. > > My comments on Tegra124 binding apply here. The common timing definition doesn't fully match the definition that is used by Tegra's Memory Controller, thus the DQS (data strobe) timing parameter is comprised of multiple sub-parameters that describe how to generate the strobe in hardware. There are also more additional parameters that are specific to Tegra and they are individually characterized for each memory model and clock rate. Hence the common timing definition isn't usable.
On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote: > > 30.04.2019 1:05, Rob Herring пишет: > > On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: > >> Add device-tree binding for NVIDIA Tegra30 External Memory Controller. > >> The binding is based on the Tegra124 EMC binding since hardware is > >> similar, although there are couple significant differences. > > > > My comments on Tegra124 binding apply here. > > The common timing definition doesn't fully match the definition that is > used by Tegra's Memory Controller, thus the DQS (data strobe) timing > parameter is comprised of multiple sub-parameters that describe how to > generate the strobe in hardware. There are also more additional > parameters that are specific to Tegra and they are individually > characterized for each memory model and clock rate. Hence the common > timing definition isn't usable. I don't understand. Every PC in the world can work with any DIMM (within a given generation) just with SPD data. Why is that not sufficient here? In any case, it seems for Tegra124 a different approach is going to be taken. Seems like an "avoid DT" solution to me, but if it's contained within the firmware it's not my problem. Rob
02.05.2019 3:17, Rob Herring пишет: > On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote: >> >> 30.04.2019 1:05, Rob Herring пишет: >>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: >>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller. >>>> The binding is based on the Tegra124 EMC binding since hardware is >>>> similar, although there are couple significant differences. >>> >>> My comments on Tegra124 binding apply here. >> >> The common timing definition doesn't fully match the definition that is >> used by Tegra's Memory Controller, thus the DQS (data strobe) timing >> parameter is comprised of multiple sub-parameters that describe how to >> generate the strobe in hardware. There are also more additional >> parameters that are specific to Tegra and they are individually >> characterized for each memory model and clock rate. Hence the common >> timing definition isn't usable. > > I don't understand. Every PC in the world can work with any DIMM > (within a given generation) just with SPD data. Why is that not > sufficient here? Because this is not a standard PC, but a custom embedded hardware that is simpler and also doesn't fully follow the standards in some cases. > In any case, it seems for Tegra124 a different approach is going to be > taken. Seems like an "avoid DT" solution to me, but if it's contained > within the firmware it's not my problem. My above comment really applies to all Terga's. The Tegra210 is also a bit more complicated case because of the proprietary signed firmware that can't be easily replaced with opensource alternative without special hacks, but AFAIK the unofficial opensource firmware will be available in some form for at least one consumer device (Nintendo Switch). Please write a detailed comment to the Tegra210's patch, saying what you would want to see changed. I'm sure Joseph will try to do his best. Note that it is always possible to define a proper device tree binding and then also "unofficially" support the downstream binding, IIRC that's what some drivers are already doing in upstream kernel. So I think you could just demand for the proper binding regardless of the firmware situation.
02.05.2019 3:52, Dmitry Osipenko пишет: > 02.05.2019 3:17, Rob Herring пишет: >> On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote: >>> >>> 30.04.2019 1:05, Rob Herring пишет: >>>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: >>>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller. >>>>> The binding is based on the Tegra124 EMC binding since hardware is >>>>> similar, although there are couple significant differences. >>>> >>>> My comments on Tegra124 binding apply here. >>> >>> The common timing definition doesn't fully match the definition that is >>> used by Tegra's Memory Controller, thus the DQS (data strobe) timing >>> parameter is comprised of multiple sub-parameters that describe how to >>> generate the strobe in hardware. There are also more additional >>> parameters that are specific to Tegra and they are individually >>> characterized for each memory model and clock rate. Hence the common >>> timing definition isn't usable. >> >> I don't understand. Every PC in the world can work with any DIMM >> (within a given generation) just with SPD data. Why is that not >> sufficient here? > > Because this is not a standard PC, but a custom embedded hardware that > is simpler and also doesn't fully follow the standards in some cases. Even if there is a way to derive at least some of required parameters from the common timing, I don't have information about how to do it and there is pretty much no chance to get it into public. Rob, please let me know if you're okay with this binding.
02.05.2019 4:39, Dmitry Osipenko пишет: > 02.05.2019 3:52, Dmitry Osipenko пишет: >> 02.05.2019 3:17, Rob Herring пишет: >>> On Wed, May 1, 2019 at 7:06 PM Dmitry Osipenko <digetx@gmail.com> wrote: >>>> >>>> 30.04.2019 1:05, Rob Herring пишет: >>>>> On Sun, Apr 14, 2019 at 11:20:07PM +0300, Dmitry Osipenko wrote: >>>>>> Add device-tree binding for NVIDIA Tegra30 External Memory Controller. >>>>>> The binding is based on the Tegra124 EMC binding since hardware is >>>>>> similar, although there are couple significant differences. >>>>> >>>>> My comments on Tegra124 binding apply here. >>>> >>>> The common timing definition doesn't fully match the definition that is >>>> used by Tegra's Memory Controller, thus the DQS (data strobe) timing >>>> parameter is comprised of multiple sub-parameters that describe how to >>>> generate the strobe in hardware. There are also more additional >>>> parameters that are specific to Tegra and they are individually >>>> characterized for each memory model and clock rate. Hence the common >>>> timing definition isn't usable. >>> >>> I don't understand. Every PC in the world can work with any DIMM >>> (within a given generation) just with SPD data. Why is that not >>> sufficient here? >> >> Because this is not a standard PC, but a custom embedded hardware that >> is simpler and also doesn't fully follow the standards in some cases. > > Even if there is a way to derive at least some of required parameters > from the common timing, I don't have information about how to do it and > there is pretty much no chance to get it into public. > > Rob, please let me know if you're okay with this binding. > I looked through the Technical Reference Manual a bit more closely and it actually has comments about how to convert some of the common timing parameters into register values. Some of those parameters are not trivial to convert and the doc could be incomplete, and some of the parameters are not documented at all. The other much more minor reason against the common timing description is that all boards come with the memory timing definition in a platform-specific form in downstream kernel. We're re-using that definition in the device-tree, hence it is easy to port the timings from downstream into upstream.
diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt new file mode 100644 index 000000000000..dffe396c5d79 --- /dev/null +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt @@ -0,0 +1,257 @@ +NVIDIA Tegra30 SoC EMC (external memory controller) +==================================================== + +Required properties : +- compatible : Should be "nvidia,tegra30-emc". +- reg : physical base address and length of the controller's registers. +- #address-cells : Should be 1 +- #size-cells : Should be 0 +- interrupts : Should contain EMC General interrupt. +- clocks : Should contain EMC clock. +- nvidia,memory-controller : phandle of the MC driver. + +The node should contain a "emc-timings" subnode for each supported RAM type +(see field RAM_CODE in register PMC_STRAPPING_OPT_A), with its unit address +being its RAM_CODE. + +Required properties for "emc-timings" nodes : +- nvidia,ram-code : Should contain the value of RAM_CODE this timing set is +used for. + +Each "emc-timings" node should contain a "timing" subnode for every supported +EMC clock rate. The "timing" subnodes should have the clock rate in Hz as +their unit address. + +Required properties for "timing" nodes : +- clock-frequency : Should contain the memory clock rate in Hz. +- The following properties contain EMC timing characterization values +(specified in the board documentation) : + - nvidia,emc-auto-cal-interval : EMC_AUTO_CAL_INTERVAL + - nvidia,emc-mode-1 : Mode Register 1 + - nvidia,emc-mode-2 : Mode Register 2 + - nvidia,emc-mode-reset : Mode Register 0 + - nvidia,emc-zcal-cnt-long : EMC_ZCAL_WAIT_CNT after clock change + - nvidia,emc-cfg-dyn-self-ref : dynamic self-refresh enabled + - nvidia,emc-cfg-periodic-qrst : FBIO "read" FIFO periodic resetting enabled +- nvidia,emc-configuration : EMC timing characterization data. These are the +registers (see section "18.13.2 EMC Registers" in the TRM) whose values need to +be specified, according to the board documentation: + + EMC_RC + EMC_RFC + EMC_RAS + EMC_RP + EMC_R2W + EMC_W2R + EMC_R2P + EMC_W2P + EMC_RD_RCD + EMC_WR_RCD + EMC_RRD + EMC_REXT + EMC_WEXT + EMC_WDV + EMC_QUSE + EMC_QRST + EMC_QSAFE + EMC_RDV + EMC_REFRESH + EMC_BURST_REFRESH_NUM + EMC_PRE_REFRESH_REQ_CNT + EMC_PDEX2WR + EMC_PDEX2RD + EMC_PCHG2PDEN + EMC_ACT2PDEN + EMC_AR2PDEN + EMC_RW2PDEN + EMC_TXSR + EMC_TXSRDLL + EMC_TCKE + EMC_TFAW + EMC_TRPAB + EMC_TCLKSTABLE + EMC_TCLKSTOP + EMC_TREFBW + EMC_QUSE_EXTRA + EMC_FBIO_CFG6 + EMC_ODT_WRITE + EMC_ODT_READ + EMC_FBIO_CFG5 + EMC_CFG_DIG_DLL + EMC_CFG_DIG_DLL_PERIOD + EMC_DLL_XFORM_DQS0 + EMC_DLL_XFORM_DQS1 + EMC_DLL_XFORM_DQS2 + EMC_DLL_XFORM_DQS3 + EMC_DLL_XFORM_DQS4 + EMC_DLL_XFORM_DQS5 + EMC_DLL_XFORM_DQS6 + EMC_DLL_XFORM_DQS7 + EMC_DLL_XFORM_QUSE0 + EMC_DLL_XFORM_QUSE1 + EMC_DLL_XFORM_QUSE2 + EMC_DLL_XFORM_QUSE3 + EMC_DLL_XFORM_QUSE4 + EMC_DLL_XFORM_QUSE5 + EMC_DLL_XFORM_QUSE6 + EMC_DLL_XFORM_QUSE7 + EMC_DLI_TRIM_TXDQS0 + EMC_DLI_TRIM_TXDQS1 + EMC_DLI_TRIM_TXDQS2 + EMC_DLI_TRIM_TXDQS3 + EMC_DLI_TRIM_TXDQS4 + EMC_DLI_TRIM_TXDQS5 + EMC_DLI_TRIM_TXDQS6 + EMC_DLI_TRIM_TXDQS7 + EMC_DLL_XFORM_DQ0 + EMC_DLL_XFORM_DQ1 + EMC_DLL_XFORM_DQ2 + EMC_DLL_XFORM_DQ3 + EMC_XM2CMDPADCTRL + EMC_XM2DQSPADCTRL2 + EMC_XM2DQPADCTRL2 + EMC_XM2CLKPADCTRL + EMC_XM2COMPPADCTRL + EMC_XM2VTTGENPADCTRL + EMC_XM2VTTGENPADCTRL2 + EMC_XM2QUSEPADCTRL + EMC_XM2DQSPADCTRL3 + EMC_CTT_TERM_CTRL + EMC_ZCAL_INTERVAL + EMC_ZCAL_WAIT_CNT + EMC_MRS_WAIT_CNT + EMC_AUTO_CAL_CONFIG + EMC_CTT + EMC_CTT_DURATION + EMC_DYN_SELF_REF_CONTROL + EMC_FBIO_SPARE + EMC_CFG_RSV + +Example SoC include file: + +/ { + emc@7000f400 { + compatible = "nvidia,tegra30-emc"; + reg = <0x7000f400 0x400>; + interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&tegra_car TEGRA30_CLK_EMC>; + #address-cells = <1>; + #size-cells = <0>; + + nvidia,memory-controller = <&mc>; + }; +}; + +Example board file: + +/ { + emc@7000f400 { + emc-timings-1 { + nvidia,ram-code = <1>; + + timing-667000000 { + clock-frequency = <667000000>; + + nvidia,emc-auto-cal-interval = <0x001fffff>; + nvidia,emc-mode-1 = <0x80100002>; + nvidia,emc-mode-2 = <0x80200018>; + nvidia,emc-mode-reset = <0x80000b71>; + nvidia,emc-zcal-cnt-long = <0x00000040>; + nvidia,emc-cfg-dyn-self-ref = <0x00000000>; + nvidia,emc-cfg-periodic-qrst = <0x00000001>; + + nvidia,emc-configuration = < + 0x00000020 /* EMC_RC */ + 0x0000006a /* EMC_RFC */ + 0x00000017 /* EMC_RAS */ + 0x00000007 /* EMC_RP */ + 0x00000005 /* EMC_R2W */ + 0x0000000c /* EMC_W2R */ + 0x00000003 /* EMC_R2P */ + 0x00000011 /* EMC_W2P */ + 0x00000007 /* EMC_RD_RCD */ + 0x00000007 /* EMC_WR_RCD */ + 0x00000002 /* EMC_RRD */ + 0x00000001 /* EMC_REXT */ + 0x00000000 /* EMC_WEXT */ + 0x00000007 /* EMC_WDV */ + 0x0000000a /* EMC_QUSE */ + 0x00000009 /* EMC_QRST */ + 0x0000000b /* EMC_QSAFE */ + 0x00000011 /* EMC_RDV */ + 0x00001412 /* EMC_REFRESH */ + 0x00000000 /* EMC_BURST_REFRESH_NUM */ + 0x00000504 /* EMC_PRE_REFRESH_REQ_CNT */ + 0x00000002 /* EMC_PDEX2WR */ + 0x0000000e /* EMC_PDEX2RD */ + 0x00000001 /* EMC_PCHG2PDEN */ + 0x00000000 /* EMC_ACT2PDEN */ + 0x0000000c /* EMC_AR2PDEN */ + 0x00000016 /* EMC_RW2PDEN */ + 0x00000072 /* EMC_TXSR */ + 0x00000200 /* EMC_TXSRDLL */ + 0x00000005 /* EMC_TCKE */ + 0x00000015 /* EMC_TFAW */ + 0x00000000 /* EMC_TRPAB */ + 0x00000006 /* EMC_TCLKSTABLE */ + 0x00000007 /* EMC_TCLKSTOP */ + 0x00001453 /* EMC_TREFBW */ + 0x0000000b /* EMC_QUSE_EXTRA */ + 0x00000006 /* EMC_FBIO_CFG6 */ + 0x00000000 /* EMC_ODT_WRITE */ + 0x00000000 /* EMC_ODT_READ */ + 0x00005088 /* EMC_FBIO_CFG5 */ + 0xf00b0191 /* EMC_CFG_DIG_DLL */ + 0x00008000 /* EMC_CFG_DIG_DLL_PERIOD */ + 0x00000008 /* EMC_DLL_XFORM_DQS0 */ + 0x00000008 /* EMC_DLL_XFORM_DQS1 */ + 0x00000008 /* EMC_DLL_XFORM_DQS2 */ + 0x00000008 /* EMC_DLL_XFORM_DQS3 */ + 0x0000000a /* EMC_DLL_XFORM_DQS4 */ + 0x0000000a /* EMC_DLL_XFORM_DQS5 */ + 0x0000000a /* EMC_DLL_XFORM_DQS6 */ + 0x0000000a /* EMC_DLL_XFORM_DQS7 */ + 0x00018000 /* EMC_DLL_XFORM_QUSE0 */ + 0x00018000 /* EMC_DLL_XFORM_QUSE1 */ + 0x00018000 /* EMC_DLL_XFORM_QUSE2 */ + 0x00018000 /* EMC_DLL_XFORM_QUSE3 */ + 0x00000000 /* EMC_DLL_XFORM_QUSE4 */ + 0x00000000 /* EMC_DLL_XFORM_QUSE5 */ + 0x00000000 /* EMC_DLL_XFORM_QUSE6 */ + 0x00000000 /* EMC_DLL_XFORM_QUSE7 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS0 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS1 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS2 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS3 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS4 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS5 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS6 */ + 0x00000000 /* EMC_DLI_TRIM_TXDQS7 */ + 0x0000000a /* EMC_DLL_XFORM_DQ0 */ + 0x0000000a /* EMC_DLL_XFORM_DQ1 */ + 0x0000000a /* EMC_DLL_XFORM_DQ2 */ + 0x0000000a /* EMC_DLL_XFORM_DQ3 */ + 0x000002a0 /* EMC_XM2CMDPADCTRL */ + 0x0800013d /* EMC_XM2DQSPADCTRL2 */ + 0x22220000 /* EMC_XM2DQPADCTRL2 */ + 0x77fff884 /* EMC_XM2CLKPADCTRL */ + 0x01f1f501 /* EMC_XM2COMPPADCTRL */ + 0x07077404 /* EMC_XM2VTTGENPADCTRL */ + 0x54000000 /* EMC_XM2VTTGENPADCTRL2 */ + 0x080001e8 /* EMC_XM2QUSEPADCTRL */ + 0x0c000021 /* EMC_XM2DQSPADCTRL3 */ + 0x00000802 /* EMC_CTT_TERM_CTRL */ + 0x00020000 /* EMC_ZCAL_INTERVAL */ + 0x00000100 /* EMC_ZCAL_WAIT_CNT */ + 0x0155000c /* EMC_MRS_WAIT_CNT */ + 0xa0f10000 /* EMC_AUTO_CAL_CONFIG */ + 0x00000000 /* EMC_CTT */ + 0x00000000 /* EMC_CTT_DURATION */ + 0x800028a5 /* EMC_DYN_SELF_REF_CONTROL */ + 0xe8000000 /* EMC_FBIO_SPARE */ + 0xff00ff49 /* EMC_CFG_RSV */ + >; + }; + }; + }; +};
Add device-tree binding for NVIDIA Tegra30 External Memory Controller. The binding is based on the Tegra124 EMC binding since hardware is similar, although there are couple significant differences. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- .../memory-controllers/nvidia,tegra30-emc.txt | 257 ++++++++++++++++++ 1 file changed, 257 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra30-emc.txt