Skip to content
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

MSP430 Support #121

Closed
2 of 4 tasks
cr1901 opened this issue Sep 8, 2019 · 18 comments
Closed
2 of 4 tasks

MSP430 Support #121

cr1901 opened this issue Sep 8, 2019 · 18 comments
Labels
Porting Adding new mcu/board/os support

Comments

@cr1901
Copy link
Collaborator

cr1901 commented Sep 8, 2019

Porting layer

  • Device Controller Driver (DCD)
  • Host Controller Driver (HCD)
  • Board Supported Package (BSP)
  • OS Abstraction Layer (OSAL)

Description
I do the vast majority of MSP430 programming in Rust nowadays. IMO it's actually easier to get a Rust toolchain set up than a C one. However, I still think non-ARM/non-32-bit support of tinyusb is worthwhile.

This would be the board I would port: http://www.ti.com/tool/MSP-EXP430F5529LP

@cr1901 cr1901 added the Porting Adding new mcu/board/os support label Sep 8, 2019
@hathach
Copy link
Owner

hathach commented Sep 9, 2019

Great idea, adding non-ARM would be great, the stack doesn't rely on any ARM specific feature, hopefully it is easy to port. I will buy the board in the your link.

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 13, 2019

My Launchpad arrived today. I will hopefully start the port this weekend :).

@pigrew
Copy link
Collaborator

pigrew commented Sep 14, 2019

I started writing updating the compiler quirks header file for the TI compiler. I'll finish writing it, and post a PR for it. I was running it for its MISRA reports. (I was using Tiva, not MSP430)

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 14, 2019

@pigrew That would help me a lot if you can do that, tyvm :)! However, I would prefer to write the actual driver myself because this is in fact a warmup for me for MSP430 USB support in Rust.

@pigrew
Copy link
Collaborator

pigrew commented Sep 14, 2019

@cr1901, I feel like I should be learning Rust, but just haven't yet. I'm sticking to C for now.... but I'd be happy to review your code once it's written.

I need to figure out how to even compile Rust. Is it compiled to C? Or just in LLVM? How's the ARM backend in LLVM doing? I read its linker isn't reliable yet?

As much as I've working on all the modules in tinyusb, I'm trying to focus only on what will get USBTMC working for me on STM32 (which means FSDEV, core, and implementing a class driver). I don't even have any USB-capable MSP430 here.... no, nothing to worry about.

The newer launchpads have thing EnergyTrace thing which is quite great. It'll give you a real-time plot of the board's current consumption, so you can know if your sleep modes are working, etc.

#157 has my compiler options patch, though I've only tested compilation for the TI ARM compiler, and not MSP430 (though I did mention MSP430 in an ifdef.... hopefully it's right). Perhaps you can review the patch to see if it works.

@hathach
Copy link
Owner

hathach commented Sep 14, 2019

@pigrew Hey, you can file an issue for TI Tiva as well, I think I have an Tiva C board laying somewhere on my shelf 😃

Hmm, what is Rust 🤔 , look like it is time to upgrade my brain 😄😄

@rajivr
Copy link

rajivr commented Sep 14, 2019

@cr1901 How are you planning on adding rust support to TinyUSB?

I am new to TinyUSB, following is the bindgen generated APIs that I currently have. One thing that noticed was that Rust lacks support for packed structs, so most likely I'll need to add descriptor support from C-land.

use tinyusb_sys::tusb_desc_endpoint_t;

