From: Rob Herring <robh@kernel.org>
To: Lin Huang <hl@rock-chips.com>
Cc: heiko@sntech.de, myungjoo.ham@samsung.com,
mark.yao@rock-chips.com, cw00.choi@samsung.com, airlied@linux.ie,
mturquette@baylibre.com, dbasehore@chromium.org,
sboyd@codeaurora.org, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org, dianders@chromium.org,
linux-rockchip@lists.infradead.org, kyungmin.park@samsung.com,
linux-arm-kernel@lists.infradead.org, tixy@linaro.org,
typ@rock-chips.com, sudeep.holla@arm.com, mark.rutland@arm.com,
linux-pm@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH v10 3/5] Documentation: bindings: add dt documentation for rk3399 dmc
Date: Mon, 12 Sep 2016 11:31:02 -0500 [thread overview]
Message-ID: <20160912163102.GA17262@rob-hp-laptop> (raw)
In-Reply-To: <1473051971-5500-4-git-send-email-hl@rock-chips.com>
On Mon, Sep 05, 2016 at 01:06:09PM +0800, Lin Huang wrote:
> This patch adds the documentation for rockchip rk3399 dmc driver.
>
> Signed-off-by: Lin Huang <hl@rock-chips.com>
> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
> Changes in v10:
And this is the first I see it?
> - add rockchip prefix in property describe
>
> Changes in v9:
> - add ddr timing property to node
>
> Changes in v8:
> - add ddr timing properties
>
> Changes in v7:
> - None
>
> Changes in v6:
> -Add more detail in Documentation
>
> Changes in v5:
> -None
>
> Changes in v4:
> -None
>
> Changes in v3:
> -None
>
> Changes in v2:
> -None
>
> Changes in v1:
> -None
> .../devicetree/bindings/devfreq/rk3399_dmc.txt | 202 +++++++++++++++++++++
> 1 file changed, 202 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
>
> diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
> new file mode 100644
> index 0000000..84660a3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
> @@ -0,0 +1,210 @@
> +* Rockchip rk3399 DMC(Dynamic Memory Controller) device
> +
> +Required properties:
> +- compatible: Must be "rockchip,rk3399-dmc".
> +- devfreq-events: Node to get DDR loading, Refer to
> + Documentation/devicetree/bindings/devfreq/
> + rockchip-dfi.txt
> +- interrupts: The interrupt number to the CPU. The interrupt
> + specifier format depends on the interrupt controller.
> + It should be DCF interrupts, when DDR dvfs finish,
> + it will happen.
> +- clocks: Phandles for clock specified in "clock-names" property
> +- clock-names : The name of clock used by the DFI, must be
> + "pclk_ddr_mon";
> +- operating-points-v2: Refer to Documentation/devicetree/bindings/power/opp.txt
> + for details.
> +- center-supply: DMC supply node.
> +- status: Marks the node enabled/disabled.
No need to document this.
> +
> +Following properties are ddr timing:
This is a lot of properties. I would prefer to see SPD data embedded
into DT if there are overlap in what SPD data defines for settings.
> +
> +- rockchip,dram_speed_bin : Value reference include/dt-bindings/clock/ddr.h,
Use '-', not '_' throughout.
> + it select ddr3 cl-trp-trcd type, default value
> + "DDR3_DEFAULT".it must selected according to
> + "Speed Bin" in ddr3 datasheet, DO NOT use
> + smaller "Speed Bin" than ddr3 exactly is.
> +
> +- rockchip,pd_idle : Config the PD_IDLE value, defined the power-down
> + idle period, memories are places into power-down
> + mode if bus is idle for PD_IDLE DFI clocks.
> +
> +- rockchip,sr_idle : Configure the SR_IDLE value, defined the
> + selfrefresh idle period, memories are places
> + into self-refresh mode if bus is idle for
> + SR_IDLE*1024 DFI clocks (DFI clocks freq is
> + half of dram's clocks), defaule value is "0".
> +
> +- rockchip,sr_mc_gate_idle : Defined the self-refresh with memory and
> + controller clock gating idle period, memories
> + are places into self-refresh mode and memory
> + controller clock arg gating if bus is idle for
> + sr_mc_gate_idle*1024 DFI clocks.
> +
> +- rockchip,srpd_lite_idle : Defined the self-refresh power down idle
> + period, memories are places into self-refresh
> + power down mode if bus is idle for
> + srpd_lite_idle*1024 DFI clocks. This parameter
> + is for LPDDR4 only.
> +
> +- rockchip,standby_idle : Defined the standby idle period, memories are
> + places into self-refresh than controller, pi,
> + phy and dram clock will gating if bus is idle
> + for standby_idle * DFI clocks.
> +
> +- rockchip,dram_dll_disb_freq : It's defined the DDR3 dll bypass frequency in
> + MHz, when ddr freq less than DRAM_DLL_DISB_FREQ,
> + ddr3 dll will bypssed note: if dll was bypassed,
> + the odt also stop working.
> +
> +- rockchip,phy_dll_disb_freq : Defined the PHY dll bypass frequency in
> + MHz (Mega Hz), when ddr freq less than
> + DRAM_DLL_DISB_FREQ, phy dll will bypssed.
> + note: phy dll and phy odt are independent.
> +
> +- rockchip,ddr3_odt_disb_freq : When dram type is DDR3, this parameter defined
Please group properties that are required and optional, and also group
by common, DDR3 only, DDR4L only, etc.
> + the odt disable frequency in MHz (Mega Hz),
> + when ddr frequency less then ddr3_odt_disb_freq,
> + the odt on dram side and controller side are
> + both disabled.
> +
> +- rockchip,ddr3_drv : When dram type is DDR3, this parameter define
> + the dram side driver stength in ohm, default
> + value is DDR3_DS_40ohm.
> +
> +- rockchip,ddr3_odt : When dram type is DDR3, this parameter define
> + the dram side ODT stength in ohm, default value
> + is DDR3_ODT_120ohm.
> +
> +- rockchip,phy_ddr3_ca_drv : When dram type is DDR3, this parameter define
> + the phy side CA line(incluing command line,
> + address line and clock line) driver strength.
> + Default value is PHY_DRV_ODT_40.
> +
> +- rockchip,phy_ddr3_dq_drv : When dram type is DDR3, this parameter define
> + the phy side DQ line(incluing DQS/DQ/DM line)
> + driver strength. default value is PHY_DRV_ODT_40.
You have the same properties, but they just vary by DDR type. Make the
property names common.
> +
> +- rockchip,phy_ddr3_odt : When dram type is DDR3, this parameter define the
> + phy side odt strength, default value is
> + PHY_DRV_ODT_240.
> +
> +- rockchip,lpddr3_odt_disb_freq : When dram type is LPDDR3, this parameter defined
> + then odt disable frequency in MHz (Mega Hz),
> + when ddr frequency less then ddr3_odt_disb_freq,
> + the odt on dram side and controller side are
> + both disabled.
> +
> +- rockchip,lpddr3_drv : When dram type is LPDDR3, this parameter define
> + the dram side driver stength in ohm, default
> + value is LP3_DS_34ohm.
> +
> +- rockchip,lpddr3_odt : When dram type is LPDDR3, this parameter define
> + the dram side ODT stength in ohm, default value
> + is LP3_ODT_240ohm.
Is this in ohms or a register value? Please make it in ohms and add a
-ohms suffix.
> +Example:
> + dmc_opp_table: dmc_opp_table {
> + compatible = "operating-points-v2";
> +
> + opp00 {
> + opp-hz = /bits/ 64 <300000000>;
> + opp-microvolt = <900000>;
> + };
> + opp01 {
> + opp-hz = /bits/ 64 <666000000>;
> + opp-microvolt = <900000>;
> + };
> + };
> +
> + dmc: dmc {
> + compatible = "rockchip,rk3399-dmc";
> + devfreq-events = <&dfi>;
> + interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru SCLK_DDRCLK>;
> + clock-names = "dmc_clk";
> + operating-points-v2 = <&dmc_opp_table>;
> + center-supply = <&ppvar_centerlogic>;
> + upthreshold = <15>;
> + downdifferential = <10>;
> + rockchip,ddr3_speed_bin = <21>;
> + rockchip,pd_idle = <0x40>;
> + rockchip,sr_idle = <0x2>;
> + rockchip,sr_mc_gate_idle = <0x3>;
> + rockchip,srpd_lite_idle = <0x4>;
> + rockchip,standby_idle = <0x2000>;
> + rockchip,dram_dll_dis_freq = <300>;
> + rockchip,phy_dll_dis_freq = <125>;
> + rockchip,auto_pd_dis_freq = <666>;
> + rockchip,ddr3_odt_dis_freq = <333>;
> + rockchip,ddr3_drv = <DDR3_DS_40ohm>;
> + rockchip,ddr3_odt = <DDR3_ODT_120ohm>;
> + rockchip,phy_ddr3_ca_drv = <PHY_DRV_ODT_40>;
> + rockchip,phy_ddr3_dq_drv = <PHY_DRV_ODT_40>;
> + rockchip,phy_ddr3_odt = <PHY_DRV_ODT_240>;
> + rockchip,lpddr3_odt_dis_freq = <333>;
> + rockchip,lpddr3_drv = <LP3_DS_34ohm>;
> + rockchip,lpddr3_odt = <LP3_ODT_240ohm>;
> + rockchip,phy_lpddr3_ca_drv = <PHY_DRV_ODT_40>;
> + rockchip,phy_lpddr3_dq_drv = <PHY_DRV_ODT_40>;
> + rockchip,phy_lpddr3_odt = <PHY_DRV_ODT_240>;
> + rockchip,lpddr4_odt_dis_freq = <333>;
> + rockchip,lpddr4_drv = <LP4_PDDS_60ohm>;
> + rockchip,lpddr4_dq_odt = <LP4_DQ_ODT_40ohm>;
> + rockchip,lpddr4_ca_odt = <LP4_CA_ODT_40ohm>;
> + rockchip,phy_lpddr4_ca_drv = <PHY_DRV_ODT_40>;
> + rockchip,phy_lpddr4_ck_cs_drv = <PHY_DRV_ODT_80>;
> + rockchip,phy_lpddr4_dq_drv = <PHY_DRV_ODT_80>;
> + rockchip,phy_lpddr4_odt = <PHY_DRV_ODT_60>;
You have DDR3, LPDDR3 and LPDDR4 all populated at the same time? It
should be one set at a time.
Rob
next prev parent reply other threads:[~2016-09-12 16:31 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-05 5:06 [PATCH v10 0/5] rk3399 support ddr frequency scaling Lin Huang
2016-09-05 5:06 ` [PATCH v10 1/5] Documentation: bindings: add dt documentation for dfi controller Lin Huang
2016-09-12 16:18 ` Rob Herring
2016-09-05 5:06 ` [PATCH v10 2/5] PM / devfreq: event: support rockchip " Lin Huang
2016-09-05 5:06 ` [PATCH v10 3/5] Documentation: bindings: add dt documentation for rk3399 dmc Lin Huang
2016-09-12 16:31 ` Rob Herring [this message]
2016-09-05 5:06 ` [PATCH v10 4/5] PM / devfreq: rockchip: add devfreq driver " Lin Huang
2016-09-05 5:06 ` [PATCH v10 5/5] drm/rockchip: Add dmc notifier in vop driver Lin Huang
2016-09-06 3:40 ` kbuild test robot
2016-09-06 17:18 ` Sean Paul
2016-09-06 18:15 ` hl
2016-09-06 18:55 ` Sean Paul
2016-09-06 19:01 ` hl
2016-09-06 19:07 ` Sean Paul
2016-09-07 18:07 ` dbasehore .
2016-09-07 18:27 ` Sean Paul
2016-09-07 18:13 ` dbasehore .
2016-09-07 20:31 ` Sean Paul
2016-09-07 22:21 ` dbasehore .
[not found] ` <CGME20160905050631epcas1p4ebe75a5f119132f583a5020187382026@epcas1p4.samsung.com>
2016-09-06 4:37 ` MyungJoo Ham
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160912163102.GA17262@rob-hp-laptop \
--to=robh@kernel.org \
--cc=airlied@linux.ie \
--cc=cw00.choi@samsung.com \
--cc=dbasehore@chromium.org \
--cc=devicetree@vger.kernel.org \
--cc=dianders@chromium.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=heiko@sntech.de \
--cc=hl@rock-chips.com \
--cc=kyungmin.park@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mark.rutland@arm.com \
--cc=mark.yao@rock-chips.com \
--cc=mturquette@baylibre.com \
--cc=myungjoo.ham@samsung.com \
--cc=sboyd@codeaurora.org \
--cc=sudeep.holla@arm.com \
--cc=tixy@linaro.org \
--cc=typ@rock-chips.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).