public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Kever Yang <kever.yang@rock-chips.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH 2/3] arm64: rk3399: add ddr controller driver
Date: Wed, 18 Jan 2017 17:55:09 +0800	[thread overview]
Message-ID: <587F3B7D.9070901@rock-chips.com> (raw)
In-Reply-To: <CAPnjgZ032uzuvVvD2Dm=hVpKT6wCDPqcJ_bjVpJUUwgGuHFCZw@mail.gmail.com>

Hi Simon,

     Thanks for your review, I would like to take all the comments but 
one below,
because I think the change only get a lot of work to do but have no any help
on understand the source code, because each of shift/mask operation has 
comment
for it.

     I have spend a lot of time on make different controller base 
addresses from dts
and gather them into one structure, which we think it does not make any 
helpful on
source code itself but only met some bug on of-platdata for address decode.

     There are hundreds of ctrl/pi/phy register in this dram controller, 
I don't think it's a
good idea to add MASK/SHIFT MACRO definition for all of them since comments
are already there.

     I will send my patch V2 without mask/shift change, but updates by 
your other comments.

Thanks,
- Kever

On 01/13/2017 10:18 AM, Simon Glass wrote:
>> >+{
>> >+       u32 *denali_phy = ddr_publ_regs->denali_phy;
>> >+       if (freq <= 125*MHz) {
>> >+               /* phy_sw_master_mode_X PHY_86/214/342/470 4bits offset_8 */
>> >+               setbits_le32(&denali_phy[86], (0x3 << 2) << 8);
>> >+               setbits_le32(&denali_phy[214], (0x3 << 2) << 8);
>> >+               setbits_le32(&denali_phy[342], (0x3 << 2) << 8);
>> >+               setbits_le32(&denali_phy[470], (0x3 << 2) << 8);
> These should really be defined as SHIFT and MASK values.
>

  reply	other threads:[~2017-01-18  9:55 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-29 10:25 [U-Boot] [RFC PATCH 0/3] arm64: rk3399: enable SPL with ATF support Kever Yang
2016-12-29 10:25 ` [U-Boot] [RFC PATCH 1/3] arm64: rk3399: add SPL support Kever Yang
2017-01-13  2:11   ` Simon Glass
2016-12-29 10:25 ` [U-Boot] [RFC PATCH 2/3] arm64: rk3399: add ddr controller driver Kever Yang
2017-01-13  2:18   ` Simon Glass
2017-01-18  9:55     ` Kever Yang [this message]
2016-12-29 10:25 ` [U-Boot] [RFC PATCH 3/3] spl: add support to booting with ATF Kever Yang
2017-01-02 15:05   ` Michal Simek
2017-01-06  7:09     ` Kever Yang
2017-01-06  7:55       ` Michal Simek
2017-01-26 14:23         ` Simon Glass
2017-01-28 21:55           ` Tom Rini
2017-01-18 12:20   ` Kever Yang
2017-01-02 15:05 ` [U-Boot] [RFC PATCH 0/3] arm64: rk3399: enable SPL with ATF support Michal Simek
2017-01-06  7:11   ` Kever Yang

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=587F3B7D.9070901@rock-chips.com \
    --to=kever.yang@rock-chips.com \
    --cc=u-boot@lists.denx.de \
    /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