// DCD = Device Controller Driver
#[no_mangle]
pub extern "C" fn dcd_init(rhport: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_int_enable(rhport: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }

    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_edpt_open(rhport: u8, p_endpoint_desc: *const tusb_desc_endpoint_t) -> bool {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_edpt_xfer(
    rhport: u8,
    ep_addr: u8,
    buffer: *mut u8,
    total_bytes: u16,
) -> bool {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_edpt_stall(rhport: u8, ep_addr: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_edpt_clear_stall(rhport: u8, ep_addr: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_int_disable(rhport: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_set_address(rhport: u8, dev_addr: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn dcd_set_config(rhport: u8, config_num: u8) {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}
#![no_std]
#![feature(asm)]

#[link(name = "asmhelpers")]
extern "C" {
    pub fn __gnu_thumb1_case_shi();
}

/// USB Endpoint Descriptor
#[allow(non_camel_case_types)]
pub enum tusb_desc_endpoint_t {}

#[no_mangle]
pub extern "C" fn tud_descriptor_configuration_cb(index: u8) -> *const u8 {
    let _ = index;
    
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn tud_descriptor_device_cb() -> *const u8 {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

#[no_mangle]
pub extern "C" fn tud_descriptor_string_cb(index: u8) -> *const u16 {
    unsafe {
        asm!("bkpt" :::: "volatile");
    }
    unimplemented!();
}

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 14, 2019

@rajivr

How are you planning on adding rust support to TinyUSB?

I wasn't. I was planning to implement the UsbBus trait from usb-device for MSP430, so when usb support in Rust matures, MSP430 will be ready.

I wasn't.

Okay, so this is not quite true. I started writing a tinyusb-sys crate for Rust, focusing on the build.rs part and deferring bindgen until later. But I ran into a few snags:

  1. I have a config option in tinyusb-sys to provide a path to the tinyusb C source. The reality is that I need to provide this config option in tinyusb-rs as well, and I'm unsure how to propagate the config option down to tinyusb-sys.

  2. Ditto for tusb_config.h, whose path is user-configurable but I don't know if I want Rust users to have to write a C header file. It would be nice to autogenerate this in the user application (think memory.x, except maybe a TOML config file or similar). The build.rs step of tinyusb-rs can auto-convert to a C header file that's passed down to tinyusb-sys.

    EDIT: I think I could use config options to always autogenerate tusb_config.h from tinyusb-rs, and then set the relevant define to user-override the path to it. The propagation problem of sending tusb_config.h path down to tinyusb-sys still applies.

  3. The high level bindings (tinyusb-rs) will be a bit involved thanks to non-weak callbacks in tinyusb. I'm not sure how to represent these callbacks in the high-level bindings (other than "the user application that uses tinyusb-rs will have to implement them or compilation will fail, akin to, say, a lang item like `panic_handler").

  4. Descriptor generation. As you pointed out, this is not going to be fun. cbindgen can help here too.

I'm not sure I want to maintain such a crate right now.

One thing that noticed was that Rust lacks support for packed structs, so most likely I'll need to add descriptor support from C-land.

#[repr(packed)] should work.

@hathach If you are following along, here's some context:

  1. You're expected to use the Rust compiler with a package manager called cargo or xargo. For various reasons, ARM world uses cargo, and MSP430 uses xargo. Rust libraries are called crates.

  2. The convention in Rust world for interfacing to C is to have two crates: One called tinyusb-sys which is "raw" C bindings, and a higher-level crate that I'll call tinyusb-rs that the end user is supposed to use directly; tinyusb-sys is a direct dependency of tinyusb-rs.

  3. build.rs is a convention for compiling C code that's linked into the final Rust application (tinyusb itself, for instance), and also code generation (converting a tinyusb config file into a C header, for instance).

  4. I mention config options, which is used for conditional code compilation/gates. The rust compiler enables/disables gates using the --cfg command line parameter (i.e. no preprocessor). cargo also knows how to handle config options.

@rajivr
Copy link

rajivr commented Sep 14, 2019

One thing that noticed was that Rust lacks support for packed structs, so most likely I'll need to add descriptor support from C-land.

#[repr(packed)] should work.

That's correct. Sorry I meant bitfield support.

@hathach
Copy link
Owner

hathach commented Sep 14, 2019

@cr1901 thanks for information, there seems to be a lot to learn with Rust. Let's me know if you need any helps to make tinyusb more friendly with rust. I probably wont help much with the rust implementation 😉

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 14, 2019

@hathach Could you preemptively download the "msp430-gcc-support-files-1.207.zip" file from this page and preemptively make a repo for me that I'll include as a submodule? This zip file contains the msp430 include files and linker scripts.

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 14, 2019

@rajivr Eep. Err, my personal opinion is that using bitfields in C is a bug, not a feature. Where are they used specifically in tinyusb when generating descriptors?

@rajivr
Copy link

rajivr commented Sep 14, 2019

Where are they used specifically in tinyusb when generating descriptors?

I recall seeing it here.

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 14, 2019

bmAttributes will become a u8; wMaxPacketSize will become a u16, and we can use macros to align the bitfields appropriately at compile-time. This isn't ideal, but I think it'll work.

@hathach
Copy link
Owner

hathach commented Sep 14, 2019

@cr1901 I am out now, but I think it would be a folder as follows

hw/mcu/ti/msp_driver

Where msp_driver (or msp430_driver) is the submodule. I will check their family line up for the name. Though if you know what it should, just create it locally for now, I will creat submodule as the name you suggested later. And you can make an pr to submodule to add those files.

@hathach
Copy link
Owner

hathach commented Sep 14, 2019

@rajivr those aren't required to build descriptor, you can just generate an byte array for descriptor, it is meant for the stack to parse the desc. User application can use it or not, In fact, in my example, I also use the uint8 array for descriptor using these macro templates

https://github.com/hathach/tinyusb/blob/master/src/device/usbd.h#L164

@cr1901
Copy link
Collaborator Author

cr1901 commented Sep 16, 2019

@rajivr I have uploaded what I have so far (not much) if interested and invited you to be a collaborator: https://github.com/cr1901/tinyusb-sys. Let's discuss more on that repo (I won't be coding on it at the moment- my bandwidth is too thin- but happy to discuss).

@hathach
Copy link
Owner

hathach commented Mar 29, 2020

implemented by #194

@hathach hathach closed this as completed Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Porting Adding new mcu/board/os support
Projects
None yet
Development

No branches or pull requests

4 participants