Add getters and types for standard reg, ranges and dma-ranges properties#19
Conversation
qwandor
force-pushed
the
typedaccessors
branch
from
27e6bc8 to
5522b25
Compare
qwandor
force-pushed
the
typedaccessors
branch
3 times, most recently
from
b62159d to
033efbb
Compare
qwandor
force-pushed
the
typedaccessors
branch
from
033efbb to
5a568a0
Compare
qwandor
changed the title
reg propertyreg, ranges and dma-ranges properties
| /// An integer value split into several big-endian u32 parts. | ||
| /// | ||
| /// This is generally used in prop-encoded-array properties. | ||
| #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] |
There was a problem hiding this comment.
| #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] | |
| #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, Hash)] |
| /// The `#size-cells` property of this node's parent node. | ||
| pub(crate) parent_size_cells: u32, | ||
| /// The `#address-cells` property of this node's parent node. | ||
| pub(crate) parent_address_cells: u32, |
There was a problem hiding this comment.
We seem to duplicate this here and in FdtChildIter. Maybe we could define a separate structure called something like ParentMetadata (or more specifically, "BusAddressSpaceParams"), and define Default trait impl as well as from_node() associated function to encapsulate the logic?
| /// | ||
| /// This is generally used in prop-encoded-array properties. | ||
| #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] | ||
| pub struct Cells<'a>(pub &'a [big_endian::U32]); |
There was a problem hiding this comment.
We expose a type from another library (zerocopy) here, which really is just an implementation detail. This means that each new major version of this library will also mean a new major version for us.
I don't think there's a strong reason to expose this, especially since each sensible use case will probably also require map(|elem| elem.get()) on this slice - can we hide this type somehow? Perhaps by only exposing an ExactSizedIterator over u32, or maybe not exposing this data at all.
Also, this is probably not actionable at this moment, but we'll need a mutable version of this so that we can construct standard properties easily. It might be worth thinking about how should it look like - maybe similarly to Cow with an "owned" or "borrowed" variant. But again, this can be left for future PRs; I'm just leaving this here as a food for thought.
There was a problem hiding this comment.
Major versions of zerocopy are hopefully rare, but fair enough. The main downside is that it prevents anyone outside the crate from constructing an instance of this (e.g. for tests), but maybe that is fine.
| fn from_cells([ea_high, ea_low, pa_high, pa_low, size]: [big_endian::U32; 5]) -> Self { | ||
| #[expect( | ||
| clippy::unwrap_used, | ||
| reason = "The Cells passed are always the correct size" |
There was a problem hiding this comment.
Let's document here what are the correct sizes ([2, 2, 1]).
| /// in the given type. | ||
| pub fn to_intsize<T: Default + From<u32> + Shl<usize, Output = T> + BitOr<Output = T>>( | ||
| self, | ||
| field: &'static str, |
There was a problem hiding this comment.
I don't exactly like that this is public API and this parameter is only used for error handling - this function doesn't actually care at all what's the field name. It sounds like a piece of metadata that should be added at a higher level. Perhaps this could return its own error struct that would be then wrapped and combined with the field name by the caller?
There was a problem hiding this comment.
I've removed the field name from the error variant, it's probably clear from context anyway.
qwandor
force-pushed
the
typedaccessors
branch
from
7966924 to
3e6341d
Compare
It is Copy and only contains a single reference, so there's no point passing it by reference. This also means that the lifetimes of things returned from getters are only tied to the underlying slice, not the Fdt itself, which may be more convenient.
This requires keeping track of the #address-cells and #size-cells of the parent node.
This saves work splitting the result in the caller.
qwandor
force-pushed
the
typedaccessors
branch
from
676a613 to
1def694
Compare
2 participants
I've also fixed lifetimes of values returned by various methods not to be tied unnecessarily to the lifetime of the
FdtorFdtNodeitself, changed methods onCopytypes to takeselfby value rather than reference, and improved theFdtProperty::as_prop_encoded_arrayhelper method to do more of the work.