Skip to content

devices: add basic support for ATtiny1626 #185

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

Merged
merged 1 commit into from
Apr 23, 2025

Conversation

ckoehne
Copy link
Contributor

@ckoehne ckoehne commented Apr 16, 2025

Pull in the ATDF file from Microchip and add all the necessary plumbing around the code-base to make it compile.

It hasn't been tested much yet and no device specific patches are included. So far, I've tested some accesses to the PORTx register on hardware. It seems to work expect for VPORTA.DIR. VPORTA.DIR resolves to address 0 which causes rust to panic on access by default.

@Rahix
Copy link
Owner

Rahix commented Apr 16, 2025

Hi, thanks a lot for your contribution! Just a heads-up, I am holding off merging new chips right now until #157 is merged, which quite significantly changes the way the register definitions are generated. I will ping you once this change has landed so you can rebase your changes ontop of it.


It seems to work expect for VPORTA.DIR. VPORTA.DIR resolves to address 0 which causes rust to panic on access by default.

Hm, probably means that some patches are necessary. Check the generated SVD file for the register in question to see whether the definitions looks good. If not, you'll need to write a patch to fix it. If you need help with that, don't hesitate asking!

@ckoehne
Copy link
Contributor Author

ckoehne commented Apr 17, 2025

Hi, thanks a lot for your contribution! Just a heads-up, I am holding off merging new chips right now until #157 is merged, which quite significantly changes the way the register definitions are generated. I will ping you once this change has landed so you can rebase your changes ontop of it.

Thanks.

It seems to work expect for VPORTA.DIR. VPORTA.DIR resolves to address 0 which causes rust to panic on access by default.

Hm, probably means that some patches are necessary. Check the generated SVD file for the register in question to see whether the definitions looks good. If not, you'll need to write a patch to fix it. If you need help with that, don't hesitate asking!

The generated SVD file looks sane (see below). The baseAddress of VPORTA is 0 and the addressOffset of DIR is 0 too, so VPORTA.DIR has address 0, which is indeed correct. It looks like rust is unable to access memory address 0 [1].

[1] https://users.rust-lang.org/t/reading-from-physical-address-0x0/117408/2

      <name>VPORTA</name>
      <description>Virtual Ports</description>
      <baseAddress>0x00000000</baseAddress>
      <addressBlock>
        <offset>0x0</offset>
        <size>0x4</size>
        <usage>registers</usage>
      </addressBlock>
      <registers>
        <register>
          <name>DIR</name>
          <description>Data Direction</description>
          <addressOffset>0x0</addressOffset>
          <size>0x8</size>
          <access>read-write</access>
          <writeConstraint>
            <range>
              <minimum>0</minimum>
              <maximum>255</maximum>
            </range>
          </writeConstraint>
        </register>

@Rahix
Copy link
Owner

Rahix commented Apr 18, 2025

Here's the notification: #157 was merged 🎉 Please rebase your work ontop of the new code-generation. You should be able to make out the new structure in src/devices/mod.rs from the way things look for the existing chips. The Makefile is dropped completely.

@LuigiPiucco
Copy link
Contributor

Hm, probably means that some patches are necessary.

I just checked the datasheet for the MCU, and it seems address 0x0 is correct; contrary to usual (old?) AVR chips, this one seems to have IO space mapped to data space at 0x0 instead of 0x20 like the other ones. I think the issue may be that Rust inserts a panic check for address 0x0 before actually dereferencing, if so we will probably require assembly in some form, or some fix in the compiler. I would have thought {read|write}_volatile operations, which are used via Vcell, wouldn't have this problem, but maybe they do. We'd need to inspect the compiled binary to be sure, though.

@Rahix
Copy link
Owner

Rahix commented Apr 18, 2025

Maybe svd2rust can learn to special case address 0 and somehow interface with a library provided symbol for read/write access to address 0? I think hardcoding platform-specific assembly in svd2rust won't fly.

@ckoehne
Copy link
Contributor Author

ckoehne commented Apr 19, 2025

Thanks for the notification!

Regarding the address 0x0 issue. I've compiled the following code and used avr-objdump to check the output binary:

#![no_std]
#![no_main]

use panic_halt as _;

#[no_mangle]
pub extern "C" fn main() -> ! {
    let dp = avr_device::attiny1626::Peripherals::take().unwrap();
    dp.vporta.dir().write(|w| w.set(1 << 7));
    loop {
    }
}
avr-objdump -D -m tinyavr2 target/avr-none/debug/test.elf
000000a0 <main>:
  a0:   8f b7           in      r24, 0x3f       ; 63
  a2:   f8 94           cli
  a4:   90 91 00 38     lds     r25, 0x3800     ; 0x803800 <DEVICE_PERIPHERALS>
  a8:   91 30           cpi     r25, 0x01       ; 1
  aa:   19 f4           brne    .+6             ; 0xb2 <.Lname24+0x2>
  ac:   8f bf           out     0x3f, r24       ; 63
  ae:   0e 94 69 00     call    0xd2    ; 0xd2 <_ZN4core6option13unwrap_failed17h6e37d64f64b596daE>
  b2:   91 e0           ldi     r25, 0x01       ; 1
  b4:   90 93 00 38     sts     0x3800, r25     ; 0x803800 <DEVICE_PERIPHERALS>
  b8:   8f bf           out     0x3f, r24       ; 63
  ba:   0e 94 67 00     call    0xce    ; 0xce <_ZN4core9panicking14panic_nounwind17h63d81beb9bde1088E>

