Skip to content

Commit b9de3a4

Browse files
simpkinssimpkins
committed
Skip checking the derivedFrom destination if it is an absolute name
The `_derive` processing code attempts to check that the derived-from register name exists. However, this check currently only handles relative names within the current peripheral or cluster. If the name contains a `.` it is an absolute name, and should be looked up from device root. However, we cannot currently do this as we don't have a reference to the `Device`. Since we are mutating a member of the device in-place, we will not be able to pass in a device reference without significant refactoring of the code. For now this diff simply skips the register name validation if the name contains a `.`. This allows users to refer to absolute names for now, when previously this would always fail. I included a new unit test that makes use of absolute references.
1 parent c285005 commit b9de3a4

File tree

3 files changed

+132
-4
lines changed

3 files changed

+132
-4
lines changed

‎res/cross_cluster_derive/expected.svd

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
<?xml version="1.0" encoding="utf-8" standalone="no"?>
2+
<device schemaVersion="1.1"
3+
xmlns:xs="http://www.w3.org/2001/XMLSchema-instance"
4+
xs:noNamespaceSchemaLocation="CMSIS-SVD_Schema_1_1.xsd">
5+
<name>ESP32S3</name>
6+
<peripherals>
7+
<peripheral>
8+
<name>USB0</name>
9+
<description>USB OTG (On-The-Go)</description>
10+
<groupName>USB</groupName>
11+
<baseAddress>0x60080000</baseAddress>
12+
<addressBlock>
13+
<offset>0x0</offset>
14+
<size>0x2A0</size>
15+
<usage>registers</usage>
16+
</addressBlock>
17+
<registers>
18+
<cluster>
19+
<name>IN_EP0</name>
20+
<description>Device IN Endpoint 0</description>
21+
<addressOffset>0x900</addressOffset>
22+
<register>
23+
<name>DIEPCTL</name>
24+
<addressOffset>0x0</addressOffset>
25+
<size>0x20</size>
26+
<resetValue>0x00008000</resetValue>
27+
<fields>
28+
<field>
29+
<name>MPS</name>
30+
<bitOffset>0</bitOffset>
31+
<bitWidth>2</bitWidth>
32+
<access>read-write</access>
33+
</field>
34+
</fields>
35+
</register>
36+
<register>
37+
<name>DIEPINT</name>
38+
<addressOffset>0x8</addressOffset>
39+
<size>0x20</size>
40+
<fields>
41+
<field>
42+
<name>XFERCOMPL</name>
43+
<bitOffset>0</bitOffset>
44+
<bitWidth>1</bitWidth>
45+
<access>read-write</access>
46+
</field>
47+
</fields>
48+
</register>
49+
</cluster>
50+
<cluster>
51+
<dim>2</dim>
52+
<dimIncrement>0x20</dimIncrement>
53+
<dimIndex>1-2</dimIndex>
54+
<name>IN_EP%s</name>
55+
<description>Device IN Endpoints 1-6</description>
56+
<addressOffset>0x920</addressOffset>
57+
<register>
58+
<name>DIEPCTL</name>
59+
<addressOffset>0x0</addressOffset>
60+
<size>0x20</size>
61+
<resetValue>0x00008000</resetValue>
62+
<fields>
63+
<field>
64+
<name>MPS</name>
65+
<bitOffset>0</bitOffset>
66+
<bitWidth>11</bitWidth>
67+
<access>read-write</access>
68+
</field>
69+
</fields>
70+
</register>
71+
<register derivedFrom="USB0.IN_EP0.DIEPINT">
72+
<name>DIEPINT</name>
73+
<addressOffset>0x8</addressOffset>
74+
</register>
75+
</cluster>
76+
</registers>
77+
</peripheral>
78+
</peripherals>
79+
</device>

‎res/cross_cluster_derive/patch.yaml

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
_svd: ../cluster/esp32s3.svd
2+
3+
USB0:
4+
_cluster:
5+
# Endpoint 0 is different than the other endpoints, and does not have
6+
# quite the same register definitions. We therefore have to cluster it
7+
# by itself.
8+
IN_EP0:
9+
description: "Device IN Endpoint 0"
10+
DIEPCTL0:
11+
name: DIEPCTL
12+
_modify:
13+
D_MPS0:
14+
name: MPS
15+
DIEPINT0:
16+
name: DIEPINT
17+
_modify:
18+
D_XFERCOMPL0:
19+
name: XFERCOMPL
20+
# The remaining endpoints are all the same.
21+
IN_EP%s:
22+
description: "Device IN Endpoints 1-6"
23+
DIEPCTL[1-6]:
24+
name: DIEPCTL
25+
_modify:
26+
D_MPS1:
27+
name: MPS
28+
DIEPINT[1-6]:
29+
name: DIEPINT
30+
_modify:
31+
D_XFERCOMPL1:
32+
name: XFERCOMPL
33+
_derive:
34+
# The DIEPINT registers for endpoints 1-N are the same as endpoint 0,
35+
# so use the same type definition.
36+
DIEPINT: USB0.IN_EP0.DIEPINT

‎src/patch/peripheral.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -255,10 +255,18 @@ pub(crate) trait RegisterBlockExt: Name {
255255
return Err(anyhow!("derive: incorrect syntax for {rname}"));
256256
};
257257

258-
self.get_reg(rderive).ok_or_else(|| {
259-
let present = self.present_registers();
260-
anyhow!("Could not find `{bpath}:{rderive}. Present registers: {present}.")
261-
})?;
258+
// Attempt to verify that the destination register name is correct.
259+
if rderive.contains('.') {
260+
// This is an absolute identifier name
261+
// TODO: at the moment we cannot verify absolute names. We don't have a reference
262+
// to the Device in order to try and look up the name. Since we are mutating a member
263+
// of the device, we cannot get a reference to it.
264+
} else {
265+
self.get_reg(rderive).ok_or_else(|| {
266+
let present = self.present_registers();
267+
anyhow!("Could not find `{bpath}:{rderive}. Present registers: {present}.")
268+
})?;
269+
}
262270

263271
match self.get_mut_reg(rname) {
264272
Some(register) => register.modify_from(info, VAL_LVL)?,
@@ -1509,4 +1517,9 @@ mod tests {
15091517
fn cluster() -> Result<()> {
15101518
test_utils::test_expected(Path::new("cluster"))
15111519
}
1520+
1521+
#[test]
1522+
fn cross_cluster_derive() -> Result<()> {
1523+
test_utils::test_expected(Path::new("cross_cluster_derive"))
1524+
}
15121525
}

0 commit comments

Comments
 (0)