On Mon, 10 Sep 2007 15:13:24 +0200 Rene Herman rene.herman@gmail.com wrote:
On 09/10/2007 02:42 PM, Krzysztof Helt wrote:
On 9/10/07, Rene Herman rene.herman@gmail.com wrote:
Well, no, sorry, but I consider that to be completely breaking the logic of the code.
No I changed the logic of this code not to wait for specifically for callibration "start" but into calibration "under way".
No, waiting for calibration the be under way is what my 0/1 ms does. You are waiting for it to be nearly done, which is complete nonsense. One line below we are waiting for 250 ms (generally with _one_ pass through the loop -- we only wake up through signals) anyway!
I don't buy your argument that it is complete nonsense. I want some merit argument like: your code uses more memory/cpu, your code is longer, your code drives hardware into undefined states, your code push hardware outside hardware limits, your code is slower/adds more delay.
Also, I don't think mimicking the hardware behaviour in driver code is correct paradigm. I think that correct way is to _drive_ the hardware. If the hardware must be taken from state A to state C and goes through state B , the driver may wait only for the C completely ignoring the B if the B is irrelevant (no need to set or get something from the hardware until the C).
Another thing I believe is that driver should be "CPU friendly" means it should gives back control to the CPU every time it knows the hardware is busy and nothing can be done for some known period of time. That's why I don't want to use all these m/udelay functions. They make CPU busy. Sometimes they are must have, sometimes not. If one knows that it must wait at least 7ms why it should wait 1ms than makes another 6ms inside the loop?
Actually, if you look into the lower delay argument, the waiting loop after the change is inefficient. The whole calibration takes 2-3ms at the highest rate, but one has to wait in 10ms blocks. In order to lower this delay one must change it to some finer way of returning time to the CPU (cpu_relax() or schedule_timeout() - like ad1848). If it is done, the loop will make thousands of iterations every few miliseconds. Taking your argument that loop should be passed in the least possible iterations (you wrote "preferably one pass"), the only way is to wait longer before entering the loop.
No, we are arguing maintaining code. Do not obscure the code flow for no reason. Fix your logic or (for what it's worth) I am going to NAK the change.
The code does not be obscured with one simple comment before msleep: /* wait till calibration is nearly done */
Also comments like "my way or highway" (NAK) won't make you many friends.
Krzysztof