svn commit: r237742 - in head/sys/arm: at91 conf

[Warner Losh]

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.

>> +		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 :)

> 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?

Warner