From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex G. Date: Fri, 14 Mar 2014 12:23:50 -0500 Subject: [U-Boot] [PATCH v1 3/9] sunxi: initial sun7i dram setup support In-Reply-To: <20140314141707.GG16360@bill-the-cat> References: <1394793212.13270.89.camel@hastur.hellion.org.uk> <1394793231-11922-3-git-send-email-ijc@hellion.org.uk> <20140314141707.GG16360@bill-the-cat> Message-ID: <53233B26.2070904@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 03/14/2014 09:17 AM, Tom Rini wrote: > On Fri, Mar 14, 2014 at 10:33:45AM +0000, Ian Campbell wrote: > > [snip] >> +static void mctl_ddr3_reset(void) +{ + struct sunxi_dram_reg >> *dram = + (struct sunxi_dram_reg *)SUNXI_DRAMC_BASE; + + { + >> clrbits_le32(&dram->mcr, DRAM_MCR_RESET); + udelay(2); + >> setbits_le32(&dram->mcr, DRAM_MCR_RESET); + } > > That seems like an odd construction, why the extra braces? > This originally had a conditional depending on the SoC family. (Yeah, they need to reset the ram differently). It seems it wasn't removed properly. > And as for the rest of the code, lots of magic numbers to #define > what/why (why udelay(2) and 22?) > Before going into more detail, remember this is ram initialization code. That's always going to be a pain :(. There's nothing magic here. It's just a fact of life. Every step is going to need a different delay. No need to bloat the headers by #defining each. It also makes raminit code more unreadable. We got these numbers from allwinner code dumps. We used to have these as sdelay() numbers, which usually meant units of 2 clock cycles. So we had to convert them to udelay() to at least make the delays independent of CPU clock. The old sdelay() numbers made no sense either. Alex -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQIcBAEBAgAGBQJTIzsgAAoJEL0kPHNPXJJKasUQAKygf9UpJc6IINH41LKOFnBf fBEVB8ECIu96JWUDE9r0BQw0l6Pc82Mqx+XUZdbNhhZNhhVf8EJ/9koHQrP/X8By J8MninT7pgDi7IH9Y+U5GfuHEA1nDQT0kf+nYMNrUIx1b9cuv5vLvhpeKL0AJotb DE8miBRCcuRcAivCjERjlVFKCicMERKx1cTrris7+aQyNAxPEGrFj0pS8RBpSgBF j+NkRSjbdnAOKHOfpXZi7VMqhHIt6NDxsuSrM0roSJz1v79+J5TGqUdPFKvGVZro BbsWB3pRq8zSdO6OYvojUjyWPG95PfhDQvLSlvKA5Z1AlgNHu32YGkFWGbUve/z7 8dn057RjY4KtrAn+jVOL8+sBAj6yr0VIJmxqa1xPqGOMBAn0Cn+JJZ6auCNPZGqx yt5glg0B5lHsVgCnmDGvoKs8SAv7n7IQfmiF+gvD8Pn0/NC31UQ2WA3khJoPt6Gp 9G5q+sn6BRrUpQOiQmhJTPX4W3g3nmXFqWYkd+VAWEHeEfHTIuqOlcfgdwMmP4Ti hrhrk/CWSOPjtYuI5yByA6RCZ93smb4uexSmrzzafzuYrdwK/DmP+TumFrE8pycT cMsxAx40UEElJ5OHN7YaWtt6NNhCT6BVlo76awqFY/vGme0cef74SQqB8EdyVYSE PDoq+Sog13y4A8OjQ5rT =e7kN -----END PGP SIGNATURE-----