Dear Jakub K.,
as you have probably noticed, I have done a preliminary merge of your
LEON3 branch into mainline. My goal was not to make a final
functional
merge, but to review your code and see how it aligns structurally
with
the current state of the mainline.
I suggest you merge the mainline into your branch now to avoid
further diverging of the two branches. Please review my changes to
your code so that we can be sure there are no regressions.
OK. Thank you for your efforts in cleaning my code. :-)
My overall comments:
* Despite the larger changeset my changes were mostly cosmetic
(coding
style unification, removal of obsolete comments, identifiers,
debugging printouts, etc.). My assessment is that your port is in
a
good shape and on a solid path to the finish.
Thank you. As you can see, my main goal was to have code working
correctly
so I left the 'visual' changes to be done at the end. I'm working on
these
now.
* The implementation of several functions is still missing or is
still probably suboptimal, e.g. the atomics, the timer driver (if
I am not mistaken) and less essential functions such as
get_cycle(),
physmem_print(), etc. Another category is memcpy_from_uspace() and
memcpy_to_uspace(), which will work in the current implementation,
but will, of course, not handle the accesses to unmapped memory in
the graceful way as intended.
As the SOCIS coding period ends by the end of the year, what are
your plans with respect to these missing pieces?
I'm planning to add all of these missing functions as well as do other
planned fixes/improvements, even after SOCIS end. I had a few days
off during the Christmas, so I'm slightly behind the schedule.
* There are still some hard-wired constants (e.g. the memory size of
64 MB in leon3_get_memory_extents()).
These are actually needed for QEMU, which apparently does not expose
memory information registers.
* There is probably some uncommitted code laying somewhere. For
example the modifications of the uspace Makefile or the following
file which is referenced but missing in your branch:
kernel/arch/sparc32/src/debug/stacktrace_asm.S
It's possible that I've missed that, I'll check immediately and commit
these.
This might be perhaps also the case of the other missing
functionality that I have mentioned previously, because you have
mentioned that the timer should be already implemented in your
last
report. Is your public branch on LaunchPad really in sync with
your
private branches?
Well... I'm doing commits from tree fragments, so it's possible that
some parts are uncommitted yet. I'll check that and ensure that
launchpad
branch contains all changes.
* The file kernel/arch/sparc32/src/trap_table.S contains probably
the
most fragile code for the whole platform and I acknowledge the
amount of time and effort you spent on it. It is also rather
prone to breakage due to code duplication. I suggest you try to
decrease the physical duplication of the code, e.g. by using more
structured macros.
Significant amount of code is duplicated between trap, syscall and
interrupt handlers. However, common parts cannot be easily folded
to one macro, as there are some subtle differences between them.
I'll try to do the best to reduce duplication.
I would really prefer the approach a) here. Even though you would
probably need to hard-wire the reference to the LEON UART device into
the input and output server at first, this hack will be contained
within the two servers and you can cleverly reuse (and perhaps
generalize) the code for input from S3C24xx UART in
uspace/srv/hid/input/port/chardev.c.
I understand that the input/output handling in HelenOS is still a bit
of a mess, the different platforms and variants use a different
number
of slightly different abstractions. But, in my opinion, introducing a
separate "sercons" won't help you and it won't help with the future
unification either.
OK, so I'll attempt to add support for character devices to
srv/hid/input and then to srv/hid/output (which is less important, as
I have output working now).
Anyway, it seems that HelenOS should have some configuration file
describing which terminal services and consoles should be used,
something
like /etc/ttys in *BSD. What do you think about that?
Regards,
Jakub
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/listinfo/helenos-devel
