Needs testing on different boards, I've only tested on arm64
Details
Details
- Reviewers
emaste • ian - Group Reviewers
ARM - Commits
- rS274751: The ranges parent bus address may just be a tag to the entry in the parent
Diff Detail
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
If my interpretation of how to traverse up the hierarchy based on being within a mapping range rather than exactly equaling the start of a range is correct, then this whole blob of functionality is kind of insane. It's already kind of insane with the range_id thing anyway, given that the two users of this function hard-code a zero and have no basis to believe that the first tuple of their parent bus is the one with the right mapping in it.
Given that we probably can't wish this functionality away completely -- setting up the console and sometimes other specialized hardware, very early, is just always going to be needed -- we should probably replace all this stuff with something like
fdt_reg_to_physaddr(pcell_t node, int idx, u_long *phys, u_long *size)
where idx is the index into the reg=<...> tuples for the node.
But I'm NOT proposing that we do that instead of committing these fixes. These changes are clearly less broken than the current code.
Oh yeah, I also determined that this function is different than the ranges code I was talking about on irc -- that was the fill-ranges stuff in simplebus.
I gave this a quick test on my beaglebone, it boots and seems to be working fine.
| sys/dev/fdt/fdt_common.c | ||
|---|---|---|
| 115 | This equality test bothers me. It appears to assume that the parent bus address in a child's tuple will appear as the child bus address in one of the parent's tuples. I don't see any reason why that's required, I think it is only required that it fall within one of the child ranges within a parent's tuples. Unfortunately, ePAPR doesn't give any multi-level example where this could happen. For example, is this type of setup not valid?... soc {
ranges = <0x0 0xf0000000 0x10000000>;
bus1 {
ranges = <0x0 0x0 0x200>;
device1 {
reg = <0x0>; // reg is at 0xf0000000
}
}
bus2 {
ranges = <0x0 0x1000 0x400>;
device2 {
reg = <0x0>; // reg is at 0xf0001000;
}
}
} | |
| 180 | If range_id is not zero things will begin to go off the rails here. I assume the intent of range_id N is to get the Nth tuple, not the Nth cell of the property. The only two callers of this function hard-code this as zero. | |
Comment Actions
I discussed this with Ian on IRC yesterday, but for posterity: we should remove this eventually and replace it with a copy of OF_decode_addr() from /sys/powerpc/ofw/ofw_machdep.c, which already handles all the cases and is quasi-standard in the tree.