diondokter / device-driver

A toolkit to create better Rust device drivers, faster
https://diondokter.github.io/device-driver/
Apache License 2.0
131 stars 5 forks source link

Add object type "memory" #47

Open chrenderle opened 1 day ago

chrenderle commented 1 day ago

Motivation

The device ST25DV has a user memory area that is up to 64kBytes big. That means setting it as a big register is not feasible since this would require 64kByte on the stack to read or write it. A buffer also doesn't make sense in this case.

Design

In my opinion a new field type "memory" would make sense. As discussed here I want to describe how I think this field type could look:

How does the user specify it

It would share many similarities to a register. So the following attributes would be identical:

The following should be different/new:

What should the generated code be like to use

I'm orienting myself on embedded-storage here.

Writing should look like this:

let buffer: [u8; 16] = /* some data here */;
let offset = 0;
my_device.write(offset, &buffer); // write the first 16 bytes

let buffer: [u8; 16] = /* some other data here */;
let offset = 2;
my_device.write(offset, &buffer); // write 16 bytes starting from byte 2

Reading should look like this:

let mut buffer: [u8; 16] = [0; 16];
let offset = 0;
my_device.read(offset, &mut buffer); // read the first 16 bytes

let mut buffer: [u8; 16] = [0; 16];
let offset = 2;
my_device.read(offset, &mut buffer); // read 16 bytes starting from byte 2

This is basically 1:1 embedded-hal NorFlash (docs) without the erase stuff. Maybe the erase stuff also makes sense. I don't know if there are any devices which would require this.

Interface trait

The interface trait would also look very similar to NorFlash from embedded-hal:

pub trait MemoryInterface {
    type Error;
    type AddressType: Copy;

    fn write_memory(
        &mut self,
        address: Self::AddressType,
        offset: u32,
        data: &[u8],
    } -> Result<(), Self::Error>;

    fn read_memory(
        &mut self,
        address: Self::AddressType,
        offset: u32,
        data: &mut [u8],
    } -> Result<(), Self::Error>;
}
diondokter commented 12 hours ago

Hi, thanks for opening the issue!

First off, this proposal would work for the usecase you're working on. However, I've been thinking about it and I don't think this is the right abstraction level for this toolkit.

See, a memory interface is quite high level. And for chips like most EEPROMs, it doesn't directly map to the interface the chips provide.

So if you create a driver for an EEPROM, what does it look like? For write, you send the write command, the start address and then the bytes to be written. It looks similar for read. So really, a memory interface as you propose will often be implemented using commands.

Also, you mention erase. For NOR-flash that would be required too. So that's also difficult to solve. Ideally we'd not force an erase implementation for chips that don't need it. Add to that, that I'm not entirely happy with the current traits: https://github.com/rust-embedded-community/embedded-storage/issues/58

So, what can we do?

I think it'd be better to extend the possibilities for commands so they can carry data buffer slices. Then you could have a write command with an address and have the user give the bytes that need to be written. Big question though is what that should exactly look like.

Maybe something like this:

"Write": {
    "type": "command",
    "address": 5,
    "input_buffer": true, <== New
    "size_bits_in": 32,
    "fields_in": {
        "address": {
            "base": "uint",
            "start": 0,
            "end": 32
        }
    },
}
device.write().dispatch(|data| data.set_address(1234), &[0, 1, 2, 3, 4, 5]).unwrap();

Thoughts?