[missing-feature] capsule: i2c_master_slave: return xfer length to userspace

[twilfredo]

Hey all,

Looking at the Kernel HIL for I2C for a command complete, there's no way for the driver to accurately return the number of bytes transceived. For example:

tock/kernel/src/hil/i2c.rs

Line 190 in 50dd293

fn command_complete(&self, buffer: &'static mut [u8], status: Result<(), Error>);

does not allow a length.

So returning bytes read/sent information back to userspace seems incomplete, for example, we don't actually return a length back. In

tock/capsules/core/src/i2c_master_slave_driver.rs

Line 151 in 50dd293

kernel_data.schedule_upcall(0, (1, status, 0)).ok();

Side-note: what is the current meaning of r.0 (i.e 1)?

For more context, I'm in the process adding libtock-rs api support for i2c-master-slave. I have basic communication working, but would be nice to patch up the kernel to also get the drivers to return transfer/read lengths back to userspace as r.0. From my understanding, we need to change I2CHwMasterClient::command_complete() to have a length, then in the capsule pass this back to userspace. I suppose this also means we have to change all the drivers to do so too.

What do you guys think about this? Is there a better way to get this information back to userspace? Is there a reason for this not be added etc...

Thanks!

[lschuermann]

From a brief look over this, it's definitely the case that our documentation of this HIL is lacking. Furthermore, this userspace API, despite being part of the core capsules, does not yet have a TRD. We should rectify both eventually.

there's no way for the driver to accurately return the number of bytes transceived. For example:

Concerning this, I believe that the HIL itself does not necessarily need to pass this information back -- the length parameter passed into the respective read, write, or read_write calls always determines the exact amount of data transcieved. IIRC, with an I2C bus, this is always determined by the master device.

I do agree that in the userspace API, with multiple potentially concurrent operations, keeping track of this state locally in the application seems less than ideal. I'm open to returning this length back from the userspace driver (and thus keeping track of it there internally). For libtock-rs specifically, given that we can't really have multiple such asynchronous operations run in parallel (i.e. the current style of syscall drivers would block), it may also be a reasonable workaround to keep track of this in the application itself.

[twilfredo]

From a brief look over this, it's definitely the case that our documentation of this HIL is lacking. Furthermore, this userspace API, despite being part of the core capsules, does not yet have a TRD. We should rectify both eventually.

there's no way for the driver to accurately return the number of bytes transceived. For example:

Concerning this, I believe that the HIL itself does not necessarily need to pass this information back -- the length parameter passed into the respective read, write, or read_write calls always determines the exact amount of data transcieved. IIRC, with an I2C bus, this is always determined by the master device.

Okay sounds good.

I do agree that in the userspace API, with multiple potentially concurrent operations, keeping track of this state locally in the application seems less than ideal. I'm open to returning this length back from the userspace driver (and thus keeping track of it there internally).

Here do you mean from the userspace driver to return back to the caller basically the min(length, buff.len())? Where length is the argument passed into the userspace driver from the application, and buff is the data buffer from the application. So basically handle all of it in the userspace driver without changes to the kernel. That is, if there were no errors (status == Ok(())), safe to assume the entire length requested was transcieved.

Similar to:

tock/capsules/core/src/i2c_master_slave_driver.rs

Line 170 in 50dd293

let len = cmp::min(app_buffer.len(), read_len as usize);

Alternatively, couldn't the len above be returned in r.0 in the subsequent schedule_upcall() ? Currently it just seems like r.0 returns an arbitrary value/(I'm not sure what it means)?

For libtock-rs specifically, given that we can't really have multiple such >asynchronous operations run in parallel (i.e. the current style of syscall >drivers would block), it may also be a reasonable workaround to keep track >of this in the application itself.