[CCC DEV] Problem with servo = possible bug with plumbing pwm code

[Steve Pretty]

I have previously mentioned that I have not been able to get a servo 
working correctly on pin 10 of my arduino uno.  The servo works fine on 
the same port using the arduino wiring software.

I have investigated this problem, and think I have a resolution.

I immediately noticed that the servo.module relies completely om the 
pwm16 code.

I therefore tried a test program to fade up an LED using pwm16 on pin 10 
- did not work.
A similar program on pin 11 using pwm8 did work correctly.

I then deeply explored the the code, in conjunction with the Atmel 328 
manual.  The 8 bit and 16 bit counter times are pretty much identical in 
the way they work, apart from the size of the working registers. The 
pwm8 and pwm16 code chains certainly appeared to be writing the correct 
values to the correct port addresses.

I finally spotted something that did not make sense in the 
\arch\common\pwm.common.module.  This is in both the pwm8.setup and the 
pwm16.setup.

The issue concerns how the control registers TCCRnA and TCCRnB are written.

Values for the variables TCCRnA.bits and TCCRnB.bits have been 
calculated that will make the timers work in the way required.

Unfortunately, the code then goes on to READ the values of the control 
registers and ORs the read value with the values in TCCRnA.bits.  I can 
see no reason for doing that - it is ORing the required configuration 
values with whatever values may have been in those registers 
previously.  This is a particularly nasty little bug - as the code will 
work for some of the people some of the time, but definitely not all of 
the people all of the time.

Removing the read / OR from the code fixes the problem - pwm8 and pmw16 
tests work fine (on both pins 10 and 9), as does servo (tested on pin 10).

In the listing below, I have copied the original code lines, commented 
out the bit that appears wrong, and corrected the copy.

--{{{ PROC pwm8.setup
--* Sets up phase-correct Pulse Width Modulation on an 8 bit timer.
--
-- Be aware that not every pin on any given board or architecture
-- can be operated as a PWM Pin. Consult your board for information on which
-- pins can produce PWM.
-- 
-- @param avr.pin The [@em AVR] pin to activate PWM on.
-- @param prescale The prescaler value to set with PWM.
PROC pwm8.setup (VAL INT avr.pin, prescale)
   PLACED [MAX.PORT]BYTE ports 0:
   #PRAGMA DEFINED ports
   INT TCCRnA, TCCRnB, OCRnx:
   BYTE TCCRnA.bits, TCCRnB.bits:

   SEQ
     TCCRnA, TCCRnB, OCRnx, TCCRnA.bits, TCCRnB.bits := pwm8.lookup 
(avr.pin)
     TCCRnB.bits := pwm8.TCCRnB.helper (TCCRnB.bits, prescale)
     ports[TCCRnA] := TCCRnA.bits
     ports[TCCRnB] := TCCRnB.bits
--    ports[TCCRnA] := ports[TCCRnA] \/ TCCRnA.bits
--    ports[TCCRnB] := ports[TCCRnB] \/ TCCRnB.bits
     ports[OCRnx]  := 0
:
--}]}

--{{{ PROC pwm16.setup
--* Sets up phase-and-frequency-correct Pulse Width Modulation on a
-- 16 bit timer.
--
-- Be aware that not every pin on any given board or architecture
-- can be operated as a PWM Pin, and [@em not every PWM pin operates on a 16
-- bit timer].
--
-- Consult your board's reference manual for information on which pins can
-- produce 16 bit PWM.
-- 
-- @param avr.pin The [@em AVR] pin to activate PWM on.
-- @param prescale The prescaler value to set with PWM.
PROC pwm16.setup (VAL INT avr.pin, prescale)
   PLACED [MAX.PORT]BYTE ports 0:
   #PRAGMA DEFINED ports
   INT TCCRnA, TCCRnB, OCRnx:
   BYTE TCCRnA.bits, TCCRnB.bits:

   SEQ
     TCCRnA, TCCRnB, OCRnx, TCCRnA.bits, TCCRnB.bits := pwm16.lookup 
(avr.pin)
     TCCRnB.bits := pwm16.TCCRnB.helper (TCCRnB.bits, prescale)
     ports[TCCRnA] := TCCRnA.bits
     ports[TCCRnB] := TCCRnB.bits
--    ports[TCCRnA] := ports[TCCRnA] \/ TCCRnA.bits
--    ports[TCCRnB] := ports[TCCRnB] \/ TCCRnB.bits
     ports[OCRnx]  := 0
:
--}}}

Steve