From: Amaury Pouly <amaury.pouly_at_gmail.com>
Date: Wed, 1 Feb 2012 18:59:31 +0100

Hi all,
While working on the fuze+ radio (which is quite picky), I ran into the
mess known as our radio code and one issue in particular which is power
handling. Currently our code works with something I consider problematic
choice:
- a tuner API: tuner_get and tuner_set
- one power handling function: tuner_power
The problem is that tuner_get/set is handled by the driver and tuner_power
is device specific but they are used as the same level. To put it
differently, the power handling is completely bypassing the driver.
Currently this works because for most (if not all) devices, tuner_power
does not actually control the tuner power but the tuner chip enable or
something similar. A corollary is that tuner_power does not reset the
device and the registers.
If one looks carefully at the si4700 driver for example, it is clear that
it assumes the registers are never destroyed and thus that tuner_power does
not really control the power. Furthermore, nothing prevents our radio code
from calling tuner_set(RADIO_TUNE,x) while "powered off" which of course
makes no sense (I find the radio code complicated to follow but this is not
a surprise I guess). Such a thing could be handled properly if the power
state was part of the driver.

Unfortunately, I have good reasons to believe that this assumption does not
hold for the fuze+ and this is problematic in two ways:
- I cannot implement tuner_power since that would destroy the register, so
I would need to enable it once and let tuner_power as a stub
- but then, the generic radio code somehow relies on the fact that calling
tuner_power stops the radio without calling any other driver function in
some occasion (correct me if I'm wrong), which is not the case

What I am proposing is twofold:

1) Remove all direct calls to tuner_power and move them to the drivers and
replace all tuner_power(ed) calls from other parts of the code by calls to
tuner_set/get by introducing two new properties: RADIO_POWER(ED). So
concretely tuner_power(en) -> tuner_set(RADIO_POWER, en) and
tuner_powered() -> tuner_get(RADIO_POWERED).

2) In drivers like the si4700, implement tuner_set(RADIO_POWER,x) the
proper way by cleanly shutting show/powering up the radio before/after
calling tuner_power; thus removing the assumption that tuner_power doesn't
reset the registers. For the other drivers, we could simply forward
tuner_set(RADIO_POWER,x) to tuner_power.

Any opinion on that ? I already have some code written but it is not clean
enough for now and I don't want to go forward if the design appears to be
too problematic.

Best regards

Amaury Pouly
Received on 2012-02-01