[v2,2/4] dt-bindings: memory: Add binding for NVIDIA Tegra30 External Memory Controller
diff mbox series

Message ID 20190414202009.31268-3-digetx@gmail.com
State Changes Requested
Headers show
Series
  • memory: tegra: Introduce Tegra30 EMC driver
Related show

Checks

Context Check Description
robh/checkpatch success

Commit Message

Dmitry Osipenko April 14, 2019, 8:20 p.m. UTC
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

Comments

Rob Herring April 29, 2019, 10:05 p.m. UTC | #1
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
Dmitry Osipenko May 2, 2019, 12:06 a.m. UTC | #2
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.
Rob Herring May 2, 2019, 12:17 a.m. UTC | #3
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
Dmitry Osipenko May 2, 2019, 12:52 a.m. UTC | #4
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.
Dmitry Osipenko May 2, 2019, 1:39 a.m. UTC | #5
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.
Dmitry Osipenko May 16, 2019, 2:55 p.m. UTC | #6
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.

Patch
diff mbox series

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 */
+				>;
+			};
+		};
+	};
+};