On Thu, Jul 5, 2012 at 10:25 AM, Devin Heitmueller <dheitmueller@xxxxxxxxxxxxxx> wrote: > On Thu, Jul 5, 2012 at 10:16 AM, Mauro Carvalho Chehab >> - /* Use both frq_lock and signal to generate the result */ >> - signal = signal || ((signal & 0x07) << 12); >> + /* Signal level is 3 bits only */ >> + >> + signal = ((1 << 12) - 1) | ((signal & 0x07) << 12); > > Are you sure this is correct? It's entirely possible the original > code used a logical or because the signal level isn't valid unless > there is a lock. The author may have been intending to say: > > if (signal != 0) /* There is a lock, so set the signal level */ > signal = (signal & 0x07) << 12; > else > signal = 0 /* Leave signal level at zero since there is no lock */ > > I agree that the way the original code was written is confusing, but > it may actually be correct. On second reading, it would have needed to be a logical AND, not an OR in order for my suggestion to have been correct. That said, empirical results are definitely a stronger argument in this case. You did test this change in cases with no signal, signal lock with weak signal, and signal lock with strong signal, right? Devin -- Devin J. Heitmueller - Kernel Labs http://www.kernellabs.com -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
