- User Since
- Jun 2 2014, 4:20 PM (194 w, 6 d)
Sat, Feb 24
With all the comments, this has gotten hard to follow, but from what I am reading it looks good to me.
Fri, Feb 23
OK. This was caused by loading uart int the boot loader and having it in the kernel. Abandoning.
Looks good to me.
Thu, Feb 22
I think this is fine, but what does it do to the minimum sized we can boot on? And does this interfere with where we load the kernel? Is there an overlap? Of is that just a u-boot issue?
You need to do runtime CPU detection, and do it for all the other SoC families.
Wed, Feb 21
daerror should be called with the periph lock held, assert it rather
than take it.
Basically, the project doesn't have the bandwidth to support multiple versions of u-boot, unless there's a demonstrated actual need (not a theoretical need) to do so.
All board use the same version. Any board / SoC family that can will get a one or two release in upstream to get it fixed, then cut loose. We don't have the resources to support something so 'backwater' as to lose upstream u-boot support.
We depend heavily on upstream support, and if that ceases, the solution isn't to freeze that version. The solution is to delete the ports affected and drop support for the board.
To date, we've had no issues since the big switch supporting one version, though the infrastructure allows for one per family if needed.
I want only one version of packages. It greatly increases the support load to deal with old versions of u-boot and we get fewer complaints of breakage if recovery from breakage is easy. Since breakage is rare, we bias to fast discovery rather than bending over backwards to keep the user up and running. The cost/benefit ratio is far too low for this platform to be able to do that absent some compelling, as yet to be articulated, benefit.
In an ideal world, you'd have a richer set of Lua functionality for the ZFS module. However, our current build system make that a bit of a challenge. So for the moment, this is fine.
Should have ticked OK sooner. There's some issues, as noted, but the new bugs introduced have been fixed so they can wait for another day.
Tue, Feb 20
These are fine, but I wonder if it might not be better to have a MD module in the loader that exports attributes of the architecture.... If we use this for more than a few things, we should consider that path instead...
I'm good either way, but if you keep what you have a quick comment wouldn't be terrible.
I'd honestly feel better if we did this in Lua instead.
Just a comment on uint64_t: ps is 10^12, which is 36 bits to represent 1s, so switching to uint64_t will gives the most safety, especially given that the howmany macro isn't careful about overflow. Since it's just used in DELAY, and we know n for delay is usually small except in some crazy old ISA drivers, it likely worked or closely worked with 32-bit types. It wouldn't hit problems until several hundred or maybe thousand milliseconds if the back of the envelop numbers I just ran are right....
I'd be tempted in DELAY to s/u_quad_t/uint64_t/g since the quad name for that type is discouraged, but it's not a bug deal either way.
Quibbles aside, this was a good change before the change to 64-bit, and is a better change with that change as clocks will only get faster from here.
Mon, Feb 19
Hey, I'm out today. Do not commit this until I sign off. I have some worries that I don't have time to articulate. I'll get to it first thing in the morning. I think it might be OK, but it might not, so I want to study it carefully. Thanks.
Sun, Feb 18
Sat, Feb 17
This looks perfect. EDK2 definintions in Include/Protocol/DevicePath.h have it just like this.
My lua extension fu is weak, but this looks decent to me.
This looks really cool!
Fri, Feb 16
Have more to review.
yea, go ahead and commit this. I'd rather we fix lua to generate the right thing, but that's a few days away.
Thu, Feb 15
Wed, Feb 14
Needs some work still... Likely will need another pass to make sure it all lines up.
lgtm, though I must admit to review fatigue
So style(9) requires a blank line at the start of a function with no local variables. I tagged a couple, but there are more.
Tue, Feb 13
this looks fine, apart maybe from the copyright stuff...