Implement Index and Bank fields#274
Conversation
| // behaviour we allow is probably OK. | ||
| // | ||
| // But warn the user, just in case. | ||
| warn!("Data field access width is smaller than normal field width at {:?}", start_pc); |
There was a problem hiding this comment.
I wondered whether to just panic!, but this weird case runs in ACPICA, it just gives odd results. Since the supposed "reference implementation" supports it acpi ought to as well, but it's weird and unlikely enough it's I felt it worth a warning.
There was a problem hiding this comment.
supports permits
| let aligned_offset = object::align_down(field.bit_index + i * access_width_bits, access_width_bits); | ||
| // Advance the read pointer. For Index fields, this also means updating the Index | ||
| // register. | ||
| let aligned_offset = match field.kind { |
There was a problem hiding this comment.
This block isn't ideal - but the alternatives didn't seem great so I stuck with this slightly awkward code.
I considered:
- Separating
do_field_read(ordo_field_write) into specialised normal and index-field variants, but that would be a nuisance to maintain - it'd be easy to fix bugs in one variant and forget to do the other - Perhaps the "advance the read pointer" could have been a closure provided by the previous
matchblock? I thought it just made the code that bit less readable.
Ultimately I don't think these functions are going to be extended much in future, so there was no need to go for a gold-plated solution.
martin-hughes
force-pushed
the
issue-212-no-tests
branch
from
8cd84f8 to
c386b96
Compare
|
The branch name is now out of date - sorry Isaac! I've rebased on top of the now-merged test infra and added the test files to this PR. |
1 participant
Add support for Index and Bank fields.
I've written a selection of tests using the framework in #269, those are not included here (you can see my branch if you want to see them. If the testing stuff gets merged at some point, then I'll make a PR for the tests themselves.
I tried to test on live hardware, but my motherboard has some AML this crate doesn't currently support, so the only testing has been virtual.
If merged, would close #212.