Rahix / avr-device

Register access crate for AVR microcontrollers
Apache License 2.0
173 stars 66 forks source link

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

Closed mgrunwald closed 10 months ago

mgrunwald commented 1 year ago

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 commented 1 year ago

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:

https://github.com/Rahix/avr-device/blob/aa5b20202063a92378baf54ba7a783a2d642dcc6/patch/common/pll.yaml#L6-L10

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Done: https://github.com/Rahix/avr-device/pull/131

mgrunwald commented 1 year ago

@Rahix ping?