[U-Boot] CONFIG_SYS_HZ: Please revert breaking net commit
Hello,
From CHANGELOG: <quote> commit 49f3bdbba8071f56d950a9498b6cdb998b35340a Author: Bartlomiej Sieka tur@semihalf.com Date: Wed Oct 1 15:26:28 2008 +0200
net: express the first argument to NetSetTimeout() in milliseconds
Enforce millisecond semantics of the first argument to NetSetTimeout() -- the change is transparent for well-behaving boards (CFG_HZ == 1000 and get_timer() countiing in milliseconds).
Rationale for this patch is to enable millisecond granularity for network-related timeouts, which is needed for the upcoming automatic software update feature.
Summary of changes: - do not scale the first argument to NetSetTimeout() by CFG_HZ - change timeout values used in the networking code to milliseconds
Signed-off-by: Rafal Czubak rcz@semihalf.com Signed-off-by: Bartlomiej Sieka tur@semihalf.com Signed-off-by: Ben Warren biggerbadderben@gmail.com </quote>
This commit breaks *silently* all boards that don't return milliseconds from get_timer, i.e. have a different value for CONFIG_SYS_HZ than 1000. I'm open to any discussion about removing CONFIG_SYS_HZ, but if it is removed, it should be removed explicitely, and not just silently ignored.
So I see following reasons to revert the above commit: - silently breaks existing boards - silently ignores a still valid CONFIG setting - bad programming style anyway: even if CONFIG_SYS_HZ would go away, there should be some internal define that should be used in code - the intended benefit 'millisecond granularity for network-related timeouts' can be achieved while still keeping CONFIG_SYS_HZ
Best Regards, Detlef
Dear Detlef Vollmann,
In message 49A44806.9050902@vollmann.ch you wrote:
commit 49f3bdbba8071f56d950a9498b6cdb998b35340a
...
This commit breaks *silently* all boards that don't return milliseconds from get_timer, i.e. have a different value for CONFIG_SYS_HZ than 1000.
No, this is not true. All these boards ARE already broken because they use anincorrect timer setup. The patch just makes the problem visible.
I'm open to any discussion about removing CONFIG_SYS_HZ, but if it is removed, it should be removed explicitely, and not just silently ignored.
There is no need in the code to use CONFIG_SYS_HZ, because the code just used get_timer() as specified, i. e. it expects correct behaviour. If your implementation is broken, you should fix your implementation.
BTW: how comes it takes you nearly half a year to detect this?
So I see following reasons to revert the above commit:
- silently breaks existing boards
These boards were already broken before.
- silently ignores a still valid CONFIG setting
It does not ignore it. It does not need it, because it is specified that get_timer() uses milisecond resolution.
- bad programming style anyway: even if CONFIG_SYS_HZ would go away, there should be some internal define that should be used in code
I don't understand what you mean.
- the intended benefit 'millisecond granularity for network-related timeouts' can be achieved while still keeping CONFIG_SYS_HZ
Instead of complaining about a patch that was discussed and cehcekd in many months ago you could rather spend your time on fixing your broken boards.
Best regards,
Wolfgang Denk
participants (2)
-
Detlef Vollmann -
Wolfgang Denk