From 8a993c0611084f8e38cdc73354bbb6de7567e868 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Mon, 15 Dec 2025 18:43:07 +0000 Subject: [PATCH 1/7] Pass Fdt by value rather than reference. 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. --- src/fdt/mod.rs | 32 ++++++++++++++++---------------- src/fdt/node.rs | 15 ++++++--------- src/fdt/property.rs | 10 +++++----- tests/fdt.rs | 12 ++++++++++++ 4 files changed, 39 insertions(+), 30 deletions(-) diff --git a/src/fdt/mod.rs b/src/fdt/mod.rs index e4c4d62..eab6eee 100644 --- a/src/fdt/mod.rs +++ b/src/fdt/mod.rs @@ -240,7 +240,7 @@ impl<'a> Fdt<'a> { Fdt::new(slice) } - fn validate_header(&self) -> Result<(), FdtParseError> { + fn validate_header(self) -> Result<(), FdtParseError> { let header = self.header(); let data = &self.data; @@ -291,7 +291,7 @@ impl<'a> Fdt<'a> { } /// Returns the header of the device tree. - pub(crate) fn header(&self) -> &FdtHeader { + pub(crate) fn header(self) -> &'a FdtHeader { let (header, _remaining_bytes) = FdtHeader::ref_from_prefix(self.data) .expect("new() checks if the slice is at least as big as the header"); header @@ -299,32 +299,32 @@ impl<'a> Fdt<'a> { /// Returns the underlying data slice of the FDT. #[must_use] - pub fn data(&self) -> &'a [u8] { + pub fn data(self) -> &'a [u8] { self.data } /// Returns the version of the FDT. #[must_use] - pub fn version(&self) -> u32 { + pub fn version(self) -> u32 { self.header().version() } /// Returns the last compatible version of the FDT. #[must_use] - pub fn last_comp_version(&self) -> u32 { + pub fn last_comp_version(self) -> u32 { self.header().last_comp_version() } /// Returns the physical ID of the boot CPU. #[must_use] - pub fn boot_cpuid_phys(&self) -> u32 { + pub fn boot_cpuid_phys(self) -> u32 { self.header().boot_cpuid_phys() } /// Returns an iterator over the memory reservation block. pub fn memory_reservations( - &self, - ) -> impl Iterator> + '_ { + self, + ) -> impl Iterator> + 'a { let mut offset = self.header().off_mem_rsvmap() as usize; core::iter::from_fn(move || { if offset >= self.header().off_dt_struct() as usize { @@ -366,7 +366,7 @@ impl<'a> Fdt<'a> { /// let root = fdt.root().unwrap(); /// assert_eq!(root.name().unwrap(), ""); /// ``` - pub fn root(&self) -> Result, FdtParseError> { + pub fn root(self) -> Result, FdtParseError> { let offset = self.header().off_dt_struct() as usize; let token = self.read_token(offset)?; if token != FdtToken::BeginNode { @@ -424,7 +424,7 @@ impl<'a> Fdt<'a> { /// let node = fdt.find_node("/child2@42").unwrap().unwrap(); /// assert_eq!(node.name().unwrap(), "child2@42"); /// ``` - pub fn find_node(&self, path: &str) -> Result>, FdtParseError> { + pub fn find_node(self, path: &str) -> Result>, FdtParseError> { if !path.starts_with('/') { return Ok(None); } @@ -441,7 +441,7 @@ impl<'a> Fdt<'a> { Ok(Some(current_node)) } - pub(crate) fn read_token(&self, offset: usize) -> Result { + pub(crate) fn read_token(self, offset: usize) -> Result { let val = big_endian::U32::ref_from_prefix(&self.data[offset..]) .map(|(val, _)| val.get()) .map_err(|_e| FdtParseError::new(FdtErrorKind::InvalidLength, offset))?; @@ -449,7 +449,7 @@ impl<'a> Fdt<'a> { } /// Returns a string from the string block. - pub(crate) fn string(&self, string_block_offset: usize) -> Result<&'a str, FdtParseError> { + pub(crate) fn string(self, string_block_offset: usize) -> Result<&'a str, FdtParseError> { let header = self.header(); let str_block_start = header.off_dt_strings() as usize; let str_block_size = header.size_dt_strings() as usize; @@ -465,7 +465,7 @@ impl<'a> Fdt<'a> { /// Returns a NUL-terminated string from a given offset. pub(crate) fn string_at_offset( - &self, + self, offset: usize, end: Option, ) -> Result<&'a str, FdtParseError> { @@ -481,7 +481,7 @@ impl<'a> Fdt<'a> { } } - pub(crate) fn find_string_end(&self, start: usize) -> Result { + pub(crate) fn find_string_end(self, start: usize) -> Result { let mut offset = start; loop { match self.data.get(offset) { @@ -493,7 +493,7 @@ impl<'a> Fdt<'a> { } } - pub(crate) fn next_sibling_offset(&self, mut offset: usize) -> Result { + pub(crate) fn next_sibling_offset(self, mut offset: usize) -> Result { offset += FDT_TAGSIZE; // Skip FDT_BEGIN_NODE // Skip node name @@ -532,7 +532,7 @@ impl<'a> Fdt<'a> { Ok(offset) } - pub(crate) fn next_property_offset(&self, mut offset: usize) -> Result { + pub(crate) fn next_property_offset(self, mut offset: usize) -> Result { let len = big_endian::U32::ref_from_prefix(&self.data[offset..]) .map(|(val, _)| val.get()) .map_err(|_e| FdtParseError::new(FdtErrorKind::InvalidLength, offset))? diff --git a/src/fdt/node.rs b/src/fdt/node.rs index 92f5761..44a0b23 100644 --- a/src/fdt/node.rs +++ b/src/fdt/node.rs @@ -17,7 +17,7 @@ use crate::fdt::property::{FdtPropIter, FdtProperty}; /// A node in a flattened device tree. #[derive(Debug, Clone, Copy)] pub struct FdtNode<'a> { - pub(crate) fdt: &'a Fdt<'a>, + pub(crate) fdt: Fdt<'a>, pub(crate) offset: usize, } @@ -242,8 +242,8 @@ impl<'a> FdtNode<'a> { /// An iterator over the children of a device tree node. enum FdtChildIter<'a> { - Start { fdt: &'a Fdt<'a>, offset: usize }, - Running { fdt: &'a Fdt<'a>, offset: usize }, + Start { fdt: Fdt<'a>, offset: usize }, + Running { fdt: Fdt<'a>, offset: usize }, Error, } @@ -263,10 +263,10 @@ impl<'a> Iterator for FdtChildIter<'a> { } }; offset = Fdt::align_tag_offset(offset); - *self = Self::Running { fdt, offset }; + *self = Self::Running { fdt: *fdt, offset }; self.next() } - Self::Running { fdt, offset } => match Self::try_next(fdt, offset) { + Self::Running { fdt, offset } => match Self::try_next(*fdt, offset) { Some(Ok(val)) => Some(Ok(val)), Some(Err(e)) => { *self = Self::Error; @@ -280,10 +280,7 @@ impl<'a> Iterator for FdtChildIter<'a> { } impl<'a> FdtChildIter<'a> { - fn try_next( - fdt: &'a Fdt<'a>, - offset: &mut usize, - ) -> Option, FdtParseError>> { + fn try_next(fdt: Fdt<'a>, offset: &mut usize) -> Option, FdtParseError>> { loop { let token = match fdt.read_token(*offset) { Ok(token) => token, diff --git a/src/fdt/property.rs b/src/fdt/property.rs index 260c594..fcf2561 100644 --- a/src/fdt/property.rs +++ b/src/fdt/property.rs @@ -200,8 +200,8 @@ impl<'a> FdtProperty<'a> { /// An iterator over the properties of a device tree node. pub(crate) enum FdtPropIter<'a> { - Start { fdt: &'a Fdt<'a>, offset: usize }, - Running { fdt: &'a Fdt<'a>, offset: usize }, + Start { fdt: Fdt<'a>, offset: usize }, + Running { fdt: Fdt<'a>, offset: usize }, Error, } @@ -221,10 +221,10 @@ impl<'a> Iterator for FdtPropIter<'a> { } }; offset = Fdt::align_tag_offset(offset); - *self = Self::Running { fdt, offset }; + *self = Self::Running { fdt: *fdt, offset }; self.next() } - Self::Running { fdt, offset } => match Self::try_next(fdt, offset) { + Self::Running { fdt, offset } => match Self::try_next(*fdt, offset) { Some(Ok(val)) => Some(Ok(val)), Some(Err(e)) => { *self = Self::Error; @@ -239,7 +239,7 @@ impl<'a> Iterator for FdtPropIter<'a> { impl<'a> FdtPropIter<'a> { fn try_next( - fdt: &'a Fdt<'a>, + fdt: Fdt<'a>, offset: &mut usize, ) -> Option, FdtParseError>> { loop { diff --git a/tests/fdt.rs b/tests/fdt.rs index a381de6..6f891ce 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -29,6 +29,18 @@ fn read_child_nodes() { assert!(children.next().is_none()); } +#[test] +fn name_outlives_fdt_and_node() { + let dtb = include_bytes!("dtb/test_children.dtb"); + let name = { + let fdt = Fdt::new(dtb).unwrap(); + let child1 = fdt.find_node("/child1").unwrap().unwrap(); + child1.name().unwrap() + }; + + assert_eq!(name, "child1"); +} + #[test] fn read_prop_values() { let dtb = include_bytes!("dtb/test_props.dtb"); From 28da5517a34ec7294ec2e755aee25195f255552a Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Fri, 12 Dec 2025 17:37:58 +0000 Subject: [PATCH 2/7] Add getter for standard `reg` property. This requires keeping track of the #address-cells and #size-cells of the parent node. --- src/error.rs | 14 +++++ src/fdt/mod.rs | 2 +- src/fdt/node.rs | 67 ++++++++++++++++++----- src/lib.rs | 2 +- src/standard.rs | 30 ++++++++++- src/standard/reg.rs | 114 +++++++++++++++++++++++++++++++++++++++ tests/dtb/test_props.dtb | Bin 517 -> 569 bytes tests/dts/test_props.dts | 3 +- tests/fdt.rs | 35 +++++++++++- 9 files changed, 249 insertions(+), 18 deletions(-) create mode 100644 src/standard/reg.rs diff --git a/src/error.rs b/src/error.rs index e179956..bc8f36d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -35,6 +35,20 @@ pub enum FdtError { /// The number of 4 byte cells expected in each element of the array. chunk: usize, }, + /// Tried to convert the address part of a `reg` property to a type which + /// was too small. + #[error("Reg address too big for chosen type ({cells} cells)")] + AddressTooBig { + /// The value of the relevant `#address-cells` property. + cells: usize, + }, + /// Tried to convert the size part of a `reg` property to a type which was + /// too small. + #[error("Reg size too big for chosen type ({cells} cells)")] + SizeTooBig { + /// The value of the relevant `#size-cells` property. + cells: usize, + }, } /// An error that can occur when parsing a device tree. diff --git a/src/fdt/mod.rs b/src/fdt/mod.rs index eab6eee..195f5c0 100644 --- a/src/fdt/mod.rs +++ b/src/fdt/mod.rs @@ -375,7 +375,7 @@ impl<'a> Fdt<'a> { offset, )); } - Ok(FdtNode { fdt: self, offset }) + Ok(FdtNode::new(self, offset)) } /// Finds a node by its path. diff --git a/src/fdt/node.rs b/src/fdt/node.rs index 44a0b23..b3885a1 100644 --- a/src/fdt/node.rs +++ b/src/fdt/node.rs @@ -13,15 +13,29 @@ use core::fmt; use super::{FDT_TAGSIZE, Fdt, FdtToken}; use crate::error::FdtParseError; use crate::fdt::property::{FdtPropIter, FdtProperty}; +use crate::standard::{DEFAULT_ADDRESS_CELLS, DEFAULT_SIZE_CELLS}; /// A node in a flattened device tree. #[derive(Debug, Clone, Copy)] pub struct FdtNode<'a> { pub(crate) fdt: Fdt<'a>, pub(crate) offset: usize, + /// 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, } impl<'a> FdtNode<'a> { + pub(crate) fn new(fdt: Fdt<'a>, offset: usize) -> Self { + Self { + fdt, + offset, + parent_address_cells: DEFAULT_ADDRESS_CELLS, + parent_size_cells: DEFAULT_SIZE_CELLS, + } + } + /// Returns the name of this node. /// /// # Errors @@ -196,10 +210,7 @@ impl<'a> FdtNode<'a> { /// assert!(children.next().is_none()); /// ``` pub fn children(&self) -> impl Iterator, FdtParseError>> + use<'a> { - FdtChildIter::Start { - fdt: self.fdt, - offset: self.offset, - } + FdtChildIter::Start { node: *self } } pub(crate) fn fmt_recursive(&self, f: &mut fmt::Formatter<'_>, indent: usize) -> fmt::Result { @@ -242,8 +253,15 @@ impl<'a> FdtNode<'a> { /// An iterator over the children of a device tree node. enum FdtChildIter<'a> { - Start { fdt: Fdt<'a>, offset: usize }, - Running { fdt: Fdt<'a>, offset: usize }, + Start { + node: FdtNode<'a>, + }, + Running { + fdt: Fdt<'a>, + offset: usize, + address_cells: u32, + size_cells: u32, + }, Error, } @@ -252,10 +270,18 @@ impl<'a> Iterator for FdtChildIter<'a> { fn next(&mut self) -> Option { match self { - Self::Start { fdt, offset } => { - let mut offset = *offset; + Self::Start { node } => { + let address_cells = match node.address_cells() { + Ok(value) => value, + Err(e) => return Some(Err(e)), + }; + let size_cells = match node.size_cells() { + Ok(value) => value, + Err(e) => return Some(Err(e)), + }; + let mut offset = node.offset; offset += FDT_TAGSIZE; // Skip FDT_BEGIN_NODE - offset = match fdt.find_string_end(offset) { + offset = match node.fdt.find_string_end(offset) { Ok(offset) => offset, Err(e) => { *self = Self::Error; @@ -263,10 +289,20 @@ impl<'a> Iterator for FdtChildIter<'a> { } }; offset = Fdt::align_tag_offset(offset); - *self = Self::Running { fdt: *fdt, offset }; + *self = Self::Running { + fdt: node.fdt, + offset, + address_cells, + size_cells, + }; self.next() } - Self::Running { fdt, offset } => match Self::try_next(*fdt, offset) { + Self::Running { + fdt, + offset, + address_cells, + size_cells, + } => match Self::try_next(*fdt, offset, *address_cells, *size_cells) { Some(Ok(val)) => Some(Ok(val)), Some(Err(e)) => { *self = Self::Error; @@ -280,7 +316,12 @@ impl<'a> Iterator for FdtChildIter<'a> { } impl<'a> FdtChildIter<'a> { - fn try_next(fdt: Fdt<'a>, offset: &mut usize) -> Option, FdtParseError>> { + fn try_next( + fdt: Fdt<'a>, + offset: &mut usize, + parent_address_cells: u32, + parent_size_cells: u32, + ) -> Option, FdtParseError>> { loop { let token = match fdt.read_token(*offset) { Ok(token) => token, @@ -296,6 +337,8 @@ impl<'a> FdtChildIter<'a> { return Some(Ok(FdtNode { fdt, offset: node_offset, + parent_address_cells, + parent_size_cells, })); } FdtToken::Prop => { diff --git a/src/lib.rs b/src/lib.rs index 0547867..c354f69 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ //! println!("{}", fdt); //! ``` -#![no_std] +#![cfg_attr(not(test), no_std)] #![warn(missing_docs, rustdoc::missing_crate_level_docs)] #![deny(unsafe_code)] #![cfg_attr(docsrs, feature(doc_cfg))] diff --git a/src/standard.rs b/src/standard.rs index 2fefa97..182cea6 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -9,15 +9,17 @@ //! Standard nodes and properties. mod memory; +mod reg; mod status; pub use self::memory::{InitialMappedArea, Memory}; +pub use self::reg::Reg; pub use self::status::Status; use crate::error::{FdtError, FdtParseError}; use crate::fdt::FdtNode; -const DEFAULT_ADDRESS_CELLS: u32 = 2; -const DEFAULT_SIZE_CELLS: u32 = 1; +pub(crate) const DEFAULT_ADDRESS_CELLS: u32 = 2; +pub(crate) const DEFAULT_SIZE_CELLS: u32 = 1; impl<'a> FdtNode<'a> { /// Returns the value of the standard `compatible` property. @@ -135,6 +137,30 @@ impl<'a> FdtNode<'a> { }) } + /// Returns the value of the standard `reg` property. + /// + /// # Errors + /// + /// Returns an error if a property's name or value cannot be read, or the + /// size of the value isn't a multiple of the expected number of address and + /// size cells. + pub fn reg(&self) -> Result> + use<'a>>, FdtError> { + let address_cells = self.parent_address_cells; + let size_cells = self.parent_size_cells; + Ok(if let Some(property) = self.property("reg")? { + Some( + property + .as_prop_encoded_array((address_cells + size_cells) as usize)? + .map(move |element| { + let (address, size) = element.split_at(address_cells as usize); + Reg { address, size } + }), + ) + } else { + None + }) + } + /// Returns the value of the standard `virtual-reg` property. /// /// # Errors diff --git a/src/standard/reg.rs b/src/standard/reg.rs new file mode 100644 index 0000000..b669789 --- /dev/null +++ b/src/standard/reg.rs @@ -0,0 +1,114 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use core::fmt::{self, Display, Formatter}; +use core::ops::{BitOr, Shl}; + +use zerocopy::big_endian; + +use crate::error::FdtError; + +/// The value of a `reg` property. +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +pub struct Reg<'a> { + /// The address of the device within the address space of the parent bus. + pub address: &'a [big_endian::U32], + /// The size of the device within the address space of the parent bus. + pub size: &'a [big_endian::U32], +} + +impl Reg<'_> { + /// Attempts to return the address as the given type, if it will fit. + /// + /// # Errors + /// + /// Returns `FdtError::AddressTooBig` if the size doesn't fit in `T`. + pub fn address + Shl + BitOr>( + self, + ) -> Result { + if size_of::() < self.address.len() * size_of::() { + Err(FdtError::AddressTooBig { + cells: self.address.len(), + }) + } else if let [address] = self.address { + Ok(address.get().into()) + } else { + let mut value = Default::default(); + for cell in self.address { + value = (value << 32) | cell.get().into(); + } + Ok(value) + } + } + + /// Attempts to return the size as the given type, if it will fit. + /// + /// # Errors + /// + /// Returns `FdtError::SizeTooBig` if the size doesn't fit in `T`. + pub fn size + Shl + BitOr>( + self, + ) -> Result { + if size_of::() < self.size.len() * size_of::() { + Err(FdtError::SizeTooBig { + cells: self.size.len(), + }) + } else if let [size] = self.size { + Ok(size.get().into()) + } else { + let mut value = Default::default(); + for cell in self.size { + value = value << size_of::() | cell.get().into(); + } + Ok(value) + } + } +} + +impl Display for Reg<'_> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + f.write_str("0x")?; + for part in self.address { + write!(f, "{part:08x}")?; + } + f.write_str(" 0x")?; + for part in self.size { + write!(f, "{part:08x}")?; + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_reg() { + let reg = Reg { + address: &[0x123_45678.into(), 0xabcd_0000.into()], + size: &[0x1122_3344.into()], + }; + assert_eq!(format!("{reg}"), "0x12345678abcd0000 0x11223344"); + } + + #[test] + fn address_size() { + let reg = Reg { + address: &[0x123_45678.into(), 0xabcd_0000.into()], + size: &[0x1122_3344.into()], + }; + assert_eq!( + reg.address::(), + Err(FdtError::AddressTooBig { cells: 2 }) + ); + assert_eq!(reg.address::(), Ok(0x1234_5678_abcd_0000)); + assert_eq!(reg.size::(), Ok(0x1122_3344)); + assert_eq!(reg.size::(), Ok(0x1122_3344)); + } +} diff --git a/tests/dtb/test_props.dtb b/tests/dtb/test_props.dtb index 098e6063553db77dcc94657905aefd11c0f267bb..c935c38df33d28e206150cf765f554a570ac7220 100644 GIT binary patch delta 200 zcmZo=*~y}Df%o5A1_mZe1_lNT1_s6*Kw1Nc1%X%qh(VwcDBdtpV;75qAp^t2N1BET zKtUHFlduW~1_O`+5KsV<4F4b$10#rL24Z0#_LwZdDDNAbmYA6X5(Wa03^x!*1?T6c vD){E7q(Wps;w(U%z`!760;FO53; #size-cells = <0x04>; + reg = <0x12345678 0x3000 0x00 0x20 0x00 0xfe00 0x00 0x100>; compatible = "abc,def", "some,other"; status = "fail"; model = "Some Model"; diff --git a/tests/fdt.rs b/tests/fdt.rs index 6f891ce..85ca33d 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -9,7 +9,7 @@ use dtoolkit::fdt::Fdt; #[cfg(feature = "write")] use dtoolkit::model::DeviceTree; -use dtoolkit::standard::{InitialMappedArea, Status}; +use dtoolkit::standard::{InitialMappedArea, Reg, Status}; #[test] fn read_child_nodes() { @@ -127,6 +127,28 @@ fn standard_properties() { .collect::>(), vec!["abc,def", "some,other"] ); + let reg = standard_props_node + .reg() + .unwrap() + .unwrap() + .collect::>(); + assert_eq!( + reg, + vec![ + Reg { + address: &[0x1234_5678.into(), 0x3000.into()], + size: &[0.into(), 32.into()], + }, + Reg { + address: &[0.into(), 0xfe00.into()], + size: &[0.into(), 256.into()], + }, + ] + ); + assert_eq!(reg[0].address::().unwrap(), 0x1234_5678_0000_3000); + assert_eq!(reg[0].size::().unwrap(), 32); + assert_eq!(reg[1].address::().unwrap(), 0xfe00); + assert_eq!(reg[1].size::().unwrap(), 256); } #[test] @@ -203,6 +225,17 @@ fn memory() { let fdt = Fdt::new(dtb).unwrap(); let memory = fdt.memory().unwrap(); + let reg = memory.reg().unwrap().unwrap().collect::>(); + assert_eq!(reg.len(), 1); + assert_eq!(reg[0].address::().unwrap(), 0x8000_0000); + assert_eq!(reg[0].size::().unwrap(), 0x2000_0000); + assert_eq!( + reg, + vec![Reg { + address: &[0x8000_0000.into()], + size: &[0x2000_0000.into()] + }] + ); assert!(memory.hotpluggable().unwrap()); assert_eq!( memory From bcc67973dd81ea09247735162a1868399fe8d82e Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 17 Dec 2025 12:22:53 +0000 Subject: [PATCH 3/7] Factor out Cells type for Reg. --- src/error.rs | 15 ++++------- src/fdt/mod.rs | 2 +- src/fdt/property.rs | 49 ++++++++++++++++++++++++++++++++-- src/standard.rs | 7 +++-- src/standard/reg.rs | 64 ++++++++++++--------------------------------- tests/fdt.rs | 14 +++++----- 6 files changed, 82 insertions(+), 69 deletions(-) diff --git a/src/error.rs b/src/error.rs index bc8f36d..a2d449f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -35,20 +35,15 @@ pub enum FdtError { /// The number of 4 byte cells expected in each element of the array. chunk: usize, }, - /// Tried to convert the address part of a `reg` property to a type which + /// Tried to convert part of a prop-encoded-array property to a type which /// was too small. - #[error("Reg address too big for chosen type ({cells} cells)")] - AddressTooBig { + #[error("{field} too big for chosen type ({cells} cells)")] + TooManyCells { + /// The name of the field. + field: &'static str, /// The value of the relevant `#address-cells` property. cells: usize, }, - /// Tried to convert the size part of a `reg` property to a type which was - /// too small. - #[error("Reg size too big for chosen type ({cells} cells)")] - SizeTooBig { - /// The value of the relevant `#size-cells` property. - cells: usize, - }, } /// An error that can occur when parsing a device tree. diff --git a/src/fdt/mod.rs b/src/fdt/mod.rs index 195f5c0..0ddaab9 100644 --- a/src/fdt/mod.rs +++ b/src/fdt/mod.rs @@ -26,7 +26,7 @@ use zerocopy::byteorder::big_endian; use zerocopy::{FromBytes, Immutable, IntoBytes, KnownLayout, Unaligned}; pub use self::node::FdtNode; -pub use self::property::FdtProperty; +pub use self::property::{Cells, FdtProperty}; use crate::error::{FdtErrorKind, FdtParseError}; use crate::memreserve::MemoryReservation; diff --git a/src/fdt/property.rs b/src/fdt/property.rs index fcf2561..14cfe62 100644 --- a/src/fdt/property.rs +++ b/src/fdt/property.rs @@ -9,7 +9,8 @@ //! A read-only API for inspecting a device tree property. use core::ffi::CStr; -use core::fmt; +use core::fmt::{self, Display, Formatter}; +use core::ops::{BitOr, Shl}; use zerocopy::{FromBytes, big_endian}; @@ -144,7 +145,7 @@ impl<'a> FdtProperty<'a> { })) } - pub(crate) fn fmt(&self, f: &mut fmt::Formatter<'_>, indent: usize) -> fmt::Result { + pub(crate) fn fmt(&self, f: &mut Formatter, indent: usize) -> fmt::Result { write!(f, "{:indent$}{}", "", self.name, indent = indent)?; if self.value.is_empty() { @@ -308,3 +309,47 @@ impl<'a> Iterator for FdtStringListIterator<'a> { Some(s) } } + +/// 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)] +pub struct Cells<'a>(pub &'a [big_endian::U32]); + +impl Cells<'_> { + /// Converts the value to the given integer type. + /// + /// # Errors + /// + /// Returns `FdtError::TooManyCells` if the value has too many cells to fit + /// in the given type. + pub fn to_intsize + Shl + BitOr>( + self, + field: &'static str, + ) -> Result { + if size_of::() < self.0.len() * size_of::() { + Err(FdtError::TooManyCells { + field, + cells: self.0.len(), + }) + } else if let [size] = self.0 { + Ok(size.get().into()) + } else { + let mut value = Default::default(); + for cell in self.0 { + value = value << 32 | cell.get().into(); + } + Ok(value) + } + } +} + +impl Display for Cells<'_> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + f.write_str("0x")?; + for part in self.0 { + write!(f, "{part:08x}")?; + } + Ok(()) + } +} diff --git a/src/standard.rs b/src/standard.rs index 182cea6..cbcddda 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -16,7 +16,7 @@ pub use self::memory::{InitialMappedArea, Memory}; pub use self::reg::Reg; pub use self::status::Status; use crate::error::{FdtError, FdtParseError}; -use crate::fdt::FdtNode; +use crate::fdt::{Cells, FdtNode}; pub(crate) const DEFAULT_ADDRESS_CELLS: u32 = 2; pub(crate) const DEFAULT_SIZE_CELLS: u32 = 1; @@ -153,7 +153,10 @@ impl<'a> FdtNode<'a> { .as_prop_encoded_array((address_cells + size_cells) as usize)? .map(move |element| { let (address, size) = element.split_at(address_cells as usize); - Reg { address, size } + Reg { + address: Cells(address), + size: Cells(size), + } }), ) } else { diff --git a/src/standard/reg.rs b/src/standard/reg.rs index b669789..a70fd9e 100644 --- a/src/standard/reg.rs +++ b/src/standard/reg.rs @@ -9,17 +9,16 @@ use core::fmt::{self, Display, Formatter}; use core::ops::{BitOr, Shl}; -use zerocopy::big_endian; - use crate::error::FdtError; +use crate::fdt::Cells; /// The value of a `reg` property. #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] pub struct Reg<'a> { /// The address of the device within the address space of the parent bus. - pub address: &'a [big_endian::U32], + pub address: Cells<'a>, /// The size of the device within the address space of the parent bus. - pub size: &'a [big_endian::U32], + pub size: Cells<'a>, } impl Reg<'_> { @@ -27,60 +26,28 @@ impl Reg<'_> { /// /// # Errors /// - /// Returns `FdtError::AddressTooBig` if the size doesn't fit in `T`. + /// Returns `FdtError::TooManyCells` if the address doesn't fit in `T`. pub fn address + Shl + BitOr>( self, ) -> Result { - if size_of::() < self.address.len() * size_of::() { - Err(FdtError::AddressTooBig { - cells: self.address.len(), - }) - } else if let [address] = self.address { - Ok(address.get().into()) - } else { - let mut value = Default::default(); - for cell in self.address { - value = (value << 32) | cell.get().into(); - } - Ok(value) - } + self.address.to_intsize("address") } /// Attempts to return the size as the given type, if it will fit. /// /// # Errors /// - /// Returns `FdtError::SizeTooBig` if the size doesn't fit in `T`. + /// Returns `FdtError::TooManyCells` if the size doesn't fit in `T`. pub fn size + Shl + BitOr>( self, ) -> Result { - if size_of::() < self.size.len() * size_of::() { - Err(FdtError::SizeTooBig { - cells: self.size.len(), - }) - } else if let [size] = self.size { - Ok(size.get().into()) - } else { - let mut value = Default::default(); - for cell in self.size { - value = value << size_of::() | cell.get().into(); - } - Ok(value) - } + self.size.to_intsize("size") } } impl Display for Reg<'_> { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - f.write_str("0x")?; - for part in self.address { - write!(f, "{part:08x}")?; - } - f.write_str(" 0x")?; - for part in self.size { - write!(f, "{part:08x}")?; - } - Ok(()) + write!(f, "{} {}", self.address, self.size) } } @@ -91,21 +58,24 @@ mod tests { #[test] fn format_reg() { let reg = Reg { - address: &[0x123_45678.into(), 0xabcd_0000.into()], - size: &[0x1122_3344.into()], + address: Cells(&[0x123_45678.into(), 0xabcd_0000.into()]), + size: Cells(&[0x1122_3344.into()]), }; - assert_eq!(format!("{reg}"), "0x12345678abcd0000 0x11223344"); + assert_eq!(reg.to_string(), "0x12345678abcd0000 0x11223344"); } #[test] fn address_size() { let reg = Reg { - address: &[0x123_45678.into(), 0xabcd_0000.into()], - size: &[0x1122_3344.into()], + address: Cells(&[0x123_45678.into(), 0xabcd_0000.into()]), + size: Cells(&[0x1122_3344.into()]), }; assert_eq!( reg.address::(), - Err(FdtError::AddressTooBig { cells: 2 }) + Err(FdtError::TooManyCells { + field: "address", + cells: 2 + }) ); assert_eq!(reg.address::(), Ok(0x1234_5678_abcd_0000)); assert_eq!(reg.size::(), Ok(0x1122_3344)); diff --git a/tests/fdt.rs b/tests/fdt.rs index 85ca33d..22a8170 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use dtoolkit::fdt::Fdt; +use dtoolkit::fdt::{Cells, Fdt}; #[cfg(feature = "write")] use dtoolkit::model::DeviceTree; use dtoolkit::standard::{InitialMappedArea, Reg, Status}; @@ -136,12 +136,12 @@ fn standard_properties() { reg, vec![ Reg { - address: &[0x1234_5678.into(), 0x3000.into()], - size: &[0.into(), 32.into()], + address: Cells(&[0x1234_5678.into(), 0x3000.into()]), + size: Cells(&[0.into(), 32.into()]), }, Reg { - address: &[0.into(), 0xfe00.into()], - size: &[0.into(), 256.into()], + address: Cells(&[0.into(), 0xfe00.into()]), + size: Cells(&[0.into(), 256.into()]), }, ] ); @@ -232,8 +232,8 @@ fn memory() { assert_eq!( reg, vec![Reg { - address: &[0x8000_0000.into()], - size: &[0x2000_0000.into()] + address: Cells(&[0x8000_0000.into()]), + size: Cells(&[0x2000_0000.into()]), }] ); assert!(memory.hotpluggable().unwrap()); From 102e34b6acf521ff7a08482daaa810ab58fd98d1 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 17 Dec 2025 12:58:08 +0000 Subject: [PATCH 4/7] Make as_prop_encoded_array return array of Cells. This saves work splitting the result in the caller. --- src/fdt/property.rs | 18 ++++++++++++------ src/standard.rs | 16 +++++----------- src/standard/memory.rs | 29 ++++++++++++++--------------- src/standard/reg.rs | 6 +++++- 4 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/fdt/property.rs b/src/fdt/property.rs index 14cfe62..c100a72 100644 --- a/src/fdt/property.rs +++ b/src/fdt/property.rs @@ -128,10 +128,11 @@ impl<'a> FdtProperty<'a> { FdtStringListIterator { value: self.value } } - pub(crate) fn as_prop_encoded_array( + pub(crate) fn as_prop_encoded_array( &self, - chunk_cells: usize, - ) -> Result + use<'a>, FdtError> { + fields_cells: [usize; N], + ) -> Result; N]> + use<'a, N>, FdtError> { + let chunk_cells = fields_cells.iter().sum(); let chunk_bytes = chunk_cells * size_of::(); if !self.value.len().is_multiple_of(chunk_bytes) { return Err(FdtError::PropEncodedArraySizeMismatch { @@ -139,9 +140,14 @@ impl<'a> FdtProperty<'a> { chunk: chunk_cells, }); } - Ok(self.value.chunks_exact(chunk_bytes).map(|chunk| { - <[big_endian::U32]>::ref_from_bytes(chunk) - .expect("chunk should be a multiple of 4 bytes because of chunks_exact") + Ok(self.value.chunks_exact(chunk_bytes).map(move |chunk| { + let mut cells = <[big_endian::U32]>::ref_from_bytes(chunk) + .expect("chunk should be a multiple of 4 bytes because of chunks_exact"); + fields_cells.map(|field_cells| { + let field; + (field, cells) = cells.split_at(field_cells); + Cells(field) + }) })) } diff --git a/src/standard.rs b/src/standard.rs index cbcddda..124bbd8 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -16,7 +16,7 @@ pub use self::memory::{InitialMappedArea, Memory}; pub use self::reg::Reg; pub use self::status::Status; use crate::error::{FdtError, FdtParseError}; -use crate::fdt::{Cells, FdtNode}; +use crate::fdt::FdtNode; pub(crate) const DEFAULT_ADDRESS_CELLS: u32 = 2; pub(crate) const DEFAULT_SIZE_CELLS: u32 = 1; @@ -145,19 +145,13 @@ impl<'a> FdtNode<'a> { /// size of the value isn't a multiple of the expected number of address and /// size cells. pub fn reg(&self) -> Result> + use<'a>>, FdtError> { - let address_cells = self.parent_address_cells; - let size_cells = self.parent_size_cells; + let address_cells = self.parent_address_cells as usize; + let size_cells = self.parent_size_cells as usize; Ok(if let Some(property) = self.property("reg")? { Some( property - .as_prop_encoded_array((address_cells + size_cells) as usize)? - .map(move |element| { - let (address, size) = element.split_at(address_cells as usize); - Reg { - address: Cells(address), - size: Cells(size), - } - }), + .as_prop_encoded_array([address_cells, size_cells])? + .map(Reg::from_cells), ) } else { None diff --git a/src/standard/memory.rs b/src/standard/memory.rs index 4b83d58..a745074 100644 --- a/src/standard/memory.rs +++ b/src/standard/memory.rs @@ -8,10 +8,8 @@ use core::ops::Deref; -use zerocopy::big_endian; - use crate::error::FdtError; -use crate::fdt::{Fdt, FdtNode}; +use crate::fdt::{Cells, Fdt, FdtNode}; impl Fdt<'_> { /// Returns the /memory node. @@ -56,14 +54,11 @@ impl<'a> Memory<'a> { ) -> Result + use<'a>>, FdtError> { Ok( if let Some(property) = self.node.property("initial-mapped-area")? { - Some(property.as_prop_encoded_array(5)?.map(|chunk| { - InitialMappedArea::from_cells( - #[expect(clippy::missing_panics_doc)] - chunk - .try_into() - .expect("as_prop_encoded_array should return chunks of the size that InitialMappedArea::from_cells expects"), - ) - })) + Some( + property + .as_prop_encoded_array([2, 2, 1])? + .map(|chunk| InitialMappedArea::from_cells(chunk)), + ) } else { None }, @@ -93,11 +88,15 @@ pub struct InitialMappedArea { } impl InitialMappedArea { - 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" + )] + fn from_cells([ea, pa, size]: [Cells; 3]) -> Self { Self { - effective_address: u64::from(ea_high.get()) << 32 | u64::from(ea_low.get()), - physical_address: u64::from(pa_high.get()) << 32 | u64::from(pa_low.get()), - size: size.get(), + effective_address: ea.to_intsize("effective address").unwrap(), + physical_address: pa.to_intsize("physical address").unwrap(), + size: size.to_intsize("size").unwrap(), } } } diff --git a/src/standard/reg.rs b/src/standard/reg.rs index a70fd9e..39c8252 100644 --- a/src/standard/reg.rs +++ b/src/standard/reg.rs @@ -21,7 +21,11 @@ pub struct Reg<'a> { pub size: Cells<'a>, } -impl Reg<'_> { +impl<'a> Reg<'a> { + pub(crate) fn from_cells([address, size]: [Cells<'a>; 2]) -> Self { + Self { address, size } + } + /// Attempts to return the address as the given type, if it will fit. /// /// # Errors From d1c1bd56e25eb81b9b69fb9376e89830e29689d6 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 17 Dec 2025 12:23:12 +0000 Subject: [PATCH 5/7] Add type and getters for standard ranges and dma-ranges properties. --- src/standard.rs | 48 ++++++++++++++++++ src/standard/ranges.rs | 105 +++++++++++++++++++++++++++++++++++++++ tests/dtb/test_props.dtb | Bin 569 -> 643 bytes tests/dts/test_props.dts | 6 ++- tests/fdt.rs | 33 +++++++++++- 5 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 src/standard/ranges.rs diff --git a/src/standard.rs b/src/standard.rs index 124bbd8..523f438 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -9,10 +9,12 @@ //! Standard nodes and properties. mod memory; +mod ranges; mod reg; mod status; pub use self::memory::{InitialMappedArea, Memory}; +pub use self::ranges::Range; pub use self::reg::Reg; pub use self::status::Status; use crate::error::{FdtError, FdtParseError}; @@ -172,6 +174,52 @@ impl<'a> FdtNode<'a> { }) } + /// Returns the value of the standard `ranges` property. + /// + /// # Errors + /// + /// Returns an error if a property's name or value cannot be read, or the + /// size of the value isn't a multiple of the expected number of cells. + pub fn ranges(&self) -> Result> + use<'a>>, FdtError> { + Ok(if let Some(property) = self.property("ranges")? { + Some( + property + .as_prop_encoded_array([ + self.address_cells()? as usize, + self.parent_address_cells as usize, + self.size_cells()? as usize, + ])? + .map(Range::from_cells), + ) + } else { + None + }) + } + + /// Returns the value of the standard `dma-ranges` property. + /// + /// # Errors + /// + /// Returns an error if a property's name or value cannot be read, or the + /// size of the value isn't a multiple of the expected number of cells. + pub fn dma_ranges( + &self, + ) -> Result> + use<'a>>, FdtError> { + Ok(if let Some(property) = self.property("dma-ranges")? { + Some( + property + .as_prop_encoded_array([ + self.address_cells()? as usize, + self.parent_address_cells as usize, + self.size_cells()? as usize, + ])? + .map(Range::from_cells), + ) + } else { + None + }) + } + /// Returns whether the standard `dma-coherent` property is present. /// /// # Errors diff --git a/src/standard/ranges.rs b/src/standard/ranges.rs new file mode 100644 index 0000000..58c231d --- /dev/null +++ b/src/standard/ranges.rs @@ -0,0 +1,105 @@ +// Copyright 2025 Google LLC +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +use core::fmt::{self, Display, Formatter}; +use core::ops::{BitOr, Shl}; + +use crate::error::FdtError; +use crate::fdt::Cells; + +/// One of the values of a `ranges` property. +#[derive(Clone, Debug, Default, Eq, PartialEq)] +pub struct Range<'a> { + /// The address in address space of the child bus. + pub child_bus_address: Cells<'a>, + /// The address in the address space of the parent bus. + pub parent_bus_address: Cells<'a>, + /// The size of the range in the child's address space. + pub length: Cells<'a>, +} + +impl<'a> Range<'a> { + pub(crate) fn from_cells( + [child_bus_address, parent_bus_address, length]: [Cells<'a>; 3], + ) -> Self { + Self { + child_bus_address, + parent_bus_address, + length, + } + } + + /// Attempts to return the child-bus-address as the given type, if it will + /// fit. + /// + /// # Errors + /// + /// Returns `FdtError::TooManyCells` if the child-bus-address doesn't fit in + /// `T`. + pub fn child_bus_address< + T: Default + From + Shl + BitOr, + >( + &self, + ) -> Result { + self.child_bus_address.to_intsize("child-bus-address") + } + + /// Attempts to return the parent-bus-address as the given type, if it will + /// fit. + /// + /// # Errors + /// + /// Returns `FdtError::TooManyCells` if the parent-bus-address doesn't fit + /// in `T`. + pub fn parent_bus_address< + T: Default + From + Shl + BitOr, + >( + &self, + ) -> Result { + self.parent_bus_address.to_intsize("parent-bus-address") + } + + /// Attempts to return the length as the given type, if it will fit. + /// + /// # Errors + /// + /// Returns `FdtError::TooManyCells` if the length doesn't fit in `T`. + pub fn length + Shl + BitOr>( + &self, + ) -> Result { + self.length.to_intsize("length") + } +} + +impl Display for Range<'_> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + write!( + f, + "{} {} {}", + self.child_bus_address, self.parent_bus_address, self.length + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn format_range() { + let range = Range { + child_bus_address: Cells(&[0x0.into(), 0x4000.into()]), + parent_bus_address: Cells(&[0xe000_0000.into()]), + length: Cells(&[0x10_0000.into()]), + }; + assert_eq!( + range.to_string(), + "0x0000000000004000 0xe0000000 0x00100000" + ); + } +} diff --git a/tests/dtb/test_props.dtb b/tests/dtb/test_props.dtb index c935c38df33d28e206150cf765f554a570ac7220..3690dfd4f876328b7b9cf0ca37d5bb28cb76c2d0 100644 GIT binary patch delta 168 zcmdnV(#)!Hf%o5A1_q{P1_lNT1_s6tKw1Nc1%X%qh(Ta7P<+Qkjq`$xKn^nyvj8za z5Hn8ZV4QCYQdS}e)S#rqz+i05z~JHn6oIG&$yGBjtXjpuuzNQH!-N~YHf%o5A1_mZe1_lNT1_s6*Kw1Nc1%X%qh(VwcDBdtp; - #size-cells = <0x04>; + #address-cells = <0x01>; + #size-cells = <0x01>; reg = <0x12345678 0x3000 0x00 0x20 0x00 0xfe00 0x00 0x100>; compatible = "abc,def", "some,other"; status = "fail"; model = "Some Model"; phandle = <0x1234>; virtual-reg = <0xabcd>; + ranges = <0x11110000 0x22220000 0x33330000 0x44440000>; + dma-ranges = <0xaaaa 0xbbbb 0xcccc 0xdddd>; dma-coherent; }; }; diff --git a/tests/fdt.rs b/tests/fdt.rs index 22a8170..8f15a7a 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -109,11 +109,13 @@ fn standard_properties() { assert!(!test_props_node.dma_coherent().unwrap()); assert_eq!(test_props_node.phandle().unwrap(), None); assert_eq!(test_props_node.virtual_reg().unwrap(), None); + assert!(test_props_node.ranges().unwrap().is_none()); + assert!(test_props_node.dma_ranges().unwrap().is_none()); assert!(test_props_node.compatible().unwrap().is_none()); // Explicit values. - assert_eq!(standard_props_node.address_cells().unwrap(), 8); - assert_eq!(standard_props_node.size_cells().unwrap(), 4); + assert_eq!(standard_props_node.address_cells().unwrap(), 1); + assert_eq!(standard_props_node.size_cells().unwrap(), 1); assert_eq!(standard_props_node.status().unwrap(), Status::Fail); assert_eq!(standard_props_node.model().unwrap(), Some("Some Model")); assert!(standard_props_node.dma_coherent().unwrap()); @@ -127,6 +129,7 @@ fn standard_properties() { .collect::>(), vec!["abc,def", "some,other"] ); + let reg = standard_props_node .reg() .unwrap() @@ -149,6 +152,32 @@ fn standard_properties() { assert_eq!(reg[0].size::().unwrap(), 32); assert_eq!(reg[1].address::().unwrap(), 0xfe00); assert_eq!(reg[1].size::().unwrap(), 256); + + let ranges = standard_props_node + .ranges() + .unwrap() + .unwrap() + .collect::>(); + assert_eq!(ranges.len(), 1); + assert_eq!(ranges[0].child_bus_address::().unwrap(), 0x1111_0000); + assert_eq!( + ranges[0].parent_bus_address::().unwrap(), + 0x2222_0000_3333_0000 + ); + assert_eq!(ranges[0].length::().unwrap(), 0x4444_0000); + + let dma_ranges = standard_props_node + .dma_ranges() + .unwrap() + .unwrap() + .collect::>(); + assert_eq!(dma_ranges.len(), 1); + assert_eq!(dma_ranges[0].child_bus_address::().unwrap(), 0xaaaa); + assert_eq!( + dma_ranges[0].parent_bus_address::().unwrap(), + 0xbbbb_0000_cccc + ); + assert_eq!(dma_ranges[0].length::().unwrap(), 0xdddd); } #[test] From a50dcbc74bdfa90a8f115bcee7860f6bd05bf2df Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 17 Dec 2025 14:06:54 +0000 Subject: [PATCH 6/7] Fix test build for Rust 1.88. --- src/standard/ranges.rs | 9 ++++++--- src/standard/reg.rs | 12 ++++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/standard/ranges.rs b/src/standard/ranges.rs index 58c231d..c690447 100644 --- a/src/standard/ranges.rs +++ b/src/standard/ranges.rs @@ -92,10 +92,13 @@ mod tests { #[test] fn format_range() { + let child_bus_address = [0x0.into(), 0x4000.into()]; + let parent_bus_address = [0xe000_0000.into()]; + let length = [0x10_0000.into()]; let range = Range { - child_bus_address: Cells(&[0x0.into(), 0x4000.into()]), - parent_bus_address: Cells(&[0xe000_0000.into()]), - length: Cells(&[0x10_0000.into()]), + child_bus_address: Cells(&child_bus_address), + parent_bus_address: Cells(&parent_bus_address), + length: Cells(&length), }; assert_eq!( range.to_string(), diff --git a/src/standard/reg.rs b/src/standard/reg.rs index 39c8252..2d361b9 100644 --- a/src/standard/reg.rs +++ b/src/standard/reg.rs @@ -61,18 +61,22 @@ mod tests { #[test] fn format_reg() { + let address = [0x123_45678.into(), 0xabcd_0000.into()]; + let size = [0x1122_3344.into()]; let reg = Reg { - address: Cells(&[0x123_45678.into(), 0xabcd_0000.into()]), - size: Cells(&[0x1122_3344.into()]), + address: Cells(&address), + size: Cells(&size), }; assert_eq!(reg.to_string(), "0x12345678abcd0000 0x11223344"); } #[test] fn address_size() { + let address = [0x123_45678.into(), 0xabcd_0000.into()]; + let size = [0x1122_3344.into()]; let reg = Reg { - address: Cells(&[0x123_45678.into(), 0xabcd_0000.into()]), - size: Cells(&[0x1122_3344.into()]), + address: Cells(&address), + size: Cells(&size), }; assert_eq!( reg.address::(), From 1def694b7b3b606ee80263e675dac866bb0051d8 Mon Sep 17 00:00:00 2001 From: Andrew Walbran Date: Wed, 17 Dec 2025 14:41:56 +0000 Subject: [PATCH 7/7] Fixes from code review. --- src/error.rs | 6 ++---- src/fdt/node.rs | 35 ++++++++++++----------------------- src/fdt/property.rs | 8 +++----- src/standard.rs | 40 ++++++++++++++++++++++++++++++++++++---- src/standard/memory.rs | 11 ++++++++--- src/standard/ranges.rs | 6 +++--- src/standard/reg.rs | 9 +++------ tests/fdt.rs | 25 +++---------------------- 8 files changed, 70 insertions(+), 70 deletions(-) diff --git a/src/error.rs b/src/error.rs index a2d449f..0744fd5 100644 --- a/src/error.rs +++ b/src/error.rs @@ -37,11 +37,9 @@ pub enum FdtError { }, /// Tried to convert part of a prop-encoded-array property to a type which /// was too small. - #[error("{field} too big for chosen type ({cells} cells)")] + #[error("prop-encoded-array field too big for chosen type ({cells} cells)")] TooManyCells { - /// The name of the field. - field: &'static str, - /// The value of the relevant `#address-cells` property. + /// The number of (32-bit) cells in the field. cells: usize, }, } diff --git a/src/fdt/node.rs b/src/fdt/node.rs index b3885a1..f3f33fd 100644 --- a/src/fdt/node.rs +++ b/src/fdt/node.rs @@ -13,17 +13,16 @@ use core::fmt; use super::{FDT_TAGSIZE, Fdt, FdtToken}; use crate::error::FdtParseError; use crate::fdt::property::{FdtPropIter, FdtProperty}; -use crate::standard::{DEFAULT_ADDRESS_CELLS, DEFAULT_SIZE_CELLS}; +use crate::standard::AddressSpaceProperties; /// A node in a flattened device tree. #[derive(Debug, Clone, Copy)] pub struct FdtNode<'a> { pub(crate) fdt: Fdt<'a>, pub(crate) offset: usize, - /// 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, + /// The `#address-cells` and `#size-cells` properties of this node's parent + /// node. + pub(crate) parent_address_space: AddressSpaceProperties, } impl<'a> FdtNode<'a> { @@ -31,8 +30,7 @@ impl<'a> FdtNode<'a> { Self { fdt, offset, - parent_address_cells: DEFAULT_ADDRESS_CELLS, - parent_size_cells: DEFAULT_SIZE_CELLS, + parent_address_space: AddressSpaceProperties::default(), } } @@ -259,8 +257,7 @@ enum FdtChildIter<'a> { Running { fdt: Fdt<'a>, offset: usize, - address_cells: u32, - size_cells: u32, + address_space: AddressSpaceProperties, }, Error, } @@ -271,11 +268,7 @@ impl<'a> Iterator for FdtChildIter<'a> { fn next(&mut self) -> Option { match self { Self::Start { node } => { - let address_cells = match node.address_cells() { - Ok(value) => value, - Err(e) => return Some(Err(e)), - }; - let size_cells = match node.size_cells() { + let address_space = match node.address_space() { Ok(value) => value, Err(e) => return Some(Err(e)), }; @@ -292,17 +285,15 @@ impl<'a> Iterator for FdtChildIter<'a> { *self = Self::Running { fdt: node.fdt, offset, - address_cells, - size_cells, + address_space, }; self.next() } Self::Running { fdt, offset, - address_cells, - size_cells, - } => match Self::try_next(*fdt, offset, *address_cells, *size_cells) { + address_space, + } => match Self::try_next(*fdt, offset, *address_space) { Some(Ok(val)) => Some(Ok(val)), Some(Err(e)) => { *self = Self::Error; @@ -319,8 +310,7 @@ impl<'a> FdtChildIter<'a> { fn try_next( fdt: Fdt<'a>, offset: &mut usize, - parent_address_cells: u32, - parent_size_cells: u32, + parent_address_space: AddressSpaceProperties, ) -> Option, FdtParseError>> { loop { let token = match fdt.read_token(*offset) { @@ -337,8 +327,7 @@ impl<'a> FdtChildIter<'a> { return Some(Ok(FdtNode { fdt, offset: node_offset, - parent_address_cells, - parent_size_cells, + parent_address_space, })); } FdtToken::Prop => { diff --git a/src/fdt/property.rs b/src/fdt/property.rs index c100a72..ce22a76 100644 --- a/src/fdt/property.rs +++ b/src/fdt/property.rs @@ -319,8 +319,8 @@ impl<'a> Iterator for FdtStringListIterator<'a> { /// 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)] -pub struct Cells<'a>(pub &'a [big_endian::U32]); +#[derive(Clone, Copy, Debug, Default, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub struct Cells<'a>(pub(crate) &'a [big_endian::U32]); impl Cells<'_> { /// Converts the value to the given integer type. @@ -329,13 +329,11 @@ impl Cells<'_> { /// /// Returns `FdtError::TooManyCells` if the value has too many cells to fit /// in the given type. - pub fn to_intsize + Shl + BitOr>( + pub fn to_int + Shl + BitOr>( self, - field: &'static str, ) -> Result { if size_of::() < self.0.len() * size_of::() { Err(FdtError::TooManyCells { - field, cells: self.0.len(), }) } else if let [size] = self.0 { diff --git a/src/standard.rs b/src/standard.rs index 523f438..4d0b5b2 100644 --- a/src/standard.rs +++ b/src/standard.rs @@ -139,6 +139,20 @@ impl<'a> FdtNode<'a> { }) } + /// Returns the values of the standard `#address-cells` and `#size_cells` + /// properties. + /// + /// # Errors + /// + /// Returns an error if a property's name or value cannot be read, or the + /// values aren't valid u32s. + pub fn address_space(&self) -> Result { + Ok(AddressSpaceProperties { + address_cells: self.address_cells()?, + size_cells: self.size_cells()?, + }) + } + /// Returns the value of the standard `reg` property. /// /// # Errors @@ -147,8 +161,8 @@ impl<'a> FdtNode<'a> { /// size of the value isn't a multiple of the expected number of address and /// size cells. pub fn reg(&self) -> Result> + use<'a>>, FdtError> { - let address_cells = self.parent_address_cells as usize; - let size_cells = self.parent_size_cells as usize; + let address_cells = self.parent_address_space.address_cells as usize; + let size_cells = self.parent_address_space.size_cells as usize; Ok(if let Some(property) = self.property("reg")? { Some( property @@ -186,7 +200,7 @@ impl<'a> FdtNode<'a> { property .as_prop_encoded_array([ self.address_cells()? as usize, - self.parent_address_cells as usize, + self.parent_address_space.address_cells as usize, self.size_cells()? as usize, ])? .map(Range::from_cells), @@ -210,7 +224,7 @@ impl<'a> FdtNode<'a> { property .as_prop_encoded_array([ self.address_cells()? as usize, - self.parent_address_cells as usize, + self.parent_address_space.address_cells as usize, self.size_cells()? as usize, ])? .map(Range::from_cells), @@ -229,3 +243,21 @@ impl<'a> FdtNode<'a> { Ok(self.property("dma-coherent")?.is_some()) } } + +/// The `#address-cells` and `#size-cells` properties of a node. +#[derive(Debug, Clone, Copy)] +pub struct AddressSpaceProperties { + /// The `#address-cells` property. + pub address_cells: u32, + /// The `#size-cells` property. + pub size_cells: u32, +} + +impl Default for AddressSpaceProperties { + fn default() -> Self { + Self { + address_cells: DEFAULT_ADDRESS_CELLS, + size_cells: DEFAULT_SIZE_CELLS, + } + } +} diff --git a/src/standard/memory.rs b/src/standard/memory.rs index a745074..49d150e 100644 --- a/src/standard/memory.rs +++ b/src/standard/memory.rs @@ -88,15 +88,20 @@ pub struct InitialMappedArea { } impl InitialMappedArea { + /// Creates an `InitialMappedArea` from an array of three `Cells` containing + /// the effective address, physical address and size respectively. + /// + /// These `Cells` must contain 2, 2 and 1 cells respectively, or the method + /// will panic. #[expect( clippy::unwrap_used, reason = "The Cells passed are always the correct size" )] fn from_cells([ea, pa, size]: [Cells; 3]) -> Self { Self { - effective_address: ea.to_intsize("effective address").unwrap(), - physical_address: pa.to_intsize("physical address").unwrap(), - size: size.to_intsize("size").unwrap(), + effective_address: ea.to_int().unwrap(), + physical_address: pa.to_int().unwrap(), + size: size.to_int().unwrap(), } } } diff --git a/src/standard/ranges.rs b/src/standard/ranges.rs index c690447..14b000b 100644 --- a/src/standard/ranges.rs +++ b/src/standard/ranges.rs @@ -46,7 +46,7 @@ impl<'a> Range<'a> { >( &self, ) -> Result { - self.child_bus_address.to_intsize("child-bus-address") + self.child_bus_address.to_int() } /// Attempts to return the parent-bus-address as the given type, if it will @@ -61,7 +61,7 @@ impl<'a> Range<'a> { >( &self, ) -> Result { - self.parent_bus_address.to_intsize("parent-bus-address") + self.parent_bus_address.to_int() } /// Attempts to return the length as the given type, if it will fit. @@ -72,7 +72,7 @@ impl<'a> Range<'a> { pub fn length + Shl + BitOr>( &self, ) -> Result { - self.length.to_intsize("length") + self.length.to_int() } } diff --git a/src/standard/reg.rs b/src/standard/reg.rs index 2d361b9..f0bf387 100644 --- a/src/standard/reg.rs +++ b/src/standard/reg.rs @@ -34,7 +34,7 @@ impl<'a> Reg<'a> { pub fn address + Shl + BitOr>( self, ) -> Result { - self.address.to_intsize("address") + self.address.to_int() } /// Attempts to return the size as the given type, if it will fit. @@ -45,7 +45,7 @@ impl<'a> Reg<'a> { pub fn size + Shl + BitOr>( self, ) -> Result { - self.size.to_intsize("size") + self.size.to_int() } } @@ -80,10 +80,7 @@ mod tests { }; assert_eq!( reg.address::(), - Err(FdtError::TooManyCells { - field: "address", - cells: 2 - }) + Err(FdtError::TooManyCells { cells: 2 }) ); assert_eq!(reg.address::(), Ok(0x1234_5678_abcd_0000)); assert_eq!(reg.size::(), Ok(0x1122_3344)); diff --git a/tests/fdt.rs b/tests/fdt.rs index 8f15a7a..1c29193 100644 --- a/tests/fdt.rs +++ b/tests/fdt.rs @@ -6,10 +6,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use dtoolkit::fdt::{Cells, Fdt}; +use dtoolkit::fdt::Fdt; #[cfg(feature = "write")] use dtoolkit::model::DeviceTree; -use dtoolkit::standard::{InitialMappedArea, Reg, Status}; +use dtoolkit::standard::{InitialMappedArea, Status}; #[test] fn read_child_nodes() { @@ -135,19 +135,7 @@ fn standard_properties() { .unwrap() .unwrap() .collect::>(); - assert_eq!( - reg, - vec![ - Reg { - address: Cells(&[0x1234_5678.into(), 0x3000.into()]), - size: Cells(&[0.into(), 32.into()]), - }, - Reg { - address: Cells(&[0.into(), 0xfe00.into()]), - size: Cells(&[0.into(), 256.into()]), - }, - ] - ); + assert_eq!(reg.len(), 2); assert_eq!(reg[0].address::().unwrap(), 0x1234_5678_0000_3000); assert_eq!(reg[0].size::().unwrap(), 32); assert_eq!(reg[1].address::().unwrap(), 0xfe00); @@ -258,13 +246,6 @@ fn memory() { assert_eq!(reg.len(), 1); assert_eq!(reg[0].address::().unwrap(), 0x8000_0000); assert_eq!(reg[0].size::().unwrap(), 0x2000_0000); - assert_eq!( - reg, - vec![Reg { - address: Cells(&[0x8000_0000.into()]), - size: Cells(&[0x2000_0000.into()]), - }] - ); assert!(memory.hotpluggable().unwrap()); assert_eq!( memory