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

attiny_hal::pac::exint::pcmsk::W missing BitWriterRaw implementation? #130

Closed
mgrunwald opened this issue May 23, 2023 · 8 comments · Fixed by #131
Closed

attiny_hal::pac::exint::pcmsk::W missing BitWriterRaw implementation? #130

mgrunwald opened this issue May 23, 2023 · 8 comments · Fixed by #131

Comments

@mgrunwald
Copy link

Hello,

I'm only a rookie, but this seems strange for me: When I want to change the PCMSK register, there is no way to access single bits, like there is e.g. for the GIMSK:

            self.exint.gimsk.modify(|_, w| w.pcie().set_bit()); // Good
            self.exint.pcmsk.modify(|_, w| w.bits(1)); // Not so good

There aren't even constants for the single bits…

I'd expect to be able to do this:

            self.exint.pcmsk.modify(|_, w| w.pcint0().set_bit());

Am I right?

@Rahix Rahix transferred this issue from Rahix/avr-hal Jun 1, 2023
@Rahix
Copy link
Owner

Rahix commented Jun 1, 2023

Transferred this issue to the avr-device project...

It seems the register information is incorrect here. The way to fix is this adding a YAML patch to correct it. For example, here we have a patch which sets a field read-only:

PLL:
PLLCSR:
_modify:
PLOCK:
access: read-only

we'd need a similar patch to set this field to read-write in the patch file for the MCU you are using. Can you try sending a pull request for this? If you need more guidance, just let me know!

@mgrunwald
Copy link
Author

Can you try sending a pull request for this? If you need more guidance, just let me know!

Thanks, @Rahix! I'll have a look and see what I can do.

@mgrunwald
Copy link
Author

@Rahix I don't think that it's a read/write issue, but that the fields are missing. Compare with GIMSK from attiny85.svd:

        <register>
          <name>GIMSK</name>
          <description>General Interrupt Mask Register</description>
          <addressOffset>0x26</addressOffset>
          <size>0x8</size>
          <access>read-write</access>
          <fields>
            <field>
              <name>PCIE</name>
              <description>Pin Change Interrupt Enable</description>
              <bitRange>[5:5]</bitRange>
              <access>read-write</access>
            </field>
            <field>
              <name>INT0</name>
              <description>External Interrupt Request 0 Enable</description>
              <bitRange>[6:6]</bitRange>
              <access>read-write</access>
            </field>
          </fields>
        </register>

PCMSK has no fields:

        <register>
          <name>PCMSK</name>
          <description>Pin Change Enable Mask</description>
          <addressOffset>0x0</addressOffset>
          <size>0x8</size>
          <access>read-write</access>
          <writeConstraint>
            <range>
              <minimum>0</minimum>
              <maximum>255</maximum>
            </range>
          </writeConstraint>
        </register>

So I tried to patch PCMSK by adding this to the end of patch/attiny85.yaml:

  PCMSK:
    _add:
      PCINT0:
        description: Enable pin change interrupt on pin 0
        bitOffset: 0
        bitWidth: 1
        access: read-write
      PCINT1:
        description: Enable pin change interrupt on pin 1
        bitOffset: 1
        bitWidth: 1
        access: read-write
      PCINT2:
        description: Enable pin change interrupt on pin 2
        bitOffset: 2
        bitWidth: 1
        access: read-write
      PCINT3:
        description: Enable pin change interrupt on pin 3
        bitOffset: 3
        bitWidth: 1
        access: read-write
      PCINT4:
        description: Enable pin change interrupt on pin 4
        bitOffset: 4
        bitWidth: 1
        access: read-write
      PCINT5:
        description: Enable pin change interrupt on pin 5
        bitOffset: 5
        bitWidth: 1
        access: read-write

Unfortunately that doesn't work and I don't get why:

  File "/home/markus/.virtualenvs/avr-device/lib/python3.11/site-packages/svdtools/patch.py", line 1229, in process_register
    raise MissingRegisterError(f"Could not find {pname}:{rspec}")
svdtools.patch.MissingRegisterError: Could not find CPU:PCMSK

I can modify PLLCSR from the same patch file without problems, but PCMSK isn't found. I'd appreciate any help!

@Rahix
Copy link
Owner

Rahix commented Jun 5, 2023

Could not find CPU:PCMSK

It's looking for PCMSK in the CPU peripheral but the register is actually in the EXINT peripheral. Try like this:

EXINT:
  PCMSK:
    _add:
      PCINT0:
        ...

@mgrunwald
Copy link
Author

Thanks, that worked :)

Currently the PCMSK has a write constraint:

        <register>
          <name>PCMSK</name>
          …
          <writeConstraint>
            <range>
              <minimum>0</minimum>
              <maximum>255</maximum>
            </range>
          </writeConstraint>

Since the two most significant bits are reserved, I'd like to change the write constraint to 63 max, but again, I fail:

EXINT:
  _modify:
    PCMSK:
      _write_constraint: [0, 63]
svdtools.patch.UnknownTagError: ('register', '_write_constraint')

I think this should work, according to the svdtools docs. Can you please show me my mistake once more?

Apart from that , the PR would be ready.

@Rahix
Copy link
Owner

Rahix commented Jun 7, 2023

Can you please show me my mistake once more?

Hmm, this might be because we're relying on an old version of this tool...

Apart from that , the PR would be ready.

Please send it :) Let's continue there, it's easier for me if I have the code in front of me as well.

@mgrunwald
Copy link
Author

Please send it :) Let's continue there, it's easier for me if I have the code in front of me as well.

Done: #131

@mgrunwald
Copy link
Author

@Rahix ping?

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 a pull request may close this issue.

2 participants