AF9033 read_ber and read_ucblocks implementation
Commit Message
Implementation of af9033_read_ber and af9033_read_ucblocks functions.
Signed-off-by: Hans-Frieder Vogt <hfvogt@gmx.net>
drivers/media/dvb/dvb-usb/af9033.c | 68 +++++++++++++++++++++++++++++++++++--
1 file changed, 66 insertions(+), 2 deletions(-)
Hans-Frieder Vogt e-mail: hfvogt <at> gmx .dot. net
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Comments
On 02.04.2012 00:19, Hans-Frieder Vogt wrote:
> Implementation of af9033_read_ber and af9033_read_ucblocks functions.
>
> Signed-off-by: Hans-Frieder Vogt<hfvogt@gmx.net>
> drivers/media/dvb/dvb-usb/af9033.c | 68 +++++++++++++++++++++++++++++++++++--
Same wrong path issue.
> + /* only update data every half second */
> + if (time_after(jiffies, state->last_stat_check + 500 * HZ / 1000)) {
500 * HZ looks odd. I suspect correct way is to use some macros for
that. See af9013 example usage of jiffies or explain the reason why 500
* HZ :)
> + sw = ~sw;
I don't see any reason for that?
It looked somehow complex, but I trust those calculations are kinda correct.
IMHO it is enough to put random raw number from register to UCB and BER
counters. Random numbers are good enough unless you are making
measurement equipment. But off-course correct calculations are allowed.
regards
Antti
On 02.04.2012 00:56, Antti Palosaari wrote:
> On 02.04.2012 00:19, Hans-Frieder Vogt wrote:
>> Implementation of af9033_read_ber and af9033_read_ucblocks functions.
>> + sw = ~sw;
>
> I don't see any reason for that?
Now I see, it is some kind of switch to make operation every second call?
As it is defined static:
+ static u8 sw = 0;
[...]
+ if (sw)
+ state->ucb = abort_cnt;
+ else
+ state->ucb = +abort_cnt;
+ sw = ~sw;
Unfortunately I am almost sure it will not work as it should. In my
understanding this kind of static variables are shared between driver
instances... and if you have device where is two or more af9033 demods
it will share it. Also if you have multiple af9033 devices, are are
shared. It works only if you have one af9033 demodulator in use.
Instead, this kind of switches should be but to driver private aka
state. Also think twice if all that logic is correct and needed.
regards
Antti
@@ -29,6 +29,10 @@ struct af9033_state {
u32 bandwidth_hz;
bool ts_mode_parallel;
bool ts_mode_serial;
+
+ u32 ber;
+ u32 ucb;
+ unsigned long last_stat_check;
};
/* write multiple registers */
@@ -594,16 +598,76 @@ err:
return ret;
}
+static int af9033_update_ch_stat(struct af9033_state *state)
+{
+ int ret = 0;
+ u32 post_err_cnt, post_bit_cnt;
+ u16 abort_cnt;
+ u8 buf[7];
+ static u8 sw = 0;
+
+ /* only update data every half second */
+ if (time_after(jiffies, state->last_stat_check + 500 * HZ / 1000)) {
+ ret = af9033_rd_regs(state, 0x800032, buf, sizeof(buf));
+ if (ret < 0)
+ goto err;
+ abort_cnt = (buf[1] << 8) + buf[0];
+ post_err_cnt = (buf[4] << 16) + (buf[3] << 8) + buf[2];
+ post_bit_cnt = (buf[6] << 8) + buf[5];
+ if (post_bit_cnt == 0) {
+ abort_cnt = 1000;
+ post_err_cnt = 1;
+ post_bit_cnt = 2;
+ } else {
+ post_bit_cnt -= (u32)abort_cnt;
+ if (post_bit_cnt == 0) {
+ post_err_cnt = 1;
+ post_bit_cnt = 2;
+ } else {
+ post_err_cnt -= (u32)abort_cnt * 8 * 8;
+ post_bit_cnt *= 204 * 8;
+ }
+ }
+ state->ber = post_err_cnt * (0xffffffff / post_bit_cnt);
+ /* reset ucblocks value every second call */
+ if (sw)
+ state->ucb = abort_cnt;
+ else
+ state->ucb = +abort_cnt;
+ sw = ~sw;
+ state->last_stat_check = jiffies;
+ }
+
+ return 0;
+err:
+ pr_debug("%s: failed=%d\n", __func__, ret);
+ return ret;
+}
+
static int af9033_read_ber(struct dvb_frontend *fe, u32 *ber)
{
- *ber = 0;
+ struct af9033_state *state = fe->demodulator_priv;
+ int ret;
+
+ ret = af9033_update_ch_stat(state);
+ if (ret < 0)
+ return ret;
+
+ *ber = state->ber;
return 0;
}
static int af9033_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
{
- *ucblocks = 0;
+ struct af9033_state *state = fe->demodulator_priv;
+ int ret;
+
+ ret = af9033_update_ch_stat(state);
+ if (ret < 0)
+ return ret;
+
+ *ucblocks = state->ucb;
return 0;
}