gnode's cc1101 radio FSTEST is readonly register

[andresv]

I noticed that in gnode's cc1101 driver there is FSTEST register defined and actually value is written into that register, however actually it is read-only register.

Here it is defined: https://github.com/tp-freeforall/prod/blob/msp430-int/tos/platforms/gnode/chips/cc1101/ChipconRegisterValues.h

And here it is used: https://github.com/tp-freeforall/prod/blob/msp430-int/tos/platforms/gnode/chips/ccpacket/hal/HalChipconControlP.nc

/**
 * Load default configuration into registers. 
 */
void configure() {
    uint8_t i;
    for (i = 0; i < NUM_REGISTERS * 2; i += 2) {
        uint8_t reg = chipconRegisterValues[i];
        uint8_t value = chipconRegisterValues[i+1];
        call HplChipconSpi.writeRegister(reg, value);
    }
}

It seems to produce no sideeffects, but it is not correct.

I am not willing to remove it yet, maybe I am missing something. It is better if mkonstapel would comment this issue first.

[cire831]

I checked in the cc1101 data sheet (manual) and this register is actually r/w. It is labelled test only and not to be written so probably not a good idea to write to it but technically it is r/w. The current code sets this register to the documented reset value (why one needs to write a register to its reset value, well TI does weird stuff). I think hitting a reset pin would do the trick (but the chip doesn't have a reset pin). The only reset that happens is when the chip powers up. So it is kind of essential that the s/w actually writes the register back to the reset value. (I suspect it doesn't change, but writing the reset value doesn't hurt either)

I don't know if the configuration tools produce a value that needs to be loaded when a configuration is loaded.

I wouldn't mess with it. Also it is in the middle of a block of registers, the value written 0x59 is the documented reset value. So I don't see any need to complicate things (yeah it isn't too bad, but why bother) by changing this.

[mkonstapel]

All the register settings are generated by their SmartRF Studio tool. The ones we've manually set are the commented ones below TEST0. The tool's output includes the FSTEST register, so I'd say we can safely leave it in.

[cire831]

Not going to be removed. The block of data written to the registers is obtained from the SmartRF Studio Tool. This tool includes the reset value for the FSTEST register.

Removing it would be moving in the wrong direction and doesn't fix anything and nothing is actually broken.