On Fri, Jan 23, 2009 at 12:48 PM, Andy Walls <awalls@xxxxxxxxx> wrote: > I will test soon, but I do have two comments by inspection. > > 1. The function s5h1409_softreset() is now called 3 times by > s5h1409_set_frontend(): once at entry, once by > s5h1409_enable_modulation(), and once at exit. Surely at least one of > these is not needed, no? > > 2. You've eliminated the 100 ms "settle delay" after the final > softreset. Can something from userland turn around and call one of the > s5h1409_ops vectors and muck with registers before things "settle"? > Does it even matter? > > I know a hardware reset requires a long-ish assertion time (in fact now > that I see it, I have to fix the cx18 driver hardware reset assertion > delay for this device - the current value isn't right). > > Regards, > Andy Just to be clear, the term "softreset" is somewhat a misnomer. It is *not* a software equivalent of a hardware reset. It does not reset any of the configuration registers. It only resets the internal status data that is used to determine lock status. The 100ms settle delay should not be needed at all. If there is a concern about something in userland mucking with the registers, that is something that would have to be addressed through locking. As far as I know, there is no known issue associated with querying the status registers as soon as the status counters are reset. It is possible that the softreset() before the tune may not be necessary, but at this point my intent was to focus on the minimal change that achieves the desired effect, and the additional softreset() should not cause any performance or reliability impact. Also, the change I made is consistent with the change I made for the s5h1411 back in October (for which there have been no problems reported). Yes, traditionally a hardware reset needs to be asserted for a specific period of time (varies by device). That does not apply here though since the chip itself is not being reset. There is additional room for optimization, but this fix alone provides a significant performance benefit and is low impact, so I wanted to get it merged independent of any other fixes. Devin -- Devin J. Heitmueller http://www.devinheitmueller.com AIM: devinheitmueller -- 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
