From owner-freebsd-arm@FreeBSD.ORG Tue Jul 31 20:59:00 2012 Return-Path: Delivered-To: arm@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F2ED1106566C for ; Tue, 31 Jul 2012 20:58:59 +0000 (UTC) (envelope-from Daan@vitsch.nl) Received: from VM01.VEHosting.nl (VM016.VEHosting.nl [IPv6:2001:1af8:2100:b020::140]) by mx1.freebsd.org (Postfix) with ESMTP id 8CE0A8FC08 for ; Tue, 31 Jul 2012 20:58:59 +0000 (UTC) Received: from [2001:470:d233:2:224:8cff:fe82:66cd] ([IPv6:2001:470:d233:2:224:8cff:fe82:66cd]) (authenticated bits=0) by VM01.VEHosting.nl (8.14.3/8.13.8) with ESMTP id q6VKx0lC053026; Tue, 31 Jul 2012 22:59:00 +0200 (CEST) (envelope-from Daan@vitsch.nl) From: Daan Vreeken Organization: Vitsch Electronics To: Warner Losh Date: Tue, 31 Jul 2012 22:58:55 +0200 User-Agent: KMail/1.9.10 References: <201206290418.q5T4IqpX018099@svn.freebsd.org> <201207310028.12533.Daan@vitsch.nl> <8A50DD5A-4329-4CAB-949D-22606527B7E4@bsdimp.com> In-Reply-To: <8A50DD5A-4329-4CAB-949D-22606527B7E4@bsdimp.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <201207312258.55870.Daan@vitsch.nl> x-ve-auth-version: mi-1.1.5 2011-02-07 - Copyright (c) 2008, 2011 - Daan Vreeken - VEHosting x-ve-auth: authenticated as 'pa4dan' on VM01.VEHosting.nl Cc: "arm@freebsd.org" Subject: Re: svn commit: r237742 - in head/sys/arm: at91 conf X-BeenThere: freebsd-arm@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the StrongARM Processor List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 Jul 2012 20:59:00 -0000 Hi Warner, On Tuesday 31 July 2012 21:37:04 Warner Losh wrote: > Hi Daan, > > thanks for your feedback. Comments inline below. I've also redirected > this to the ARM list. > > On Jul 30, 2012, at 4:28 PM, Daan Vreeken wrote: > > Hi Warner, > > > > On Friday 29 June 2012 06:18:52 Warner Losh wrote: > >> Author: imp > >> Date: Fri Jun 29 04:18:52 2012 > >> New Revision: 237742 > >> URL: http://svn.freebsd.org/changeset/base/237742 > >> > >> Log: > >> Initital support for AT91SAM9X25 SoC and the SAM9X25-EK evaluation > >> board. Much work remains. > > > > ... > > > >> Added: head/sys/arm/at91/at91sam9x25.c > >> ======================================================================== > >>=== === --- /dev/null 00:00:00 1970 (empty, because file is newly added) > >> +++ head/sys/arm/at91/at91sam9x25.c Fri Jun 29 04:18:52 2012 (r237742) > >> @@ -0,0 +1,343 @@ > > > > ... > > > >> +static uint32_t > >> +at91_pll_outa(int freq) > >> +{ > >> + > >> + switch (freq / 10000000) { > > > > You seem to be dividing freq into multiples of 10MHz instead of 1MHz > > here. I think dividing by 1e6 was the intention. > > You're right. That was wrong in the 9g20 code too. I'll correct it there > as well. Ok. > >> + case 747 ... 801: return ((1 << 29) | (0 << 14)); > >> + case 697 ... 746: return ((1 << 29) | (1 << 14)); > >> + case 647 ... 696: return ((1 << 29) | (2 << 14)); > >> + case 597 ... 646: return ((1 << 29) | (3 << 14)); > >> + case 547 ... 596: return ((1 << 29) | (1 << 14)); > >> + case 497 ... 546: return ((1 << 29) | (2 << 14)); > >> + case 447 ... 496: return ((1 << 29) | (3 << 14)); > >> > >> + case 397 ... 446: return ((1 << 29) | (4 << 14)); > > > > The (4 << 14) looks a bit strange here, as OUTA only occupies bit 14 and > > 15 of CKGR_PLLAR. (See Atmel doc11054, page 201 and 1103.) > > Yes. I've never liked this code, and had an item on my todo list to go dig > into it. Thanks for doing the digging for me :) You're welcome. > > Maybe this entire routine could be written as something like: > > uint8_t outa; > > > > freq /= 1000000; > > // optional: > > //freq += 3; > > // see doc11054, page 1103 > > outa = 3 - ((freq / 50) & 3); > > > > return ((1 << 29) | (outa << 14)); > > I like this code a lot. In fact, it looks like all the other PLLA OUTA > calculations are wrong for all the other chips. > > > Just glancing at the code, setting ICPLLA in PMC_PLLICPR for frequencies > > <= 600MHz seems to be missing at this moment (or I'm just not seeing it). > > (see page 212 and 1103) > > You're right. But it doesn't matter since we never seem to actually set > PLLA on startup. > > I've gone through and fixed things, and put better labels on the references > to the data sheets. > > See http://people.freebsd.org/~imp/clocks.diff for my fix. Does that look > good to you? How about to the arm@ peanut gallery? Almost perfect :) Although I think you've got the overlap-fix backwards. With 'freq -= 3', freq has to be 3 higher instead of lower before the next range is selected. I see that you've also fixed 195MHz to 155MHz in the 9260 PLL code. I've had the same diff in one of our local patch sets at work for a while. I'll see if I can find some time to filter out more local changes that could aid others. Regards, -- Daan Vreeken Vitsch Electronics http://Vitsch.nl/ http://VitschVPN.nl/ tel: +31-(0)40-7113051 KvK nr: 17174380