WIP: Add more allowed values to TREQ_SEL in rp2040.svd #1063

Closed

jannic wants to merge 2 commits from treq_sel_values into develop

jannic commented

2022-10-16 04:43:26 -05:00

(Migrated from github.com)

Fixes #1062

This change was previously made by @jounathaen in https://github.com/rp-rs/rp2040-pac/pull/59 for the rust PAC. I think it would be useful to add those values in the pico-sdk SVD as well.

Also removes the "(Optional)" annotation from the Timer 2/3 values, as those are always present on the rp2040.

Fixes #1062 This change was previously made by @jounathaen in https://github.com/rp-rs/rp2040-pac/pull/59 for the rust PAC. I think it would be useful to add those values in the pico-sdk SVD as well. Also removes the "(Optional)" annotation from the Timer 2/3 values, as those are always present on the rp2040.

👍 1

lurch commented

2022-10-16 06:11:34 -05:00

(Migrated from github.com)

rp2040.svd is autogenerated (using our internal-only tooling) from other files, so I'm afraid it doesn't make sense to edit it "manually" as it'll only get overwritten again in future. When I'm a bit less busy (which isn't likely to be for quite a while!), I'll look at including your changes here into our internal tooling. Thanks.

P.S. Presumably you'd want https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2040/hardware_regs/include/hardware/regs/dma.h#L160 to be updated with this expanded list of values too? (as that also gets autogenerated using the same tooling)

`rp2040.svd` is autogenerated (using our internal-only tooling) from other files, so I'm afraid it doesn't make sense to edit it "manually" as it'll only get overwritten again in future. When I'm a bit less busy (which isn't likely to be for quite a while!), I'll look at including your changes here into our internal tooling. Thanks. P.S. Presumably you'd want https://github.com/raspberrypi/pico-sdk/blob/develop/src/rp2040/hardware_regs/include/hardware/regs/dma.h#L160 to be updated with this expanded list of values too? (as that also gets autogenerated using the same tooling)

jannic commented

2022-10-16 06:35:41 -05:00

(Migrated from github.com)

Sure, if the svd is auto generated, it's more useful to fix the source files.
I didn't notice that dma.h has the same issue as I'm only using rust, but it makes sense to update it at the same time.

Just for reference, in case you don't already know: The rust files are usually also autogenerated from the svd files, using https://github.com/rust-embedded/svd2rust/.
To avoid manually editing the svd files in case some fix is needed, there is a patch tool available in https://github.com/stm32-rs/svdtools which allows for rather complex edits of the svd file: https://crates.io/crates/svdtools#device-and-peripheral-yaml-format

That's how https://github.com/rp-rs/rp2040-pac can already include those new enumeratedValues while still being based on the original rp2040.svd file.

Sure, if the svd is auto generated, it's more useful to fix the source files. I didn't notice that dma.h has the same issue as I'm only using rust, but it makes sense to update it at the same time. Just for reference, in case you don't already know: The rust files are usually also autogenerated from the svd files, using https://github.com/rust-embedded/svd2rust/. To avoid manually editing the svd files in case some fix is needed, there is a patch tool available in https://github.com/stm32-rs/svdtools which allows for rather complex edits of the svd file: https://crates.io/crates/svdtools#device-and-peripheral-yaml-format That's how https://github.com/rp-rs/rp2040-pac can already include those new enumeratedValues while still being based on the original rp2040.svd file.

👀 1

lurch commented

2022-10-16 06:55:58 -05:00

(Migrated from github.com)

To avoid manually editing the svd files .... which allows for rather complex edits of the svd file

Neat!

https://crates.io/crates/svdtools
"modifying vendor-supplied, often buggy SVD files."

Haha 😄 Please do let us know about any other "bugs" you encounter in our SVD file, and we'll endeavour to fix them when we have the time.

> To avoid manually editing the svd files .... which allows for rather complex edits of the svd file Neat! > https://crates.io/crates/svdtools "modifying vendor-supplied, often buggy SVD files." Haha :smile: Please **do** let us know about any other "bugs" you encounter in our SVD file, and we'll endeavour to fix them when we have the time.

jannic commented

2022-10-16 07:34:19 -05:00

(Migrated from github.com)

"modifying vendor-supplied, often buggy SVD files."

Haha 😄 Please do let us know about any other "bugs" you encounter in our SVD file, and we'll endeavour to fix them when we have the time.

I'm sure that comment was not referring to rp2040.svd! There might be a reason that tool was created by the stm32-rs developers.

While the current patch file https://github.com/rp-rs/rp2040-pac/blob/main/svd/rp2040.yaml is far from small, I think most changes are not due to bugs in rp2040.svd, but just tweaks to help svd2rust to generate nicer rust code.

> > "modifying vendor-supplied, often buggy SVD files." > > Haha :smile: Please **do** let us know about any other "bugs" you encounter in our SVD file, and we'll endeavour to fix them when we have the time. I'm sure that comment was not referring to rp2040.svd! There might be a reason that tool was created by the stm32-rs developers. While the current patch file https://github.com/rp-rs/rp2040-pac/blob/main/svd/rp2040.yaml is far from small, I think most changes are not due to bugs in rp2040.svd, but just tweaks to help svd2rust to generate nicer rust code.

jannic commented

2023-08-01 11:44:45 -05:00

(Migrated from github.com)

What's the state if this pull request? I guess it doesn't make much sense to fix the merge conflicts, given that the svd is generated from some non-public source anyway.

lurch commented

2023-08-01 12:11:45 -05:00

(Migrated from github.com)

We won't be merging this PR (so I've just set it to "draft" status), but it serves as a useful reminder of what changes are being requested 🙂

We won't be merging this PR (so I've just set it to "draft" status), but it serves as a useful reminder of what changes are being requested :slightly_smiling_face:

kilograham commented

2024-08-08 10:26:25 -05:00

(Migrated from github.com)