stb0899: fix the limits for signal strength values
PATCH 1/1 stb0899: fix the limits for signal strength values
stb0899_read_signal_strength() adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +5 -3
@@ -67,7 +67,7 @@ * Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - { -950, -128 }, + { -750, -128 }, { -748, -94 }, { -745, -92 }, { -735, -90 },
@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } }; /* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/
@@ -964,6 +964,7 @@ int val; u32 reg; + *strength = 0; switch (state->delsys) { case SYS_DVBS: case SYS_DSS:
@@ -987,7 +988,7 @@ val = STB0899_GETFIELD(IF_AGC_GAIN, reg); *strength = stb0899_table_lookup(stb0899_dvbs2rf_tab, ARRAY_SIZE(stb0899_dvbs2rf_tab) - 1, val); - *strength += 750; + *strength += 950; dprintk(state->verbose, FE_DEBUG, 1, "IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm", val & 0x3fff, *strength); }
@@ -1009,6 +1010,7 @@ u8 buf[2]; u32 reg; + *snr = 0; reg = stb0899_read_reg(state, STB0899_VSTATUS); switch (state->delsys) { case SYS_DVBS:
Hi Klaus,
The patch didn't apply. It seems to be due to your emailer that mangled the whitespaces.
The patch looks correct on my eyes. Yet, I'd like to have Manu's ack on it.
Em 19-01-2012 15:39, Klaus Schmidinger escreveu:
Quoted 30 lines
stb0899_read_signal_strength()adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".Signed-off-by Klaus Schmidinger <[email protected]>linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +2
-2@@ -67,7 +67,7 @@ * Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - { -950, -128 }, + { -750, -128 }, { -748, -94 }, { -745, -92 }, { -735, -90 },@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } }; /* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 967
Quoted 4 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0@@ -964,6 +964,7 @@ int val; u32 reg; + *strength = 0;
This is not needed, as strength is not initialized only on invalid delivery systems, where -EINVAL is returned.
Quoted 13 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-1switch (state->delsys) { case SYS_DVBS: case SYS_DSS:@@ -987,7 +988,7 @@ val = STB0899_GETFIELD(IF_AGC_GAIN, reg); *strength = stb0899_table_lookup(stb0899_dvbs2rf_tab, ARRAY_SIZE(stb0899_dvbs2rf_tab) - 1, val); - *strength += 750; + *strength += 950; dprintk(state->verbose, FE_DEBUG, 1, "IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm", val & 0x3fff, *strength); }
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1013
Quoted 4 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0@@ -1009,6 +1010,7 @@ u8 buf[2]; u32 reg; + *snr = 0;
This is not needed, as strength is not initialized only on invalid delivery systems, where -EINVAL is returned.
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1016
Quoted 3 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +0
-0reg = stb0899_read_reg(state, STB0899_VSTATUS); switch (state->delsys) { case SYS_DVBS:
PS.: Another alternative for it would be the enclosed patch, wich will be a little simpler, and will also preserve the slope on the boundary values, used at the stb0899_table_lookup() interpolation logic.
Regards,
Mauro
-
[PATCH] stb0899: fix the limits for signal strength values
The current minimal measures for strengh is 750 - 950, for table stb0899_dvbsrf_tab[], and 750 - 999, for stb0899_dvbs2rf_tab[]. Both are negative values. However, the strength measure is unsigned.
Don't allow negative values for strengh to underflow. Instead, shift the scale, in order to have 0 as the lowest strength.
drivers/media/dvb/frontends/stb0899_drv.c +2 -2
@@ -975,7 +975,7 @@ static int stb0899_read_signal_strength(struct dvb_frontend *fe, u16 *strength) val = (s32 )(s8 )STB0899_GETFIELD (AGCIQVALUE , reg); *strength =stb0899_table_lookup (stb0899_dvbsrf_tab,ARRAY_SIZE (stb0899_dvbsrf_tab) -1 , val); - *strength +=750 ; + *strength +=950 ;dprintk (state->verbose,FE_DEBUG ,1 ,"AGCIQVALUE = 0x%02x, C = %d * 0.1 dBm" , val &0xff , *strength); }
@@ -987,7 +987,7 @@ static int stb0899_read_signal_strength(struct dvb_frontend *fe, u16 *strength) val =STB0899_GETFIELD (IF_AGC_GAIN , reg); *strength =stb0899_table_lookup (stb0899_dvbs2rf_tab,ARRAY_SIZE (stb0899_dvbs2rf_tab) -1 , val); - *strength +=750 ; + *strength +=999 ;dprintk (state->verbose,FE_DEBUG ,1 ,"IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm" , val &0x3fff , *strength); }
Quoted 4 lines · Mauro Carvalho Chehab
Hi Klaus,
The patch didn't apply. It seems to be due to your emailer that mangled the whitespaces.
Sorry about that. Here it is again as an attachment.
Quoted 34 lines
The patch looks correct on my eyes. Yet, I'd like to have Manu's ack on it.
Em 19-01-2012 15:39, Klaus Schmidinger escreveu:
Quoted 31 lines
stb0899_read_signal_strength()adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".Signed-off-by Klaus Schmidinger<[email protected]>linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +2
-2@@ -67,7 +67,7 @@ * Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - { -950, -128 }, + { -750, -128 }, { -748, -94 }, { -745, -92 }, { -735, -90 },@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } }; /* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/@@ -964,6 +964,7 @@
Quoted 6 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 967
Quoted 3 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0int val; u32 reg; + *strength = 0;This is not needed, as strength is not initialized only on invalid delivery systems, where
-EINVALis returned.
What about the 'if' conditions within the valid delivery system cases? For instance
case SYS_DSS:
if (internal->lock) {
...
if (STB0899_GETFIELD(VSTATUS_LOCKEDVIT, reg)) {
...
*strength = ...
}
}
break;So there may be cases where strength has an undefined value, even if this function returns 0.
Quoted 16 lines
Quoted 16 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-1switch (state->delsys) { case SYS_DVBS: case SYS_DSS:@@ -987,7 +988,7 @@ val = STB0899_GETFIELD(IF_AGC_GAIN, reg); *strength = stb0899_table_lookup(stb0899_dvbs2rf_tab, ARRAY_SIZE(stb0899_dvbs2rf_tab) - 1, val); - *strength += 750; + *strength += 950; dprintk(state->verbose, FE_DEBUG, 1, "IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm", val& 0x3fff, *strength); }@@ -1009,6 +1010,7 @@ u8 buf[2]; u32 reg;
Quoted 4 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1013
Quoted 1 line
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0+ *snr = 0;This is not needed, as strength is not initialized only on invalid delivery systems, where
-EINVALis returned.
See above.
Quoted 8 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1016
Quoted 3 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +0
-0reg = stb0899_read_reg(state, STB0899_VSTATUS); switch (state->delsys) { case SYS_DVBS:PS.: Another alternative for it would be the enclosed patch, wich will be a little simpler, and will also preserve the slope on the boundary values, used at the
stb0899_table_lookup()interpolation logic.
Well, as long as there are no negative values for strength... However, I don't quite see why the range is 750 - 950 for DVB-S and 750 - 999 for DVB-S2. Shouldn't this be the same maximum value in both cases?
Klaus
Quoted 25 lines
[PATCH] stb0899: fix the limits for signal strength values
The current minimal measures for strengh is 750 - 950, for table stb0899_dvbsrf_tab[], and 750 - 999, for stb0899_dvbs2rf_tab[]. Both are negative values. However, the strength measure is unsigned.
Don't allow negative values for strengh to underflow. Instead, shift the scale, in order to have 0 as the lowest strength.
Signed-off-by Mauro Carvalho Chehab<[email protected]>drivers/media/dvb/frontends/stb0899_drv.c +1
-1@@ -975,7 +975,7 @@ static int stb0899_read_signal_strength(struct dvb_frontend *fe, u16 *strength) val = (s32 )(s8 )STB0899_GETFIELD (AGCIQVALUE , reg); *strength =stb0899_table_lookup (stb0899_dvbsrf_tab,ARRAY_SIZE (stb0899_dvbsrf_tab) -1 , val); - *strength +=750 ; + *strength +=950 ;dprintk (state->verbose,FE_DEBUG ,1 ,"AGCIQVALUE = 0x%02x, C = %d * 0.1 dBm" , val&0xff , *strength); }
drivers/media/dvb/frontends/stb0899_drv.c static int stb0899_read_signal_strength(struct dvb_frontend *fe, u16 *strength) · line 993
Quoted 8 lines
drivers/media/dvb/frontends/stb0899_drv.c +1
-1@@ -987,7 +987,7 @@ static int stb0899_read_signal_strength(struct dvb_frontend *fe, u16 *strength) val =STB0899_GETFIELD (IF_AGC_GAIN , reg); *strength =stb0899_table_lookup (stb0899_dvbs2rf_tab,ARRAY_SIZE (stb0899_dvbs2rf_tab) -1 , val); - *strength +=750 ; + *strength +=999 ;dprintk (state->verbose,FE_DEBUG ,1 ,"IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm" , val&0x3fff , *strength); }
stb0899: fix the limits for signal strength values
stb0899_read_signal_strength() adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".
linux/drivers/media/dvb/frontends/stb0899_drv.c +5 -3
@@ -67,7 +67,7 @@* Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - {-950 ,-128 }, + {-750 ,-128 }, {-748 ,-94 }, {-745 ,-92 }, {-735 ,-90 },
@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } };/* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/
@@ -964,6 +964,7 @@int val;u32 reg; + *strength =0 ;switch (state->delsys) {case SYS_DVBS :case SYS_DSS :
@@ -987,7 +988,7 @@ val =STB0899_GETFIELD (IF_AGC_GAIN , reg); *strength =stb0899_table_lookup (stb0899_dvbs2rf_tab,ARRAY_SIZE (stb0899_dvbs2rf_tab) -1 , val); - *strength +=750 ; + *strength +=950 ;dprintk (state->verbose,FE_DEBUG ,1 ,"IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm" , val &0x3fff , *strength); }
@@ -1009,6 +1010,7 @@u8 buf[2 ];u32 reg; + *snr =0 ; reg =stb0899_read_reg (state,STB0899_VSTATUS );switch (state->delsys) {case SYS_DVBS :
Em 24-01-2012 07:33, Klaus Schmidinger escreveu:
Quoted 7 lines · Mauro Carvalho Chehab
Quoted 4 lines
Hi Klaus,
The patch didn't apply. It seems to be due to your emailer that mangled the whitespaces.
Sorry about that. Here it is again as an attachment.
Thanks!
Quoted 54 lines
Quoted 34 lines
The patch looks correct on my eyes. Yet, I'd like to have Manu's ack on it.
Em 19-01-2012 15:39, Klaus Schmidinger escreveu:
Quoted 31 lines
stb0899_read_signal_strength()adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".Signed-off-by Klaus Schmidinger<[email protected]>linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +2
-2@@ -67,7 +67,7 @@ * Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - { -950, -128 }, + { -750, -128 }, { -748, -94 }, { -745, -92 }, { -735, -90 },@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } }; /* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/@@ -964,6 +964,7 @@Quoted 6 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 967
Quoted 3 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0int val; u32 reg; + *strength = 0;This is not needed, as strength is not initialized only on invalid delivery systems, where
-EINVALis returned.What about the 'if' conditions within the valid delivery system cases? For instance
case SYS_DSS: if (internal->lock) { ... if (STB0899_GETFIELD(VSTATUS_LOCKEDVIT, reg)) { ... *strength = ... } } break;
Quoted 2 lines
So there may be cases where strength has an undefined value, even if this function returns 0.
Good point. I wandering why gcc didn't complain about that.
Quoted 32 lines
Quoted 16 lines
Quoted 16 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-1switch (state->delsys) { case SYS_DVBS: case SYS_DSS:@@ -987,7 +988,7 @@ val = STB0899_GETFIELD(IF_AGC_GAIN, reg); *strength = stb0899_table_lookup(stb0899_dvbs2rf_tab, ARRAY_SIZE(stb0899_dvbs2rf_tab) - 1, val); - *strength += 750; + *strength += 950; dprintk(state->verbose, FE_DEBUG, 1, "IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm", val& 0x3fff, *strength); }@@ -1009,6 +1010,7 @@ u8 buf[2]; u32 reg;Quoted 4 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1013
Quoted 1 line
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +1
-0+ *snr = 0;This is not needed, as strength is not initialized only on invalid delivery systems, where
-EINVALis returned.See above.
Quoted 8 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 line 1016
Quoted 3 lines
linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:23:00.000000000 +0200 renamed from linux/drivers/media/dvb/frontends/stb0899_drv.c 2011-06-11 16:54:32.000000000 +0200 +0
-0reg = stb0899_read_reg(state, STB0899_VSTATUS); switch (state->delsys) { case SYS_DVBS:PS.: Another alternative for it would be the enclosed patch, wich will be a little simpler, and will also preserve the slope on the boundary values, used at the
stb0899_table_lookup()interpolation logic.
Quoted 4 lines
Well, as long as there are no negative values for strength... However, I don't quite see why the range is 750 - 950 for DVB-S and 750 - 999 for DVB-S2. Shouldn't this be the same maximum value in both cases?
This is a very good question. Unfortunately, I don't have the datasheets for this device.
Manu shold be able to answer that better than me.
Based on the table lookup routine, I suspect that what those tables are doing are to interpolate some log curve by breaking it into a few straight line segments. Changing from 999 to 950 will affect such interpolation. So, it is better to check this at the datasheets before changing from -999 to -950, or from -750 to -950.
Manu,
Could you please review it?
Thanks!
Mauro
Em 24-01-2012 07:33, Klaus Schmidinger escreveu:
Quoted 57 lines
stb0899: fix the limits for signal strength values
stb0899_read_signal_strength()adds an offset to the result of the table lookup. That offset must correspond to the lowest value in the lookup table, to make sure the result doesn't get below 0, which would mean a "very high" value since the parameter is unsigned.
'strength' and 'snr' need to be initialized to 0 to make sure they have a defined result in case there is no "internal->lock".Signed-off-by Klaus Schmidinger <[email protected]>linux/drivers/media/dvb/frontends/stb0899_drv.c +5
-3@@ -67,7 +67,7 @@* Crude linear extrapolation below -84.8dBm and above -8.0dBm. */ static const struct stb0899_tab stb0899_dvbsrf_tab[] = { - {-950 ,-128 }, + {-750 ,-128 }, {-748 ,-94 }, {-745 ,-92 }, {-735 ,-90 },@@ -131,7 +131,7 @@ { -730, 13645 }, { -750, 13909 }, { -766, 14153 }, - { -999, 16383 } + { -950, 16383 } };/* DVB-S2 Es/N0 quant in dB/100 vs read value * 100*/ @@ -964,6 +964,7 @@int val;u32 reg; + *strength =0 ;switch (state->delsys) {case SYS_DVBS :case SYS_DSS :@@ -987,7 +988,7 @@ val =STB0899_GETFIELD (IF_AGC_GAIN , reg); *strength =stb0899_table_lookup (stb0899_dvbs2rf_tab,ARRAY_SIZE (stb0899_dvbs2rf_tab) -1 , val); - *strength +=750 ; + *strength +=950 ;dprintk (state->verbose,FE_DEBUG ,1 ,"IF_AGC_GAIN = 0x%04x, C = %d * 0.1 dBm" , val &0x3fff , *strength); }@@ -1009,6 +1010,7 @@u8 buf[2 ];u32 reg; + *snr =0 ; reg =stb0899_read_reg (state,STB0899_VSTATUS );switch (state->delsys) {case SYS_DVBS :