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

runtime/volatile: add GetBits #1239

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

firelizzard18
Copy link
Contributor

Adds another convenience method for registers to read multiple bits at an offset. This change will allow cleaner code:

// instead of
(nxp.MCG.S.Get() & nxp.MCG_S_CLKST_Msk) != (2 << nxp.MCG_S_CLKST_Pos)

// use
nxp.MCG.S.GetBits(nxp.MCG_S_CLKST_11, nxp.MCG_S_CLKST_Pos) != 2

@deadprogram
Copy link
Member

Isn't this mostly the same as https://github.com/tinygo-org/tinygo/blob/dev/src/runtime/volatile/register.go#L56

nxp.MCG.S.HasBits(nxp.MCG_S_CLKST_11 << nxp.MCG_S_CLKST_Pos)

Also, all of the related functions in volatile return a bool.

@firelizzard18
Copy link
Contributor Author

HasBits returns whether any bit is set at the given positions, which is why it returns a boolean, whereas GetBits extracts the value of the specified bits, regardless of whether they are all set or not, which is why it returns a uintN.

MCG.S HasBits GetBits
xxxx00xx false 00
xxxx01xx true 01
xxxx10xx true 10
xxxx11xx true 11
  • nxp.MCG.S.HasBits(nxp.MCG_S_CLKST_11 << nxp.MCG_S_CLKST_Pos) returns true if MCG.S is not xxxx00xx
  • nxp.MCG.S.GetBits(nxp.MCG_S_CLKST_11, nxp.MCG_S_CLKST_Pos) returns AB given MCG.S is xxxxABxx
  • nxp.MCG.S.GetBits(nxp.MCG_S_CLKST_11, nxp.MCG_S_CLKST_Pos) != 2 returns true only if MCG.S is xxxx10xx

@aykevl
Copy link
Member

aykevl commented Apr 14, 2021

I think there is a better way to do this: by treating these struct fields as bitfields (which they are) and auto generating getters/setters for these bitfields in gen-device-svd. At the moment, HasBits is already much too easy to misuse.

Compare this:

// current code
(nxp.MCG.S.Get() & nxp.MCG_S_CLKST_Msk) != (2 << nxp.MCG_S_CLKST_Pos)

// this PR
nxp.MCG.S.GetBits(nxp.MCG_S_CLKST_11, nxp.MCG_S_CLKST_Pos) != 2

// my proposal
nxp.MCG.S.GetCLKST() != 2

This would mean that the struct would look more like this (all autogenerated from the SVD file):

type MCG_Type struct {
    // ... other registers
    S MCG_S_Type
    // ... other registers
}

type MCG_S_Type struct {
    volatile.Register8
}

//go:inline
func (field *MCG_S_Type) GetCLKST() uint8 {
    return (field.Register8.Get() & nxp.MCG_S_CLKST_Msk) >> nxp.MCG_S_CLKST_Pos
}

Other options: adding setters, treating single-bit fields as booleans (returning a bool instead of an integer), ...
Eventually we could hopefully replace and remove HasBits/SetBits/ReplaceBits entirely.

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

Successfully merging this pull request may close these issues.

3 participants