Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for RP2040 #14

Open
demotomohiro opened this issue Sep 12, 2024 · 4 comments
Open

Add support for RP2040 #14

demotomohiro opened this issue Sep 12, 2024 · 4 comments

Comments

@demotomohiro
Copy link

I used svd2nim for SVD file for RP2040 in https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2040/hardware_regs/RP2040.svd
But procedures in generated module rp2040.nim doesn't write registers atomitically.

Please read '2.1.2. Atomic Register Access' in RP2040 Datasheet in https://datasheets.raspberrypi.com/rp2040/rp2040-datasheet.pdf
This header file in Raspberry Pi Pico SDK wraps atomic memory accesses:
https://github.com/raspberrypi/pico-sdk/blob/master/src/rp2_common/hardware_base/include/hardware/address_mapped.h

modifyIt templates should not be generated for RP2040 as it seems cannot be atomic.
I think following procs should to be added:

# Common procs for writing RP2040 registers.
# Not exported.
const
  RegAliasXorBits = 0x1000'u32
  RegAliasSetBits = 0x2000'u32
  RegAliasClrBits = 0x3000'u32

proc storeToReg(reg: object; mask: uint32; regAlias: uint32) {.inline.} =
  volatileStore(cast[ptr uint32](cast[uint32](reg.loc) or regAlias), mask)

# You can use setBits/clearBits/xorBits for modifying 1 bit bool fields in a register.
# When you write int or enum fields in the register, use `writeMasked`.
# Because for example, if you want to set '0b10' to the 2bit field of the register, 
# you need to call `setBits` and `clearBits` but that requires write to the register twice.
# Write to some registers have side effect and you might want to write only once.
proc setBits*(reg: object; mask: uint32) {.inline.} =
  storeToReg(reg, mask, RegAliasSetBits)

proc clearBits*(reg: object; mask: uint32) {.inline.} =
  storeToReg(reg, mask, RegAliasClrBits)

proc xorBits*(reg: object; mask: uint32) {.inline.} =
  storeToReg(reg, mask, RegAliasXorBits)

# Set new values for the bits of register.
# You need to use this when you want to set int/enum type field of a register.
# Concurrent write to different bits is safe but same bits is unsafe.
proc writeMasked*(reg: object; values, writeMask: uint32) =
  xorBits(reg, (volatileLoad(cast[ptr uint32](reg.loc)) xor values) and writeMask)
import std/options

type
  SomeRegister_Fields* = distinct uint32

# It seems `set[SomeEnum]` cannot be used for specific size sets.
# https://github.com/nim-lang/Nim/issues/21789
# So use bitwise operators.
const
  SomeRegisterFieldBit0* = 0x1.SomeRegister_Fields
  SomeRegisterFieldBit1* = 0x2.SomeRegister_Fields
  SomeRegisterFieldBit2* = 0x4.SomeRegister_Fields
  SomeRegisterFieldBit3* = 0x8.SomeRegister_Fields
  ...

# Set only bits that is 1 in `val`.
proc setBits*(reg: Register_Type, val: SomeRegister_Fields) =
  setBits(reg, val)

# Clear only bits that is 1 on `val`.
proc clearBits*(reg: Register_Type, val: SomeRegister_Fields) =
  clearBits(reg, val)

# Flip only bits that is 1 on `val`.
proc xorBits*(reg: Register_Type, val: SomeRegister_Fields)
  xorBits(reg, val)

# For int/enum fields.
proc writeMasked*(reg: Register_Type, Field0 = none(uint32), Field1 = none(SomeEnum), Field2 = none(bool)) =
  var x: uint32
  if Field2.isSome:
    x.setMask((Field2 shl 24).masked(24 .. 24))
  if Field1.isSome:
    x.setMask((Field1.uint32 shl 16).masked(16 .. 23))
  if Field0.isSome:
    x.setMask((Field0 shl 0).masked(0 .. 15))
  reg.writeMasked x
@auxym
Copy link
Collaborator

auxym commented Sep 12, 2024

Thanks for reporting. I'll have to look into it in more detail. I'm also not a very deep expert on low-level stuff, so if someone wants to propose a PR, I'm very open.

@dwhall
Copy link
Contributor

dwhall commented Sep 14, 2024

The atomic access this microcontroller offers is a neat feature. But there isn't anything in the SVD 1.3 file spec that reliably serves as an indicator for this capability. So, for now, I recommend that support for atomic access be done outside the svd2nim tool.

Here is a snippet from the cited RP2040.svd file for a register that has atomic access:

            <register>
                <name>GPIO_OUT_SET</name>
                <addressOffset>0x00000014</addressOffset>
                <description>GPIO output value set</description>
                <resetValue>0x00000000</resetValue>
                <fields>
                    <field>
                        <name>GPIO_OUT_SET</name>
                        <description>Perform an atomic bit-set on GPIO_OUT, i.e. `GPIO_OUT |= wdata`</description>
                        <bitRange>[29:0]</bitRange>
                        <access>write-only</access>
                    </field>
                </fields>
            </register>

As you can see, the only mention of "atomic" is in the description. The field access write-only, which is not indicative of atomic access.

@demotomohiro
Copy link
Author

The atomic access this microcontroller offers is a neat feature. But there isn't anything in the SVD 1.3 file spec that reliably serves as an indicator for this capability.

That right. So how about to generate code for RP2040, when svd2nim see that following device name and vendor in the input SVD file?

    <vendor>Raspberry Pi</vendor>
    <name>RP2040</name>
    <series>RP</series>

According to '2.1.2. Atomic Register Access' in RP2040 Datasheet, all registers in RP2040 supports atomic access excepts some registers in SIO.

If it is still not good and should not be merged to svd2nim, I will fork svd2nim and implement the feature.

@auxym
Copy link
Collaborator

auxym commented Sep 17, 2024

I don't mind "special case" codegen for the rp2040. I'd accept a PR.

It might be a bit of PITA with the current (poor) organization of the codegen module in svd2nim. One improvement I planned to make at some point was creating some sort of "CodegenOptions" global singleton in the codegen module that would contain all the info necessary for all the procs in there. It could also contain some sort of "rp2040" flag or device name string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants