Add object type "memory"

[chrenderle]

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:

• access
• address
• byteorder
• bitorder
• repeat
• allow bit overlap
• allow address overlap
• cfg
• description

The following should be different/new:

• alignment: What alignment has to be considered when accessing the memory. When the memory is stored as u16 this would be 2. U8 would be 1, u32 4, etc.
• size_bytes: this would describe how many bytes the memory is long.

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]

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: rust-embedded-community/embedded-storage#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?

[chrenderle]

In my opinion the commands are not the best way to access memory if the memory feature is not added. Its still a workaround.

Instead I would prefer access to the interface with a mutable reference. This is already possible with the method "interface" on the device driver struct. For my case i can add a method "write_eeprom" and "read_eeprom".

[diondokter]

I'm not sure why you think having commands with buffer data is a workaround.

For example, in this datasheet of your device: https://www.st.com/resource/en/datasheet/st25dv04k.pdf
Section 7.6 describes the RF commands with which you can read and write memory.
For example 7.6.14 Write Multiple Blocks, where you give a command and some data.

They look like commands, and in the datasheet they're called commands too.

If we had commands with buffer data you could implement the interface like:

i2c.transaction(DEV_ID, &mut [
    Operation::Write(&[request_flags, command_address]),
    Operation::Write(command_data), // The fieldset containing the first block number and the number of blocks
    Operation::Write(command_buffer), // The slice of bytes the user provides
    Operation::Write(&calculated_crc.to_le_bytes()),
]).unwrap();

This is modeled exactly how the datasheet specifies. I don't know what part of this would be a workaround. To me this seems like the proper solution.

The problem I have with the memory interface isn't that it wouldn't your problem. It would! My problem is that it's not actually modeling what the device is doing. Maybe I'm wrong, but it seems like your desire is for a high level interface while this toolkit only really generates a low level interface. For the memory interface, you'd have to implement some commands from scratch which is exactly the thing this toolkit is trying to avoid.

Instead, the intended use is create an easy to use high level layer on top of the generate low level driver.

For my case i can add a method "write_eeprom" and "read_eeprom".

That's completely fine and good. In my S2-LP driver I have separated the low level parts from the high level parts. The high level struct contains the low level generated struct:
https://github.com/diondokter/s2lp/blob/36684eaa45be188d58f1668558adb866a06dd25a/src/lib.rs#L12-L19

This allows better modelling of the functionality for this chip. You might want to do the same maybe.

[chrenderle]

The section you referred to is for the RF access. I'm only modelling the I2C access part. The ST25DV is a dynamic NFC tag where you can access the EEPROM via I2C while at the same time a NFC reader can access the EEPROM via RF. The RF side works with commands where a command to access the EEPROM would make sense.

For the I2C access the EEPROM memory is the same as a big register with 64K of consecutive bytes. So reading the first byte of the EEPROM at address 0x0000 or reading the first byte of a register (for example EH_CTRL_Dyn) at 0x2002 would look exactly the same apart from the address. So:

i2c.transaction(DEV_ID, &mut [
    Operation::Write(memory_address.get_address_slice()), // the u16 address as &[u8; 2]
    Operation::Read(&mut data),
]).unwrap();

Since the I2C interface to the IC only knows memory addresses it would make sense for me to have a low-level interface to the EEPROM memory as I described initially. The high level layer on top would then consider additional things like the I2C password or the user memory areas.

I hope I described my thoughts well enough. I still need to get better at explaining my thoughts.

[diondokter]

Ah ok, now I see. It's probably not just you. I've been sick for a couple of days, so my thinking isn't entirely straight haha.

I'll take some time to think about how best to model this and look around how other people tend to solve this.

[chrenderle]

While working on the driver I found 1 difference between the registers and the eeprom memory. After writing a byte to a eeprom address you have to wait 5ms for the data to be written. This is not the case for registers. So they are not identical from the driver perspective.

[diondokter]

Ah yes. Usually for eeproms or flash there's a status register somewhere with a 'busy' bit that you're supposed to poll.

Btw, I'm coming around on the idea of adding this. Though probably the name would become 'region' or 'memregion' or something like that.

The biggest concern I have now is whether there's enough justification for adding it. I mean, what would the toolkit do for you that would not be straightforward when doing it by hand?
On the other hand, you can say the same about the buffer objects which are also very simple...

[chrenderle]

As far as I understand the documentation there is no busy bit somewhere but the IC just doesn't acknowledge on the i2c bus if you try to write data while it is still writing to the EEPROM. There is also a section about polling on ack.

Initially I thought that I could only access the EEPROM if it was declared in the device driver. But while implementing the driver I found out that I can access it via the mutable reference to the interface. So for my implementation there is no hard need for this. Though I would say it is nicer to have everything declared in the device driver. When someone looks at the code they can find everything in one place.