As you can see, the code immediately panics instead of even trying to access address 0x0. When using e.g. VPORTB instead of VPORTA, the resulting binary looks good:

000000a0 <main>:
  a0:   8f b7           in      r24, 0x3f       ; 63
  a2:   f8 94           cli
  a4:   90 91 00 38     lds     r25, 0x3800     ; 0x803800 <DEVICE_PERIPHERALS>
  a8:   91 30           cpi     r25, 0x01       ; 1
  aa:   39 f0           breq    .+14            ; 0xba <.Lname25+0x3>
  ac:   91 e0           ldi     r25, 0x01       ; 1
  ae:   90 93 00 38     sts     0x3800, r25     ; 0x803800 <DEVICE_PERIPHERALS>
  b2:   8f bf           out     0x3f, r24       ; 63
  b4:   80 e8           ldi     r24, 0x80       ; 128
  b6:   84 b9           out     0x04, r24       ; 4
  b8:   ff cf           rjmp    .-2             ; 0xb8 <.Lname25+0x1>
  ba:   8f bf           out     0x3f, r24       ; 63
  bc:   0e 94 64 00     call    0xc8    ; 0xc8 <_ZN4core6option13unwrap_failed17h6e37d64f64b596daE>

@ckoehne ckoehne force-pushed the corvink/attiny1626 branch 2 times, most recently from 5885a55 to a1a99cd Compare April 19, 2025 16:41
@Rahix
Copy link
Owner

Rahix commented Apr 21, 2025

Okay, as the VPORTx registers are only an optimization and there is a way to use the I/O pins without them, I'd say let's patch out the VPORTA register for now so people won't get confused. Then I would suggest raising an issue with svd2rust to get some input on how they would suggest handling the situation. We can add back the VPORTA once a solution has been found.

@LuigiPiucco
Copy link
Contributor

LuigiPiucco commented Apr 21, 2025

In the meantime, I asked about this on Zulip, and it seems the access to 0 issue could be fixed via the compiler, with a small addition. It will require an RFC, so it may take a moment to arrive, but then we'd solve the issue for all MCUs, and without loosing access to anything.

Edit: it appears an RFC is unneeded, so it can be fixed relatively fast. I'll write a proper PR soon.

@ckoehne
Copy link
Contributor Author

ckoehne commented Apr 22, 2025

Thanks a lot for your help and fast response!

How should we continue? Should I opt out VPORTA.DIR to get this merged or do you prefer waiting for your PR to get merged?

@Rahix
Copy link
Owner

Rahix commented Apr 22, 2025

I'd remove VPORTA.DIR for now and then add it back later once the compiler supports it. Adding it back is not a breaking change, so we can do this at any time.

@ckoehne ckoehne force-pushed the corvink/attiny1626 branch 2 times, most recently from 97ca55d to 11750a5 Compare April 23, 2025 16:02
Pull in the ATDF file from Microchip and add all the necessary plumbing
around the code-base to make it compile.

It hasn't been tested much yet. So far, I've tested some accesses to the
PORTx register on hardware. The common patches for the xmega series seem
reasonable, so I've applied them. Additionally, VPORTA.DIR was patched
out for 2 series devices because it doesn't work. VPORTA.DIR is mapped
to memory address 0x0. This address is commonly used as NULL pointer and
it looks like rust isn't able to handle accesses to that address [1]. To
check that, I've created a short example programm accessing VPORTA.DIR
[2] and checking the resulting binary with avr-objdump [3]. The produced
binary panics as soon as I'm trying to access VPORTA.DIR [4]. By
changing the code to access a different register of VPORTA like OUT, the
code produces a sane binary [5].

[1] https://users.rust-lang.org/t/reading-from-physical-address-0x0/117408/2
[2]
```
#![no_std]
#![no_main]

use panic_halt as _;

pub extern "C" fn main() -> ! {
    let dp = unsafe { avr_device::attiny1626::Peripherals::steal() };
    dp.vporta.dir().write(|w| w.set(1 << 7));
    loop {}
}
```
[3] `avr-objdump -D -m tinyavr2 target/avr-none/debug/test.elf`
[4]
```
000000a0 <main>:
  a0:   81 e0           ldi     r24, 0x01       ; 1
  a2:   80 93 00 38     sts     0x3800, r24     ; 0x803800 <DEVICE_PERIPHERALS>
  a6:   0e 94 59 00     call    0xb2    ; 0xb2 <_ZN4core9panicking14panic_nounwind17h63d81beb9bde1088E>
```
[5]
```
000000a0 <main>:
  a0:   81 e0           ldi     r24, 0x01       ; 1
  a2:   80 93 00 38     sts     0x3800, r24     ; 0x803800 <DEVICE_PERIPHERALS>
  a6:   80 e8           ldi     r24, 0x80       ; 128
  a8:   81 b9           out     0x01, r24       ; 1
  aa:   ff cf           rjmp    .-2             ; 0xaa <.Luint32_t_name+0x3>
```

Signed-off-by: Corvin Köhne <[email protected]>
@ckoehne ckoehne force-pushed the corvink/attiny1626 branch from 11750a5 to deb3651 Compare April 23, 2025 16:04
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! Please open an issue to track the fix for VPORTA.DIR so it doesn't get lost.

@Rahix Rahix merged commit 0eb1616 into Rahix:main Apr 23, 2025
2 checks passed
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