Skip to content

Commit 461078b

Browse files
Merge #485
485: Expand derived clusters r=burrbull a=smindinvern This PR addresses two issues encountered when attempting to process the svd file [here](https://raw.githubusercontent.com/renesas/fsp/master/ra/fsp/src/bsp/cmsis/Device/RENESAS/SVD/RA.svd). 1. The SVD file defines a cluster in the 'R_GPT_ODC' peripheral that derives from a sibling cluster. AFAICT from the SVD schema, this is allowed, and we should expand the cluster definition in the same way registers are handled. 2. The SVD file contains many `EnumeratedValue`s that are "reserved", but aren't filtered out because they're named "other". They have an `<isDefault>` child element, which is mutually exclusive with the `<value>` child element according to the SVD schema, i.e. they don't represent a value that we would want to define in the generated code, and so can be skipped. When running `svd2rust-regress` locally 3 tests fail, but the same tests also fail on `master`, so it seems there's no indication of a regression here. This seems to fix #462. Co-authored-by: Nickolas Lloyd <smindinvern@users.noreply.github.com>
2 parents 56be787 + c7d6061 commit 461078b

File tree

2 files changed

+65
-30
lines changed

2 files changed

+65
-30
lines changed

src/generate/peripheral.rs

Lines changed: 49 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -87,59 +87,78 @@ pub fn render(
8787

8888
// erc: *E*ither *R*egister or *C*luster
8989
let ercs = p.registers.as_ref().map(|x| x.as_ref()).unwrap_or(&[][..]);
90-
let registers: &[&Register] = &util::only_registers(&ercs)[..];
9190

92-
// make a pass to expand derived registers. Ideally, for the most minimal
91+
// make a pass to expand derived registers and clusters. Ideally, for the most minimal
9392
// code size, we'd do some analysis to figure out if we can 100% reuse the
9493
// code that we're deriving from. For the sake of proving the concept, we're
9594
// just going to emit a second copy of the accessor code. It'll probably
9695
// get inlined by the compiler anyway, right? :-)
9796

9897
// Build a map so that we can look up registers within this peripheral
99-
let mut reg_map = HashMap::new();
100-
for r in registers {
101-
reg_map.insert(&r.name, svd_parser::svd::register::Register::clone(r));
98+
let mut erc_map = HashMap::new();
99+
for erc in ercs {
100+
erc_map.insert(util::erc_name(erc), erc.clone());
102101
}
103102

104-
// Build up an alternate erc list by expanding any derived registers
105-
let mut alt_erc: Vec<RegisterCluster> = registers
103+
// Build up an alternate erc list by expanding any derived registers/clusters
104+
let ercs: Vec<RegisterCluster> = ercs
106105
.iter()
107-
.filter_map(|r| match r.derived_from {
106+
.filter_map(|erc| match util::erc_derived_from(erc) {
108107
Some(ref derived) => {
109-
let ancestor = match reg_map.get(derived) {
110-
Some(r) => r,
108+
let ancestor = match erc_map.get(derived) {
109+
Some(erc) => erc,
111110
None => {
112111
eprintln!(
113-
"register {} derivedFrom missing register {}",
114-
r.name, derived
112+
"register/cluster {} derivedFrom missing register/cluster {}",
113+
util::erc_name(erc),
114+
derived
115115
);
116116
return None;
117117
}
118118
};
119119

120-
match *ancestor {
121-
Register::Array(ref info, ref array_info) => Some(RegisterCluster::Register(
122-
Register::Array(r.derive_from(info), array_info.clone()),
123-
)),
124-
Register::Single(ref info) => Some(RegisterCluster::Register(
125-
Register::Single(r.derive_from(info)),
126-
)),
120+
match (erc, ancestor) {
121+
(RegisterCluster::Register(reg), RegisterCluster::Register(other_reg)) => {
122+
match other_reg {
123+
Register::Array(ref info, ref array_info) => {
124+
Some(RegisterCluster::Register(Register::Array(
125+
reg.derive_from(info),
126+
array_info.clone(),
127+
)))
128+
}
129+
Register::Single(ref info) => Some(RegisterCluster::Register(
130+
Register::Single(reg.derive_from(info)),
131+
)),
132+
}
133+
}
134+
(
135+
RegisterCluster::Cluster(cluster),
136+
RegisterCluster::Cluster(other_cluster),
137+
) => match other_cluster {
138+
Cluster::Array(ref info, ref array_info) => Some(RegisterCluster::Cluster(
139+
Cluster::Array(cluster.derive_from(info), array_info.clone()),
140+
)),
141+
Cluster::Single(ref info) => Some(RegisterCluster::Cluster(
142+
Cluster::Single(cluster.derive_from(info)),
143+
)),
144+
},
145+
_ => {
146+
eprintln!(
147+
"{} can't derive from {}",
148+
util::erc_name(erc),
149+
util::erc_name(ancestor)
150+
);
151+
None
152+
}
127153
}
128154
}
129-
None => Some(RegisterCluster::Register((*r).clone())),
155+
None => Some(erc.clone()),
130156
})
131157
.collect();
132158

133-
// Now add the clusters to our alternate erc list
134-
let clusters = util::only_clusters(ercs);
135-
for cluster in &clusters {
136-
alt_erc.push(RegisterCluster::Cluster((*cluster).clone()));
137-
}
138-
139159
// And revise registers, clusters and ercs to refer to our expanded versions
140-
let registers: &[&Register] = &util::only_registers(&alt_erc)[..];
141-
let clusters = util::only_clusters(ercs);
142-
let ercs = &alt_erc;
160+
let registers: &[&Register] = &util::only_registers(&ercs)[..];
161+
let clusters = util::only_clusters(&ercs);
143162

144163
// No `struct RegisterBlock` can be generated
145164
if registers.is_empty() && clusters.is_empty() {
@@ -151,7 +170,7 @@ pub fn render(
151170

152171
// Push any register or cluster blocks into the output
153172
let mut mod_items = TokenStream::new();
154-
mod_items.extend(register_or_cluster_block(ercs, &defaults, None, nightly)?);
173+
mod_items.extend(register_or_cluster_block(&ercs, &defaults, None, nightly)?);
155174

156175
// Push all cluster related information into the peripheral module
157176
for c in &clusters {

src/util.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,22 @@ impl U32Ext for u32 {
307307
}
308308
}
309309

310+
/// Return the name of either register or cluster.
311+
pub fn erc_name(erc: &RegisterCluster) -> &String {
312+
match erc {
313+
RegisterCluster::Register(r) => &r.name,
314+
RegisterCluster::Cluster(c) => &c.name,
315+
}
316+
}
317+
318+
/// Return the name of either register or cluster from which this register or cluster is derived.
319+
pub fn erc_derived_from(erc: &RegisterCluster) -> &Option<String> {
320+
match erc {
321+
RegisterCluster::Register(r) => &r.derived_from,
322+
RegisterCluster::Cluster(c) => &c.derived_from,
323+
}
324+
}
325+
310326
/// Return only the clusters from the slice of either register or clusters.
311327
pub fn only_clusters(ercs: &[RegisterCluster]) -> Vec<&Cluster> {
312328
let clusters: Vec<&Cluster> = ercs

0 commit comments

Comments
 (0)