i2c: Implement I2C controller via GPIO and bitbanging #58

harry · 2020-08-07T11:22:45+08:00

harry commented

2020-08-07 11:22:45 +08:00

Overview

This PR implements an I2C bitbanging controller using GPIOs on Zynq, currently supporting only the ZC706 board. This would help circumvent inconspicuous issues pertaining to the official I2C controller module for Zynq.

Sidenote: Possible Pitfall with Register Macros

During my implementation process, I discovered a possible pitfall when defining registers with the libregister macros. Specifically, if you only plan to use some non-contiguous registers in a block of registers, instead of a regular C-representation (#[repr(C)]) RegisterBlock that are often retrieved via a new() function as a mutable borrow from a static (&'static mut), you should define a simple class whose fields are individual &'static muts pointing to each of the non-contiguous registers, and then simply let other parts of the code to retrieve this class via a new(). For example, you should do:

// regs.rs
// Assume that you need to define a block of registers (e.g. for a controller) where:
// register_a is 32-bit and has an absolute address of 0x00010004;
// register_b is 32-bit and has an absolute address of 0x0001000c;

// Then, define a simple class as:
pub struct RegisterWrapper {
    pub register_a: &'static mut RegisterA,
    pub register_b: &'static mut RegisterB,
}

impl RegisterWrapper {
    pub fn new() -> Self {
        Self {
            register_a: RegisterA::new(),
            register_b: RegisterB::new(),
            // These two lines ensures the `new()` function defined by
            // the following `register_at!` macros would be called,
            // whenever `RegisterWrapper::new()` is called.
        }
    }
}

// And then, define these individual registers as:
register!(register_a, RegisterA, RW, u32);
register_at!(RegisterA, 0x00010004, new);
// ... insert your `register_bit!` macros here ...

register!(register_b, RegisterB, RW, u32);
register_at!(RegisterA, 0x0001000c, new);
// ... insert your `register_bit!` macros here ...

Then finally, in your code, you can access this block of registers using:

// mod.rs
pub struct MyController {
    regs: regs::RegisterWrapper
}

impl MyController {
    pub fn new() -> Self {
        Self {
            regs: regs::RegisterWrapper::new(),
        };
    }
    
    // Use the register block to modify or read the register, like:
    fn something() -> () {
        self.regs.register_a.modify(|_, w| {
            w.some_bit(true)
        });
    }
}

# Overview This PR implements an I2C bitbanging controller using GPIOs on Zynq, currently supporting only the ZC706 board. This would help circumvent inconspicuous issues pertaining to the official I2C controller module for Zynq. ## Sidenote: Possible Pitfall with Register Macros During my implementation process, I discovered a possible pitfall when defining registers with the `libregister` macros. Specifically, if you only plan to use some non-contiguous registers in a block of registers, instead of a regular C-representation (`#[repr(C)]`) `RegisterBlock` that are often retrieved via a `new()` function as a mutable borrow from a static (`&'static mut`), you should define a simple class whose fields are individual `&'static mut`s pointing to each of the non-contiguous registers, and then simply let other parts of the code to retrieve this class via a `new()`. For example, you should do: ```rs // regs.rs // Assume that you need to define a block of registers (e.g. for a controller) where: // register_a is 32-bit and has an absolute address of 0x00010004; // register_b is 32-bit and has an absolute address of 0x0001000c; // Then, define a simple class as: pub struct RegisterWrapper { pub register_a: &'static mut RegisterA, pub register_b: &'static mut RegisterB, } impl RegisterWrapper { pub fn new() -> Self { Self { register_a: RegisterA::new(), register_b: RegisterB::new(), // These two lines ensures the `new()` function defined by // the following `register_at!` macros would be called, // whenever `RegisterWrapper::new()` is called. } } } // And then, define these individual registers as: register!(register_a, RegisterA, RW, u32); register_at!(RegisterA, 0x00010004, new); // ... insert your `register_bit!` macros here ... register!(register_b, RegisterB, RW, u32); register_at!(RegisterA, 0x0001000c, new); // ... insert your `register_bit!` macros here ... ``` Then finally, in your code, you can access this block of registers using: ```rs // mod.rs pub struct MyController { regs: regs::RegisterWrapper } impl MyController { pub fn new() -> Self { Self { regs: regs::RegisterWrapper::new(), }; } // Use the register block to modify or read the register, like: fn something() -> () { self.regs.register_a.modify(|_, w| { w.some_bit(true) }); } } ```

sb10q closed this pull request

2020-08-07 11:35:49 +08:00

sb10q reviewed 2020-08-07 11:49:24 +08:00

libboard_zynq/src/i2c/mod.rs

													
				@ -0,0 +20,4 @@

				impl I2C {

				    #[cfg(feature = "target_zc706")]

				    pub fn i2c() -> Self {

sb10q commented

2020-08-07 11:49:24 +08:00

Shouldn't this function be called new?

Shouldn't this function be called ``new``?

harry commented

2020-08-07 11:54:00 +08:00

Poster

I named it this way by referring to the UART implementation (the constructor function on lines 16-60). But I do realise new() would be more suitable. I wonder: why is the constructor named this way on UART?

I named it this way by referring to the UART implementation [(the constructor function on lines 16-60)](https://git.m-labs.hk/M-Labs/zynq-rs/src/branch/master/libboard_zynq/src/uart/mod.rs#L16-L60). But I do realise `new()` would be more suitable. I wonder: why is the constructor named this way on UART?

sb10q commented

2020-08-07 11:55:00 +08:00

I don't think there's a good reason for this and I think it should be named new in the UART as well.

I don't think there's a good reason for this and I think it should be named ``new`` in the UART as well.

harry commented

2020-08-07 12:07:05 +08:00

Poster

I see. For future reference, the Eth class in the eth module also has its constructor named something other than new() (line 151). So in the experiment main.rs code, the Ethernet driver is instantiated with zynq::eth::Eth::default(hardware_addr);.

I see. For future reference, the `Eth` class in the `eth` module also has its constructor named something other than `new()` [(line 151)](https://git.m-labs.hk/M-Labs/zynq-rs/src/branch/master/libboard_zynq/src/eth/mod.rs#L151). So in the experiment `main.rs` code, the Ethernet driver is instantiated with `zynq::eth::Eth::default(hardware_addr);`.

sb10q reviewed 2020-08-07 11:51:08 +08:00

sb10q reviewed 2020-08-07 11:54:03 +08:00

libboard_zynq/src/i2c/mod.rs

													
				@ -0,0 +11,4 @@

				const INVALID_BUS: &'static str = "Invalid I2C bus";

				#[cfg(feature = "target_zc706")]

				const GPIO_OUTPUT_MASK: u16 = 0xFFFF - 0x000C;