From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Porter Date: Wed, 20 Mar 2013 09:25:43 -0400 Subject: [U-Boot] [PATCH 1/4] am33xx: add pll and clock support for TI814x CPSW In-Reply-To: References: <1363381100-6364-1-git-send-email-mporter@ti.com> <1363381100-6364-2-git-send-email-mporter@ti.com> Message-ID: <20130320132543.GF18335@beef> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Mar 15, 2013 at 09:13:54PM +0000, Tom Rini wrote: > On Fri, Mar 15, 2013 at 04:58:17PM -0400, Matt Porter wrote: > > [snip] > > + /* Ethernet */ > > + writel(PRCM_MOD_EN, &cmalwon->l3slowclkstctrl); > > + while ((readl(&cmalwon->l3slowclkstctrl) & 0x2100) != 0x2100) > > + ; > > + writel(PRCM_MOD_EN, &cmalwon->ethclkstctrl); > > + writel(PRCM_MOD_EN, &cmalwon->ethernet0clkctrl); > > + while ((readl(&cmalwon->ethernet0clkctrl) & 0x30000) != 0) > > + ; > > + writel(PRCM_MOD_EN, &cmalwon->ethernet1clkctrl); > > + while ((readl(&cmalwon->ethernet1clkctrl) & 0x30000) != 0) > > Please define away the magic numbers. > > [snip] > > +void sata_pll_config(void) > > +{ > > + /* TRM 21.3.1 */ > > + writel(0xc12c003c, &spll->pllcfg1); > > I'm OK with comments, but please make it a multi-line thing that > explains what's going on so that it's clear that yes, really, we > shouldn't have defined these. I'll address this with gratuitous defines in v2...and expand the comment. -Matt